Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
On 03/30/2012 11:26 AM, Tejun Heo wrote: 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... ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
On Mon, Apr 09, 2012 at 11:47:51AM +0800, Ren Mingxin wrote: On 04/04/2012 04:01 PM, Michael S. Tsirkin wrote: On Mon, Apr 02, 2012 at 12:00:45PM -0700, Tejun Heo wrote: On Mon, Apr 02, 2012 at 11:56:18AM -0700, James Bottomley wrote: 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? 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? I mean 1) - I'll apply the original patch. - 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. -- Thanks, Ren ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
On Mon, Apr 09, 2012 at 11:47:51AM +0800, Ren Mingxin wrote: On 04/04/2012 04:01 PM, Michael S. Tsirkin wrote: On Mon, Apr 02, 2012 at 12:00:45PM -0700, Tejun Heo wrote: On Mon, Apr 02, 2012 at 11:56:18AM -0700, James Bottomley wrote: 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? 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 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! 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? - 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. -- Thanks, Ren ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
On 04/04/2012 04:01 PM, Michael S. Tsirkin wrote: On Mon, Apr 02, 2012 at 12:00:45PM -0700, Tejun Heo wrote: On Mon, Apr 02, 2012 at 11:56:18AM -0700, James Bottomley wrote: 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? 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? - 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. -- Thanks, Ren ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
On Mon, Apr 02, 2012 at 12:00:45PM -0700, Tejun Heo wrote: Hello, James. On Mon, Apr 02, 2012 at 11:56:18AM -0700, James Bottomley wrote: 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. 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? -- tejun ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
On Fri, Mar 30, 2012 at 08:26:06AM -0700, Tejun Heo wrote: On Fri, Mar 30, 2012 at 05:53:52PM +0800, Ren Mingxin wrote: 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. Signed-off-by: Ren Mingxin re...@cn.fujitsu.com 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. [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. -- tejun ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
On Mon, Apr 02, 2012 at 09:19:05AM +0800, Ren Mingxin wrote:
On 03/30/2012 11:28 PM, Tejun Heo wrote:
On Fri, Mar 30, 2012 at 08:26:06AM -0700, Tejun Heo wrote:
On Fri, Mar 30, 2012 at 05:53:52PM +0800, Ren Mingxin wrote:
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.
Signed-off-by: Ren Mingxinre...@cn.fujitsu.com
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.
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 vdindex(vdindexppartno 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.
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*.
Besides, does anybody have comments?
Looking forward to your replies ;-)
--
Thanks,
Ren
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
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.
Signed-off-by: Ren Mingxin re...@cn.fujitsu.com
---
virtio_blk.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index c4a60ba..54612c2 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -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);
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
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: On 03/30/2012 07:22 PM, Asias He wrote: On Fri, Mar 30, 2012 at 5:53 PM, Ren Mingxinre...@cn.fujitsu.com wrote: 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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
On 03/30/2012 11:38 PM, Tejun Heo wrote: On Fri, Mar 30, 2012 at 08:26:06AM -0700, Tejun Heo wrote: On Fri, Mar 30, 2012 at 05:53:52PM +0800, Ren Mingxin wrote: 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. Signed-off-by: Ren Mingxinre...@cn.fujitsu.com 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. -- Thanks, Ren ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
On 03/30/2012 11:28 PM, Tejun Heo wrote:
On Fri, Mar 30, 2012 at 08:26:06AM -0700, Tejun Heo wrote:
On Fri, Mar 30, 2012 at 05:53:52PM +0800, Ren Mingxin wrote:
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.
Signed-off-by: Ren Mingxinre...@cn.fujitsu.com
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.
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 vdindex(vdindexppartno as partitions)?
And how about the rssd in the patch 3 then?
Besides, does anybody have comments?
Looking forward to your replies ;-)
--
Thanks,
Ren
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Hello, On Mon, Apr 02, 2012 at 10:20:09AM +0300, Michael S. Tsirkin wrote: 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. 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. -- tejun ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Hello, James. On Mon, Apr 02, 2012 at 11:56:18AM -0700, James Bottomley wrote: 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. -- tejun ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
On Mon, 2012-04-02 at 11:52 -0700, Tejun Heo wrote: 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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
Hi, Ren
On Fri, Mar 30, 2012 at 5:53 PM, Ren Mingxin re...@cn.fujitsu.com wrote:
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 ).
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.
Signed-off-by: Ren Mingxin re...@cn.fujitsu.com
---
virtio_blk.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index c4a60ba..54612c2 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Asias He
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
On Fri, Mar 30, 2012 at 05:53:52PM +0800, Ren Mingxin wrote: 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. Signed-off-by: Ren Mingxin re...@cn.fujitsu.com 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. -- tejun ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
On Fri, Mar 30, 2012 at 08:26:06AM -0700, Tejun Heo wrote: On Fri, Mar 30, 2012 at 05:53:52PM +0800, Ren Mingxin wrote: 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. Signed-off-by: Ren Mingxin re...@cn.fujitsu.com 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. And also let's make that clear in the function name - say, format_legacy_disk_name() or something. Thanks. -- tejun ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
On Fri, Mar 30, 2012 at 08:26:06AM -0700, Tejun Heo wrote: On Fri, Mar 30, 2012 at 05:53:52PM +0800, Ren Mingxin wrote: 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. Signed-off-by: Ren Mingxin re...@cn.fujitsu.com 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. -- tejun ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming
On Sat, Mar 31, 2012 at 9:14 AM, Ren Mingxin re...@cn.fujitsu.com wrote: Hi, He: On 03/30/2012 07:22 PM, Asias He wrote: On Fri, Mar 30, 2012 at 5:53 PM, Ren Mingxinre...@cn.fujitsu.com wrote: 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. OK. -- Asias He ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
