block: move sd_format_disk_name() into block core as disk_name_format()
Subsystems (5)
cover block: move sd_format_disk_name() into block core as disk_name_format()
This patch series renames "sd_format_disk_name()" to "disk_name_format()" and moves it into block core. So that who needs formatting disk name can use it, instead of duplicating these similar help functions.
Ren Mingxin (4):
block: add function disk_name_format() into block core
scsi: replace sd_format_disk_name() to disk_name_format()
block: replace rssd_disk_name_format() to disk_name_format()
virtio_blk: use disk_name_format() to support mass of disks naming
block/genhd.c | 49 ++++++++++++++++++++++++++++++++++++++ drivers/block/mtip32xx/mtip32xx.c | 33 ------------------------- drivers/block/virtio_blk.c | 13 ---------- drivers/scsi/sd.c | 48 ------------------------------------- include/linux/genhd.h | 2 + 5 files changed, 54 insertions(+), 91 deletions(-)
Any comment will be appreciated.
Thanks,
Ren
PATCH 1/4 block: add function disk_name_format() into block core
According to 3e1a7ff8a0a7b948f2684930166954f9e8e776fe,
I copied function "sd_format_disk_name()" into block core with the name of "disk_name_format()".
--- block/genhd.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/genhd.h | 2 ++ 2 files changed, 51 insertions(+)
block/genhd.c +49 -0
@@ -357,6 +357,55 @@ void unregister_blkdev(unsigned int major, const char *name)EXPORT_SYMBOL (unregister_blkdev); +/* +* disk_name_format - format disk name +* @prefix: name prefix - ie. "sd" for SCSI disks +* @index: index of the disk to format name for +* @buf: output buffer +* @buflen: length of the output buffer +* +* Take SCSI disks for example. Disk names starts at sda. The +* 26th device is sdz and the 27th is sdaa. The last one for +* two lettered suffix is sdzz which is followed by sdaaa. +* +* This is basically 26 base counting with one extra 'nil' entry +* at the beginning from the second digit on and can be +* determined using similar method as 26 base conversion with the +* index shifted -1 after each digit is computed. +* +* CONTEXT: +* Don't care. +* +* RETURNS: +* 0 on success, -errno on failure. +*/ + +int disk_name_format (char *prefix,int index,char *buf,int buflen) +{ +const int base ='z' -'a' +1 ; +char *begin = buf +strlen (prefix); +char *end = buf + buflen; +char *p; +int unit; + + p = end -1 ; + *p ='\0' ; + unit = base; +do { +if (p == begin) +return -EINVAL ; + *--p ='a' + (index % unit); + index = (index / unit) -1 ; + }while (index >=0 ); + +memmove (begin, p, end - p); +memcpy (buf, prefix,strlen (prefix)); + +return 0 ; +} + +EXPORT_SYMBOL (disk_name_format); +static struct kobj_map *bdev_map;/**
include/linux/genhd.h +2 -0
@@ -407,6 +407,8 @@ static inline void free_part_info(struct hd_struct *part)extern void part_round_stats (int cpu,struct hd_struct *part);/* block/genhd.c */ +extern int disk_name_format (char *prefix,int index,char *buf,int buflen); +extern void add_disk (struct gendisk *disk);extern void del_gendisk (struct gendisk *gp);extern struct gendisk *get_gendisk (dev_t dev,int *partno);
PATCH 1/4 block: add function disk_name_format() into block core
In reply to Ren Mingxin According to 3e1a7ff8a0a7b948f2684930166954f9e8e776fe,
I copied function "sd_format_disk_name()" into block core with the name of "disk_name_format()".
--- block/genhd.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/genhd.h | 2 ++ 2 files changed, 51 insertions(+)
block/genhd.c +49 -0
@@ -357,6 +357,55 @@ void unregister_blkdev(unsigned int major, const char *name)EXPORT_SYMBOL (unregister_blkdev); +/* +* disk_name_format - format disk name +* @prefix: name prefix - ie. "sd" for SCSI disks +* @index: index of the disk to format name for +* @buf: output buffer +* @buflen: length of the output buffer +* +* Take SCSI disks for example. Disk names starts at sda. The +* 26th device is sdz and the 27th is sdaa. The last one for +* two lettered suffix is sdzz which is followed by sdaaa. +* +* This is basically 26 base counting with one extra 'nil' entry +* at the beginning from the second digit on and can be +* determined using similar method as 26 base conversion with the +* index shifted -1 after each digit is computed. +* +* CONTEXT: +* Don't care. +* +* RETURNS: +* 0 on success, -errno on failure. +*/ + +int disk_name_format (char *prefix,int index,char *buf,int buflen) +{ +const int base ='z' -'a' +1 ; +char *begin = buf +strlen (prefix); +char *end = buf + buflen; +char *p; +int unit; + + p = end -1 ; + *p ='\0' ; + unit = base; +do { +if (p == begin) +return -EINVAL ; + *--p ='a' + (index % unit); + index = (index / unit) -1 ; + }while (index >=0 ); + +memmove (begin, p, end - p); +memcpy (buf, prefix,strlen (prefix)); + +return 0 ; +} + +EXPORT_SYMBOL (disk_name_format); +static struct kobj_map *bdev_map;/**
include/linux/genhd.h +2 -0
@@ -407,6 +407,8 @@ static inline void free_part_info(struct hd_struct *part)extern void part_round_stats (int cpu,struct hd_struct *part);/* block/genhd.c */ +extern int disk_name_format (char *prefix,int index,char *buf,int buflen); +extern void add_disk (struct gendisk *disk);extern void del_gendisk (struct gendisk *gp);extern struct gendisk *get_gendisk (dev_t dev,int *partno);
PATCH 2/4 scsi: replace sd_format_disk_name() to disk_name_format()
In reply to Ren Mingxin Since "sd_format_disk_name()"has been copied into block
core as "disk_name_format()", the original function should be removed, and the place used original function should be replaced by the renamed function.
--- sd.c | 48 +----------------------------------------------- 1 file changed, 1 insertion(+), 47 deletions(-)
drivers/scsi/sd.c +1 -47
@@ -2525,52 +2525,6 @@ static void sd_unlock_native_capacity(struct gendisk *disk) sdev->host->hostt->unlock_native_capacity (sdev); } -/** -* sd_format_disk_name - format disk name -* @prefix: name prefix - ie. "sd" for SCSI disks -* @index: index of the disk to format name for -* @buf: output buffer -* @buflen: length of the output buffer -* -* SCSI disk names starts at sda. The 26th device is sdz and the -* 27th is sdaa. The last one for two lettered suffix is sdzz -* which is followed by sdaaa. -* -* This is basically 26 base counting with one extra 'nil' entry -* at the beginning from the second digit on and can be -* determined using similar method as 26 base conversion with the -* index shifted -1 after each digit is computed. -* -* CONTEXT: -* Don't care. -* -* RETURNS: -* 0 on success, -errno on failure. -*/ -static int sd_format_disk_name (char *prefix,int index,char *buf,int buflen) -{ -const int base ='z' -'a' +1 ; -char *begin = buf +strlen (prefix); -char *end = buf + buflen; -char *p; -int unit; - - p = end -1 ; - *p ='\0' ; - unit = base; -do { -if (p == begin) -return -EINVAL ; - *--p ='a' + (index % unit); - index = (index / unit) -1 ; - }while (index >=0 ); - -memmove (begin, p, end - p); -memcpy (buf, prefix,strlen (prefix)); - -return 0 ; -} -/* * The asynchronous part of sd_probe */
@@ -2685,7 +2639,7 @@ static int sd_probe(struct device *dev) goto out_put; } - error =sd_format_disk_name ("sd" , index, gd->disk_name,DISK_NAME_LEN ); + error =disk_name_format ("sd" , index, gd->disk_name,DISK_NAME_LEN );if (error) { sdev_printk(KERN_WARNING, sdp, "SCSI disk (sd) name
length exceeded.\n");
goto out_free_index;PATCH 2/4 scsi: replace sd_format_disk_name() to disk_name_format()
In reply to Ren Mingxin Since "sd_format_disk_name()"has been copied into block
core as "disk_name_format()", the original function should be removed, and the place used original function should be replaced by the renamed function.
--- sd.c | 48 +----------------------------------------------- 1 file changed, 1 insertion(+), 47 deletions(-)
drivers/scsi/sd.c +1 -47
@@ -2525,52 +2525,6 @@ static void sd_unlock_native_capacity(struct gendisk *disk) sdev->host->hostt->unlock_native_capacity (sdev); } -/** -* sd_format_disk_name - format disk name -* @prefix: name prefix - ie. "sd" for SCSI disks -* @index: index of the disk to format name for -* @buf: output buffer -* @buflen: length of the output buffer -* -* SCSI disk names starts at sda. The 26th device is sdz and the -* 27th is sdaa. The last one for two lettered suffix is sdzz -* which is followed by sdaaa. -* -* This is basically 26 base counting with one extra 'nil' entry -* at the beginning from the second digit on and can be -* determined using similar method as 26 base conversion with the -* index shifted -1 after each digit is computed. -* -* CONTEXT: -* Don't care. -* -* RETURNS: -* 0 on success, -errno on failure. -*/ -static int sd_format_disk_name (char *prefix,int index,char *buf,int buflen) -{ -const int base ='z' -'a' +1 ; -char *begin = buf +strlen (prefix); -char *end = buf + buflen; -char *p; -int unit; - - p = end -1 ; - *p ='\0' ; - unit = base; -do { -if (p == begin) -return -EINVAL ; - *--p ='a' + (index % unit); - index = (index / unit) -1 ; - }while (index >=0 ); - -memmove (begin, p, end - p); -memcpy (buf, prefix,strlen (prefix)); - -return 0 ; -} -/* * The asynchronous part of sd_probe */
@@ -2685,7 +2639,7 @@ static int sd_probe(struct device *dev) goto out_put; } - error =sd_format_disk_name ("sd" , index, gd->disk_name,DISK_NAME_LEN ); + error =disk_name_format ("sd" , index, gd->disk_name,DISK_NAME_LEN );if (error) { sdev_printk(KERN_WARNING, sdp, "SCSI disk (sd) name
length exceeded.\n");
goto out_free_index;PATCH 3/4 block: replace rssd_disk_name_format() to disk_name_format()
In reply to Ren Mingxin Currently, block core has been supplied "disk_name_format()", so
we should remove duplicate function "rssd_disk_name_format()" and use the new function to format rssd disk names.
--- mtip32xx.c | 33 +-------------------------------- 1 file changed, 1 insertion(+), 32 deletions(-)
drivers/block/mtip32xx/mtip32xx.c +1 -32
@@ -2836,37 +2836,6 @@ static int mtip_hw_resume(struct driver_data *dd) }/* -* Helper function for reusing disk name -* upon hot insertion. -*/ -static int rssd_disk_name_format (char *prefix, -int index, -char *buf, -int buflen) -{ -const int base ='z' -'a' +1 ; -char *begin = buf +strlen (prefix); -char *end = buf + buflen; -char *p; -int unit; - - p = end -1 ; - *p ='\0' ; - unit = base; -do { -if (p == begin) -return -EINVAL ; - *--p ='a' + (index % unit); - index = (index / unit) -1 ; - }while (index >=0 ); - -memmove (begin, p, end - p); -memcpy (buf, prefix,strlen (prefix)); - -return 0 ; -} - -/* * Block layer IOCTL handler. * * @dev Pointer to the block_device structure.
@@ -3140,7 +3109,7 @@ static int mtip_block_initialize(struct driver_data *dd)if (rv) goto ida_get_error; - rv =rssd_disk_name_format ("rssd" , + rv =disk_name_format ("rssd" , index, dd->disk->disk_name,DISK_NAME_LEN );
Re: [PATCH 3/4] block: replace rssd_disk_name_format() to disk_name_format()
Quoted 8 lines · Ren Mingxin
Currently, block core has been supplied "
disk_name_format()", so
we should remove duplicate function "rssd_disk_name_format()" and use the new function to format rssd disk names.Signed-off-by Ren Mingxin <[email protected]>--- mtip32xx.c | 33 +-------------------------------- 1 file changed, 1 insertion(+), 32 deletions(-)
Looks fine.
Should the subject be "mtip32xx: ..." instead of "block: ..."? I understand "block:" as relating to block core. I am fairly new here. If "block:" can be used for block drivers too, that is fine too.
--
Regards,
Asai Thambi
Re: [PATCH 3/4] block: replace rssd_disk_name_format() to disk_name_format()
Quoted 11 lines · On 03/31/2012 07:54 AM, Asai Thambi Samymuthu Pattrayasamy (asamymuthupa)…
Quoted 8 lines · Ren Mingxin
Currently, block core has been supplied "
disk_name_format()", so
we should remove duplicate function "rssd_disk_name_format()" and use the new function to format rssd disk names.Signed-off-by Ren Mingxin<[email protected]>--- mtip32xx.c | 33 +-------------------------------- 1 file changed, 1 insertion(+), 32 deletions(-)
Quoted 7 lines
Looks fine.
Should the subject be "mtip32xx: ..." instead of "block: ..."? I understand "block:" as relating to block core. I am fairly new here. If "block:" can be used for block drivers too, that is fine too.
Good idea! Will be fixed in the next version.
Signature · 2 lines
Thanks, Ren
Re: [PATCH 3/4] block: replace rssd_disk_name_format() to disk_name_format()
In reply to Asai Thambi Samymuthu Pattrayasamy (asamymuthupa) [CONT - Type 2]Quoted 11 lines · On 03/31/2012 07:54 AM, Asai Thambi Samymuthu Pattrayasamy (asamymuthupa)…
Quoted 8 lines · Ren Mingxin
Currently, block core has been supplied "
disk_name_format()", so
we should remove duplicate function "rssd_disk_name_format()" and use the new function to format rssd disk names.Signed-off-by Ren Mingxin<[email protected]>--- mtip32xx.c | 33 +-------------------------------- 1 file changed, 1 insertion(+), 32 deletions(-)
Quoted 7 lines
Looks fine.
Should the subject be "mtip32xx: ..." instead of "block: ..."? I understand "block:" as relating to block core. I am fairly new here. If "block:" can be used for block drivers too, that is fine too.
Good idea! Will be fixed in the next version.
Signature · 2 lines
Thanks, Ren
Re: [PATCH 3/4] block: replace rssd_disk_name_format() to disk_name_format()
In reply to Ren MingxinQuoted 8 lines · Ren Mingxin
Currently, block core has been supplied "
disk_name_format()", so
we should remove duplicate function "rssd_disk_name_format()" and use the new function to format rssd disk names.Signed-off-by Ren Mingxin <[email protected]>--- mtip32xx.c | 33 +-------------------------------- 1 file changed, 1 insertion(+), 32 deletions(-)
Looks fine.
Should the subject be "mtip32xx: ..." instead of "block: ..."? I understand "block:" as relating to block core. I am fairly new here. If "block:" can be used for block drivers too, that is fine too.
--
Regards,
Asai Thambi
PATCH 3/4 block: replace rssd_disk_name_format() to disk_name_format()
In reply to Ren Mingxin Currently, block core has been supplied "disk_name_format()", so
we should remove duplicate function "rssd_disk_name_format()" and use the new function to format rssd disk names.
--- mtip32xx.c | 33 +-------------------------------- 1 file changed, 1 insertion(+), 32 deletions(-)
drivers/block/mtip32xx/mtip32xx.c +1 -32
@@ -2836,37 +2836,6 @@ static int mtip_hw_resume(struct driver_data *dd) }/* -* Helper function for reusing disk name -* upon hot insertion. -*/ -static int rssd_disk_name_format (char *prefix, -int index, -char *buf, -int buflen) -{ -const int base ='z' -'a' +1 ; -char *begin = buf +strlen (prefix); -char *end = buf + buflen; -char *p; -int unit; - - p = end -1 ; - *p ='\0' ; - unit = base; -do { -if (p == begin) -return -EINVAL ; - *--p ='a' + (index % unit); - index = (index / unit) -1 ; - }while (index >=0 ); - -memmove (begin, p, end - p); -memcpy (buf, prefix,strlen (prefix)); - -return 0 ; -} - -/* * Block layer IOCTL handler. * * @dev Pointer to the block_device structure.
@@ -3140,7 +3109,7 @@ static int mtip_block_initialize(struct driver_data *dd)if (rv) goto ida_get_error; - rv =rssd_disk_name_format ("rssd" , + rv =disk_name_format ("rssd" , index, dd->disk->disk_name,DISK_NAME_LEN );
PATCH 4/4 virtio_blk: use disk_name_format() to support mass of disks naming
In reply to Ren Mingxin The current virtblk's naming algorithm only supports 263 disks.
If there are mass of virtblks(exceeding 263), there will be disks with the same name.
By renaming "sd_format_disk_name()" to "disk_name_format()" and moving it into block core, virtio_blk can use this function to support mass of disks.
--- virtio_blk.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-)
drivers/block/virtio_blk.c +1 -12
@@ -442,18 +442,7 @@ static int __devinit virtblk_probe(struct virtio_device *vdev) q->queuedata = vblk; -if (index <26 ) { -sprintf (vblk->disk->disk_name,"vd%c" ,'a' + index %26 ); - }else if (index < (26 +1 ) *26 ) { -sprintf (vblk->disk->disk_name,"vd%c%c" , -'a' + index /26 -1 ,'a' + index %26 ); - }else { -const unsigned int m1 = (index /26 -1 ) /26 -1 ; -const unsigned int m2 = (index /26 -1 ) %26 ; -const unsigned int m3 = index %26 ; -sprintf (vblk->disk->disk_name,"vd%c%c%c" , -'a' + m1,'a' + m2,'a' + m3); - } +disk_name_format ("vd" , index, vblk->disk->disk_name,DISK_NAME_LEN ); vblk->disk->major = major; vblk->disk->first_minor =index_to_minor (index);
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Hi, Ren
Quoted 3 lines · Ren Mingxin
The current virtblk's naming algorithm only supports 263 disks. If there are mass of virtblks(exceeding 263), there will be disks with the same name.
This fix is pretty nice. However, current virtblk's naming still supports up to 18278 disks, no?
( index 0 -> vda, index 18277 -> vdzzz ).
Quoted 39 lines
By renaming "
sd_format_disk_name()" to "disk_name_format()" and moving it into block core,virtio_blkcan use this function to support mass of disks.Signed-off-by Ren Mingxin <[email protected]>---
virtio_blk.c | 13 +------------1 file changed, 1 insertion(+), 12 deletions(-)drivers/block/virtio_blk.c +1
-12@@ -442,18 +442,7 @@ static int __devinit virtblk_probe(struct virtio_device *vdev) q->queuedata = vblk; - if (index < 26) { - sprintf(vblk->disk->disk_name, "vd%c", 'a' + index % 26); - } else if (index < (26 + 1) * 26) { - sprintf(vblk->disk->disk_name, "vd%c%c", - 'a' + index / 26 - 1, 'a' + index % 26); - } else { - const unsigned int m1 = (index / 26 - 1) / 26 - 1; - const unsigned int m2 = (index / 26 - 1) % 26; - const unsigned int m3 = index % 26; - sprintf(vblk->disk->disk_name, "vd%c%c%c", - 'a' + m1, 'a' + m2, 'a' + m3); - } + disk_name_format("vd", index, vblk->disk->disk_name, DISK_NAME_LEN); vblk->disk->major = major;vblk->disk->first_minor =
index_to_minor(index);--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 5 lines
Asias He _______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Hi, He:
Quoted 7 lines · Asias He
Quoted 3 lines · Ren Mingxin
The current virtblk's naming algorithm only supports 263 disks.
If there are mass of virtblks(exceeding 263), there will be disks with the same name.This fix is pretty nice. However, current virtblk's naming still supports up to 18278 disks, no?
( index 0 -> vda, index 18277 -> vdzzz ).
Sorry, I intended to type 26^3+ (26^2+26)...
It may still be a restriction which should be improved though.
Thanks,
Ren
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Quoted 11 lines · Ren Mingxin
Hi, He:
Quoted 7 lines · Asias He
Quoted 4 lines · Ren Mingxin
The current virtblk's naming algorithm only supports 263 disks. If there are mass of virtblks(exceeding 263), there will be disks with the same name.
Quoted 7 lines
Quoted 3 lines
This fix is pretty nice. However, current virtblk's naming still supports up to 18278 disks, no?
( index 0 -> vda, index 18277 -> vdzzz ).Sorry, I intended to type 26^3+ (26^2+26)...
It may still be a restriction which should be improved though.
OK.
Signature · 5 lines
Asias He _______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
In reply to Asias HeHi, He:
Quoted 7 lines · Asias He
Quoted 3 lines · Ren Mingxin
The current virtblk's naming algorithm only supports 263 disks.
If there are mass of virtblks(exceeding 263), there will be disks with the same name.This fix is pretty nice. However, current virtblk's naming still supports up to 18278 disks, no?
( index 0 -> vda, index 18277 -> vdzzz ).
Sorry, I intended to type 26^3+ (26^2+26)...
It may still be a restriction which should be improved though.
Thanks,
Ren
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
In reply to Ren MingxinQuoted 9 lines · Ren Mingxin
The current virtblk's naming algorithm only supports 263 disks.
If there are mass of virtblks(exceeding 263), there will be disks with the same name.By renaming "
sd_format_disk_name()" to "disk_name_format()" and moving it into block core,virtio_blkcan use this function to support mass of disks.Signed-off-by Ren Mingxin <[email protected]>
I guess it's already way too late but why couldn't they have been named vdD-P where both D and P are integers denoting disk number and partition number? [sh]dX's were created when there weren't supposed to be too many disks, so we had to come up with the horrible alphabet based numbering scheme but vd is new enough. I mean, naming is one thing but who wants to figure out which sequence is or guess what comes next vdzz9? :(
If we're gonna move it to block layer, let's add big blinking red comment saying "don't ever use it for any new driver".
Thanks.
Signature · 1 line
tejun
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Quoted 19 lines · Tejun Heo
Quoted 9 lines · Ren Mingxin
The current virtblk's naming algorithm only supports 263 disks.
If there are mass of virtblks(exceeding 263), there will be disks with the same name.By renaming "
sd_format_disk_name()" to "disk_name_format()" and moving it into block core,virtio_blkcan use this function to support mass of disks.Signed-off-by Ren Mingxin <[email protected]>I guess it's already way too late but why couldn't they have been named vdD-P where both D and P are integers denoting disk number and partition number? [sh]dX's were created when there weren't supposed to be too many disks, so we had to come up with the horrible alphabet based numbering scheme but vd is new enough. I mean, naming is one thing but who wants to figure out which sequence is or guess what comes next vdzz9? :(
Quoted 2 lines
If we're gonna move it to block layer, let's add big blinking red comment saying "don't ever use it for any new driver".
And also let's make that clear in the function name - say, format_legacy_disk_name() or something.
Thanks.
Signature · 1 line
tejun
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Quoted 19 lines · Tejun Heo
Quoted 18 lines · Tejun Heo
Quoted 9 lines · Ren Mingxin
The current virtblk's naming algorithm only supports 263 disks.
If there are mass of virtblks(exceeding 263), there will be disks with the same name.By renaming "
sd_format_disk_name()" to "disk_name_format()" and moving it into block core,virtio_blkcan use this function to support mass of disks.Signed-off-by Ren Mingxin<[email protected]>I guess it's already way too late but why couldn't they have been named vdD-P where both D and P are integers denoting disk number and partition number? [sh]dX's were created when there weren't supposed to be too many disks, so we had to come up with the horrible alphabet based numbering scheme but vd is new enough. I mean, naming is one thing but who wants to figure out which sequence is or guess what comes next vdzz9? :(
Quoted 4 lines
Quoted 2 lines
If we're gonna move it to block layer, let's add big blinking red comment saying "don't ever use it for any new driver".
And also let's make that clear in the function name - say,
format_legacy_disk_name()or something.
So, to legacy disks [sh]d, we'd name them as [sh]d[a-z]{1,}. To new devices like vd, we'd name them as vd<index>(vd<index>p<partno> as partitions)? And how about the rssd in the patch 3 then?
Besides, does anybody have comments?
Looking forward to your replies ;-)
Signature · 2 lines
Thanks, Ren
Michael S. Tsirkin
maintainerRe: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Quoted 20 lines · Ren Mingxin
Quoted 19 lines · Tejun Heo
Quoted 18 lines · Tejun Heo
Quoted 9 lines · Ren Mingxin
The current virtblk's naming algorithm only supports 263 disks.
If there are mass of virtblks(exceeding 263), there will be disks with the same name.By renaming "
sd_format_disk_name()" to "disk_name_format()" and moving it into block core,virtio_blkcan use this function to support mass of disks.Signed-off-by Ren Mingxin<[email protected]>I guess it's already way too late but why couldn't they have been named vdD-P where both D and P are integers denoting disk number and partition number? [sh]dX's were created when there weren't supposed to be too many disks, so we had to come up with the horrible alphabet based numbering scheme but vd is new enough. I mean, naming is one thing but who wants to figure out which sequence is or guess what comes next vdzz9? :(
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
If we're gonna move it to block layer, let's add big blinking red comment saying "don't ever use it for any new driver".
And also let's make that clear in the function name - say,
format_legacy_disk_name()or something.So, to legacy disks [sh]d, we'd name them as [sh]d[a-z]{1,}. To new devices like vd, we'd name them as vd<index>(vd<index>p<partno> as partitions)?
Pleae don't rename virtio disks, it is way too late for that:
virtio block driver was merged around 2007, it is not new by any measure, and there are many systems out there using the current naming scheme.
Quoted 1 line
And how about the rssd in the patch 3 then?
Probably same. Renaming existing devices will break setups. I think the idea is to avoid using the
legacy naming in new drivers *that will be added from now on*.
Quoted 6 lines
Besides, does anybody have comments?
Looking forward to your replies ;-)Signature · 2 lines
Thanks, Ren
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Hello,
Quoted 4 lines · Michael S. Tsirkin
Pleae don't rename virtio disks, it is way too late for that:
virtio block driver was merged around 2007, it is not new by any measure, and there are many systems out there using the current naming scheme.
There's no point in renaming device nodes of already deployed drivers. It'll cause a lot of pain.
Quoted 5 lines
Quoted 1 line
And how about the rssd in the patch 3 then?
Probably same. Renaming existing devices will break setups. I think the idea is to avoid using the
legacy naming in new drivers *that will be added from now on*.
Yeap.
Thanks.
Signature · 1 line
tejun
James Bottomley
maintainerRe: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Quoted 5 lines · Tejun Heo
Quoted 3 lines
Probably same. Renaming existing devices will break setups. I think the idea is to avoid using the
legacy naming in new drivers *that will be added from now on*.Yeap.
So if we're agreed no other devices going forwards should ever use this interface, is there any point unifying the interface? No matter how many caveats you hedge it round with, putting the API in a central place
will be a bit like a honey trap for careless bears. It might be safer
just to leave it buried in the three current drivers.
James
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Hello, James.
Quoted 5 lines · James Bottomley
So if we're agreed no other devices going forwards should ever use this interface, is there any point unifying the interface? No matter how many caveats you hedge it round with, putting the API in a central place
will be a bit like a honey trap for careless bears. It might be safer
just to leave it buried in the three current drivers.
Yeah, that was my hope but I think it would be easier to enforce to have a common function which is clearly marked legacy so that new driver writers can go look for the naming code in the existing ones, find out they're all using the same function which is marked legacy and explains what to do for newer drivers.
Thanks.
Signature · 1 line
tejun
Michael S. Tsirkin
maintainerRe: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Quoted 9 lines · Tejun Heo
Hello, James.
Quoted 5 lines · James Bottomley
So if we're agreed no other devices going forwards should ever use this interface, is there any point unifying the interface? No matter how many caveats you hedge it round with, putting the API in a central place
will be a bit like a honey trap for careless bears. It might be safer
just to leave it buried in the three current drivers.
Quoted 7 lines
Yeah, that was my hope but I think it would be easier to enforce to have a common function which is clearly marked legacy so that new driver writers can go look for the naming code in the existing ones, find out they're all using the same function which is marked legacy and explains what to do for newer drivers.
Thanks.
I think I'm not the only one to be confused about the preferred direction here.
James, do you agree to the approach above?
It would be nice to fix virtio block for 3.4, so how about this:
- I'll just apply the original bugfix patch for 3.4 - it only affects virtio
- Ren will repost the refactoring patch on top, and we can keep up the discussion
Ren if you agree, can you make this a two patch series please?
Quoted 6 lines
Signature · 5 lines
tejun _______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Quoted 12 lines · Michael S. Tsirkin
Quoted 11 lines · Tejun Heo
Quoted 5 lines · James Bottomley
So if we're agreed no other devices going forwards should ever use this interface, is there any point unifying the interface? No matter how many caveats you hedge it round with, putting the API in a central place
will be a bit like a honey trap for careless bears. It might be safer
just to leave it buried in the three current drivers.Yeah, that was my hope but I think it would be easier to enforce to have a common function which is clearly marked legacy so that new driver writers can go look for the naming code in the existing ones, find out they're all using the same function which is marked legacy and explains what to do for newer drivers.
Quoted 8 lines
I think I'm not the only one to be confused about the preferred direction here.
James, do you agree to the approach above?It would be nice to fix virtio block for 3.4, so how about this:
- I'll just apply the original bugfix patch for 3.4 - it only affects virtio
Sorry, about only affects virtio, I'm not very clear here:
1) Just duplicate the disk name format function in virtio_blk
like the original patch: https://lkml.org/lkml/2012/3/28/45
2) Move the disk name format function into block core like this patch series but only affects virtio(not affect mtip32xx).
Do you mean the 2) one or something else?
Quoted 5 lines
- Ren will repost the refactoring patch on top, and we can keep up the discussion
Ren if you agree, can you make this a two patch series please?
Sure.
Signature · 2 lines
Thanks, Ren
Michael S. Tsirkin
maintainerRe: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Quoted 22 lines · Ren Mingxin
Quoted 12 lines · Michael S. Tsirkin
Quoted 11 lines · Tejun Heo
Quoted 5 lines · James Bottomley
So if we're agreed no other devices going forwards should ever use this interface, is there any point unifying the interface? No matter how many caveats you hedge it round with, putting the API in a central place
will be a bit like a honey trap for careless bears. It might be safer
just to leave it buried in the three current drivers.Yeah, that was my hope but I think it would be easier to enforce to have a common function which is clearly marked legacy so that new driver writers can go look for the naming code in the existing ones, find out they're all using the same function which is marked legacy and explains what to do for newer drivers.
Quoted 8 lines
I think I'm not the only one to be confused about the preferred direction here.
James, do you agree to the approach above?It would be nice to fix virtio block for 3.4, so how about this:
- I'll just apply the original bugfix patch for 3.4 - it only affects virtio
Quoted 6 lines
Sorry, about only affects virtio, I'm not very clear here:
1) Just duplicate the disk name format function invirtio_blklike the original patch: https://lkml.org/lkml/2012/3/28/452) Move the disk name format function into block core like this patch series but only affects virtio(not affect mtip32xx).
Do you mean the 2) one or something else?
I mean 1) - I'll apply the original patch.
Quoted 11 lines
Quoted 5 lines
- Ren will repost the refactoring patch on top, and we can keep up the discussion
Ren if you agree, can you make this a two patch series please?
Sure.
Signature · 2 lines
Thanks, Ren
Michael S. Tsirkin
maintainerRe: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
In reply to Ren MingxinQuoted 17 lines · Ren Mingxin
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Tejun Heo
Quoted 5 lines · James Bottomley
So if we're agreed no other devices going forwards should ever use this interface, is there any point unifying the interface? No matter how many caveats you hedge it round with, putting the API in a central place
will be a bit like a honey trap for careless bears. It might be safer
just to leave it buried in the three current drivers.Yeah, that was my hope but I think it would be easier to enforce to have a common function which is clearly marked legacy so that new driver writers can go look for the naming code in the existing ones, find out they're all using the same function which is marked legacy and explains what to do for newer drivers.
I think I'm not the only one to be confused about the preferred direction here.
James, do you agree to the approach above?
Quoted 8 lines
Quoted 4 lines
It would be nice to fix virtio block for 3.4, so how about this:
- I'll just apply the original bugfix patch for 3.4 - it only affects virtioSorry, about only affects virtio, I'm not very clear here:
1) Just duplicate the disk name format function invirtio_blklike the original patch: https://lkml.org/lkml/2012/3/28/45
So I'd like to apply this, and we can discuss the deduplication for 3.5. Please post a version of this that
1. isn't line-wrapped and doesn't have damaged whitespace so I can run git am on it
2. lists the # of duspported disks correctly as 26^3+(26^2+26) in the description
Thanks!
Quoted 15 lines
2) Move the disk name format function into block core like this patch series but only affects virtio(not affect mtip32xx).
Do you mean the 2) one or something else?Quoted 5 lines
- Ren will repost the refactoring patch on top, and we can keep up the discussion
Ren if you agree, can you make this a two patch series please?
Sure.
Signature · 2 lines
Thanks, Ren
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
In reply to Michael S. TsirkinQuoted 12 lines · Michael S. Tsirkin
Quoted 11 lines · Tejun Heo
Quoted 5 lines · James Bottomley
So if we're agreed no other devices going forwards should ever use this interface, is there any point unifying the interface? No matter how many caveats you hedge it round with, putting the API in a central place
will be a bit like a honey trap for careless bears. It might be safer
just to leave it buried in the three current drivers.Yeah, that was my hope but I think it would be easier to enforce to have a common function which is clearly marked legacy so that new driver writers can go look for the naming code in the existing ones, find out they're all using the same function which is marked legacy and explains what to do for newer drivers.
Quoted 8 lines
I think I'm not the only one to be confused about the preferred direction here.
James, do you agree to the approach above?It would be nice to fix virtio block for 3.4, so how about this:
- I'll just apply the original bugfix patch for 3.4 - it only affects virtio
Sorry, about only affects virtio, I'm not very clear here:
1) Just duplicate the disk name format function in virtio_blk
like the original patch: https://lkml.org/lkml/2012/3/28/45
2) Move the disk name format function into block core like this patch series but only affects virtio(not affect mtip32xx).
Do you mean the 2) one or something else?
Quoted 5 lines
- Ren will repost the refactoring patch on top, and we can keep up the discussion
Ren if you agree, can you make this a two patch series please?
Sure.
Signature · 2 lines
Thanks, Ren
James Bottomley
maintainerRe: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
In reply to Tejun HeoQuoted 5 lines · Tejun Heo
Quoted 3 lines
Probably same. Renaming existing devices will break setups. I think the idea is to avoid using the
legacy naming in new drivers *that will be added from now on*.Yeap.
So if we're agreed no other devices going forwards should ever use this interface, is there any point unifying the interface? No matter how many caveats you hedge it round with, putting the API in a central place
will be a bit like a honey trap for careless bears. It might be safer
just to leave it buried in the three current drivers.
James
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
In reply to Tejun HeoQuoted 19 lines · Tejun Heo
Quoted 18 lines · Tejun Heo
Quoted 9 lines · Ren Mingxin
The current virtblk's naming algorithm only supports 263 disks.
If there are mass of virtblks(exceeding 263), there will be disks with the same name.By renaming "
sd_format_disk_name()" to "disk_name_format()" and moving it into block core,virtio_blkcan use this function to support mass of disks.Signed-off-by Ren Mingxin<[email protected]>I guess it's already way too late but why couldn't they have been named vdD-P where both D and P are integers denoting disk number and partition number? [sh]dX's were created when there weren't supposed to be too many disks, so we had to come up with the horrible alphabet based numbering scheme but vd is new enough. I mean, naming is one thing but who wants to figure out which sequence is or guess what comes next vdzz9? :(
Quoted 4 lines
Quoted 2 lines
If we're gonna move it to block layer, let's add big blinking red comment saying "don't ever use it for any new driver".
And also let's make that clear in the function name - say,
format_legacy_disk_name()or something.
So, to legacy disks [sh]d, we'd name them as [sh]d[a-z]{1,}. To new devices like vd, we'd name them as vd<index>(vd<index>p<partno> as partitions)? And how about the rssd in the patch 3 then?
Besides, does anybody have comments?
Looking forward to your replies ;-)
Signature · 2 lines
Thanks, Ren
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
In reply to Tejun HeoQuoted 5 lines · Tejun Heo
Quoted 4 lines · Ren Mingxin
The current virtblk's naming algorithm only supports 263 disks.
If there are mass of virtblks(exceeding 263), there will be disks with the same name.
Quoted 8 lines
Quoted 5 lines
By renaming "
sd_format_disk_name()" to "disk_name_format()" and moving it into block core,virtio_blkcan use this function to support mass of disks.Signed-off-by Ren Mingxin <[email protected]>I guess it's already way too late but why couldn't they have been named vdD-P where both D and P are integers denoting disk number and
So, if a device name ends in digit the partition code adds delimiter 'p' automatically and this is already in use by md. So, partitioned md devices are named mdDpP where D and P are base 10 number indicating the sequence like md12p4. So, let's please add comment that new drivers should name their devices PREFIX%d where the sequence number can be allocated by ida.
Thanks.
Signature · 1 line
tejun
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Quoted 11 lines · Tejun Heo
Quoted 10 lines · Tejun Heo
Quoted 9 lines · Ren Mingxin
The current virtblk's naming algorithm only supports 263 disks.
If there are mass of virtblks(exceeding 263), there will be disks with the same name.By renaming "
sd_format_disk_name()" to "disk_name_format()" and moving it into block core,virtio_blkcan use this function to support mass of disks.Signed-off-by Ren Mingxin<[email protected]>
Quoted 10 lines
Quoted 2 lines
I guess it's already way too late but why couldn't they have been named vdD-P where both D and P are integers denoting disk number and
So, if a device name ends in digit the partition code adds delimiter 'p' automatically and this is already in use by md. So, partitioned md devices are named mdDpP where D and P are base 10 number indicating the sequence like md12p4. So, let's please add comment that new drivers should name their devices PREFIX%d where the sequence number can be allocated by ida.
Oh, got it. Just like md devices' names.
Signature · 2 lines
Thanks, Ren
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
In reply to Tejun HeoQuoted 11 lines · Tejun Heo
Quoted 10 lines · Tejun Heo
Quoted 9 lines · Ren Mingxin
The current virtblk's naming algorithm only supports 263 disks.
If there are mass of virtblks(exceeding 263), there will be disks with the same name.By renaming "
sd_format_disk_name()" to "disk_name_format()" and moving it into block core,virtio_blkcan use this function to support mass of disks.Signed-off-by Ren Mingxin<[email protected]>
Quoted 10 lines
Quoted 2 lines
I guess it's already way too late but why couldn't they have been named vdD-P where both D and P are integers denoting disk number and
So, if a device name ends in digit the partition code adds delimiter 'p' automatically and this is already in use by md. So, partitioned md devices are named mdDpP where D and P are base 10 number indicating the sequence like md12p4. So, let's please add comment that new drivers should name their devices PREFIX%d where the sequence number can be allocated by ida.
Oh, got it. Just like md devices' names.
Signature · 2 lines
Thanks, Ren
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
In reply to Tejun HeoQuoted 5 lines · Tejun Heo
Quoted 4 lines · Ren Mingxin
The current virtblk's naming algorithm only supports 263 disks.
If there are mass of virtblks(exceeding 263), there will be disks with the same name.
Quoted 8 lines
Quoted 5 lines
By renaming "
sd_format_disk_name()" to "disk_name_format()" and moving it into block core,virtio_blkcan use this function to support mass of disks.Signed-off-by Ren Mingxin <[email protected]>I guess it's already way too late but why couldn't they have been named vdD-P where both D and P are integers denoting disk number and
So, if a device name ends in digit the partition code adds delimiter 'p' automatically and this is already in use by md. So, partitioned md devices are named mdDpP where D and P are base 10 number indicating the sequence like md12p4. So, let's please add comment that new drivers should name their devices PREFIX%d where the sequence number can be allocated by ida.
Thanks.
Signature · 1 line
tejun
Michael S. Tsirkin
maintainerRe: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
In reply to Tejun HeoQuoted 9 lines · Tejun Heo
Quoted 8 lines · Ren Mingxin
The current virtblk's naming algorithm only supports 263 disks.
If there are mass of virtblks(exceeding 263), there will be disks with the same name.By renaming "
sd_format_disk_name()" to "disk_name_format()" and moving it into block core,virtio_blkcan use this function to support mass of disks.
Quoted 5 lines
Quoted 1 line
Signed-off-by Ren Mingxin <[email protected]>I guess it's already way too late but why couldn't they have been named vdD-P where both D and P are integers denoting disk number and partition number?
Yes they could and yes it's way too late to change device names for virtio - if we did this will break countless setups.
Quoted 10 lines
[sh]dX's were created when there weren't supposed
to be too many disks, so we had to come up with the horrible alphabet based numbering scheme but vd is new enough. I mean, naming is one thing but who wants to figure out which sequence is or guess what comes next vdzz9? :(If we're gonna move it to block layer, let's add big blinking red comment saying "don't ever use it for any new driver".
Thanks.
Right. virtio would have to use the legacy one though.
Quoted 2 lines
Signature · 1 line
tejun
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
In reply to Tejun HeoQuoted 2 lines · Tejun Heo
If we're gonna move it to block layer, let's add big blinking red comment saying "don't ever use it for any new driver".
Big ACK to that...
PATCH 4/4 virtio_blk: use disk_name_format() to support mass of disks naming
In reply to Ren Mingxin The current virtblk's naming algorithm only supports 263 disks.
If there are mass of virtblks(exceeding 263), there will be disks with the same name.
By renaming "sd_format_disk_name()" to "disk_name_format()" and moving it into block core, virtio_blk can use this function to support mass of disks.
--- virtio_blk.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-)
drivers/block/virtio_blk.c +1 -12
@@ -442,18 +442,7 @@ static int __devinit virtblk_probe(struct virtio_device *vdev) q->queuedata = vblk; -if (index <26 ) { -sprintf (vblk->disk->disk_name,"vd%c" ,'a' + index %26 ); - }else if (index < (26 +1 ) *26 ) { -sprintf (vblk->disk->disk_name,"vd%c%c" , -'a' + index /26 -1 ,'a' + index %26 ); - }else { -const unsigned int m1 = (index /26 -1 ) /26 -1 ; -const unsigned int m2 = (index /26 -1 ) %26 ; -const unsigned int m3 = index %26 ; -sprintf (vblk->disk->disk_name,"vd%c%c%c" , -'a' + m1,'a' + m2,'a' + m3); - } +disk_name_format ("vd" , index, vblk->disk->disk_name,DISK_NAME_LEN ); vblk->disk->major = major; vblk->disk->first_minor =index_to_minor (index);
James Bottomley
maintainerQuoted 9 lines · Ren Mingxin
This patch series renames "
sd_format_disk_name()" to "disk_name_format()" and moves it into block core. So that who needs formatting disk name can use it, instead of duplicating these similar help functions.Ren Mingxin (4):
block: add functiondisk_name_format()into block core
scsi: replacesd_format_disk_name()todisk_name_format()
These two should be a single patch so it makes it obvious to anyone looking through the change sets that the code is merely being moved.
Other than that, everything looks fine to me.
James
Hi, James:
Quoted 11 lines · James Bottomley
Quoted 8 lines · Ren Mingxin
This patch series renames "
sd_format_disk_name()" to "disk_name_format()" and moves it into block core. So that who needs formatting disk name can use it, instead of duplicating these similar help functions.Ren Mingxin (4):
block: add functiondisk_name_format()into block core
scsi: replacesd_format_disk_name()todisk_name_format()These two should be a single patch so it makes it obvious to anyone looking through the change sets that the code is merely being moved.
Thank you very much for so quick checking. I'll merge these two patches into one in the next version.
Quoted 3 lines
Other than that, everything looks fine to me.
Thanks,
Ren
Hi, James:
Quoted 11 lines · James Bottomley
Quoted 8 lines · Ren Mingxin
This patch series renames "
sd_format_disk_name()" to "disk_name_format()" and moves it into block core. So that who needs formatting disk name can use it, instead of duplicating these similar help functions.Ren Mingxin (4):
block: add functiondisk_name_format()into block core
scsi: replacesd_format_disk_name()todisk_name_format()These two should be a single patch so it makes it obvious to anyone looking through the change sets that the code is merely being moved.
Thank you very much for so quick checking. I'll merge these two patches into one in the next version.
Quoted 3 lines
Other than that, everything looks fine to me.
Thanks,
Ren
James Bottomley
maintainerQuoted 9 lines · Ren Mingxin
This patch series renames "
sd_format_disk_name()" to "disk_name_format()" and moves it into block core. So that who needs formatting disk name can use it, instead of duplicating these similar help functions.Ren Mingxin (4):
block: add functiondisk_name_format()into block core
scsi: replacesd_format_disk_name()todisk_name_format()
These two should be a single patch so it makes it obvious to anyone looking through the change sets that the code is merely being moved.
Other than that, everything looks fine to me.
James
Michael S. Tsirkin
maintainerQuoted 6 lines · Ren Mingxin
This patch series renames "
sd_format_disk_name()" to "disk_name_format()" and moves it into block core. So that who needs formatting disk name can use it, instead of duplicating these similar help functions.
I see there are some responses about naming and comments, so that would need to be fixed.
Besides this, Jens, would you like to take the patchset through your tree or prefer for me to merge it through virtio tree? Block tree seems more appropriate, right?
Also this is a bugfix technically since it fixes setups with a ton of disks, so 3.4 material?
Quoted 18 lines
Ren Mingxin (4):
block: add functiondisk_name_format()into block core
scsi: replacesd_format_disk_name()todisk_name_format()
block: replacerssd_disk_name_format()todisk_name_format()
virtio_blk: usedisk_name_format()to support mass of disks namingblock/genhd.c | 49 ++++++++++++++++++++++++++++++++++++++ drivers/block/mtip32xx/mtip32xx.c | 33 ------------------------- drivers/block/virtio_blk.c | 13 ---------- drivers/scsi/sd.c | 48 ------------------------------------- include/linux/genhd.h | 2 + 5 files changed, 54 insertions(+), 91 deletions(-)Any comment will be appreciated.
Thanks,
Ren