virtio_blk: add helper function to support mass of disks naming

PATCH In review Ren Mingxin 4 participants 9 messages Mar 28, 2012 – Mar 29, 2012

Subsystems (5)

SCSI KVM Block layer virtio core +1 more subsystems

lore

Share & tools

lore

Ren Mingxin

author

2012-03-28 06:54 UTC #0 lore raw

PATCH 1/1 virtio_blk: add helper function to support mass of disks naming

Hi,

The current virtblk's naming algorithm just supports 26^3 disks. If there are mass of virtblks(exceeding 26^3), there will be disks with the same name.

According to "sd_format_disk_name()", I add function "virtblk_name_format()" for virtblk to support mass of disks.

Signed-off-by Ren Mingxin <[email protected]>

---
 virtio_blk.c |   37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

drivers/block/virtio_blk.c +25 -0

index c4a60ba..07b8bf9 100644
@@ -374,6 +374,30 @@ static int init_vq(struct virtio_blk *vblk)
        return err;
 }
 
+static int virtblk_name_format(char *prefix, int index, char *buf, int buflen)
+{
+       const int base = 'z' - 'a' + 1;
+       char *begin = buf + strlen(prefix);
+       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;
+}
+
 static int __devinit virtblk_probe(struct virtio_device *vdev)
 {

        struct virtio_blk *vblk;

@@ -442,18 +466,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);
- }
+ virtblk_name_format("vd", index, vblk->disk->disk_name, DISK_NAME_LEN);

        vblk->disk->major = major;
        vblk->disk->first_minor = index_to_minor(index);

Michael S. Tsirkin

maintainer

2012-03-28 10:58 UTC #1 lore raw

Quoted 11 lines · Ren Mingxin

Hi,
The current virtblk's naming algorithm just supports 26^3 disks. If there are mass of virtblks(exceeding 26^3), there will be disks with the same name.
According to "sd_format_disk_name()", I add function "virtblk_name_format()" for virtblk to support mass of disks.
Signed-off-by Ren Mingxin <[email protected]>

Nod. This is basically what 3e1a7ff8a0a7b948f2684930166954f9e8e776fe did. Except, maybe we should move this function into block core instead of duplicating it? Where would be a good place to put it? Jens, care to comment?

Quoted 63 lines

---
 virtio_blk.c |   37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

drivers/block/virtio_blk.c +26 ~~-12~~

index c4a60ba..07b8bf9 100644
@@ -374,6 +374,30 @@ static int init_vq(struct virtio_blk *vblk)
        return err;
 }
 
+static int virtblk_name_format(char *prefix, int index, char *buf,
 int buflen)
+{
+       const int base = 'z' - 'a' + 1;
+       char *begin = buf + strlen(prefix);
+       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;
+}
+
 static int __devinit virtblk_probe(struct virtio_device *vdev)
 {
        struct virtio_blk *vblk;

@@ -442,18 +466,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);
-       }
+       virtblk_name_format("vd", index, vblk->disk->disk_name,
 DISK_NAME_LEN);

        vblk->disk->major = major;
        vblk->disk->first_minor = index_to_minor(index);

Rusty Russell

2012-03-29 04:40 UTC #2 lore raw

Quoted 11 lines · Michael S. Tsirkin

Quoted 10 lines · Ren Mingxin
Hi,
The current virtblk's naming algorithm just supports 26^3 disks. If there are mass of virtblks(exceeding 26^3), there will be disks with the same name.
According to "sd_format_disk_name()", I add function "virtblk_name_format()" for virtblk to support mass of disks.

Quoted 6 lines

Quoted 1 line
Signed-off-by Ren Mingxin <[email protected]>
Nod. This is basically what 3e1a7ff8a0a7b948f2684930166954f9e8e776fe did. Except, maybe we should move this function into block core instead of duplicating it? Where would be a good place to put it? Jens, care to comment?

Indeed.

Cheers,
Rusty.

Signature · 1 line

  How could I marry someone with more hair than me?  http://baldalex.org

Ren Mingxin

author

2012-03-29 05:36 UTC #3 lore raw

In reply to Michael S. Tsirkin

Quoted 7 lines · Michael S. Tsirkin

Quoted 6 lines · Ren Mingxin
Hi,
The current virtblk's naming algorithm just supports 26^3 disks. If there are mass of virtblks(exceeding 26^3), there will be disks with the same name.

Quoted 8 lines

Quoted 5 lines
According to "sd_format_disk_name()", I add function "virtblk_name_format()" for virtblk to support mass of disks.
Signed-off-by Ren Mingxin<[email protected]>
Nod. This is basically what 3e1a7ff8a0a7b948f2684930166954f9e8e776fe did. Except, maybe we should move this function into block core instead of duplicating it? Where would be a good place to put it?

Yes, this was also what I thought.

How about placing the "sd_format_disk_name()"
as "disk_name_format()" into "block/genhd.c"
("include/linux/genhd.h")?

Thanks.
Ren

Jens Axboe

maintainer

2012-03-29 12:03 UTC #4 lore raw

Quoted 17 lines · Ren Mingxin

Quoted 7 lines · Michael S. Tsirkin
Quoted 6 lines · Ren Mingxin
Hi,
The current virtblk's naming algorithm just supports 26^3 disks. If there are mass of virtblks(exceeding 26^3), there will be disks with the same name.
Quoted 8 lines
Quoted 5 lines
According to "sd_format_disk_name()", I add function "virtblk_name_format()" for virtblk to support mass of disks.
Signed-off-by Ren Mingxin<[email protected]>
Nod. This is basically what 3e1a7ff8a0a7b948f2684930166954f9e8e776fe did. Except, maybe we should move this function into block core instead of duplicating it? Where would be a good place to put it?

Quoted 5 lines

Yes, this was also what I thought.
How about placing the "sd_format_disk_name()"
as "disk_name_format()" into "block/genhd.c"
("include/linux/genhd.h")?

Yes, that sounds like the appropriate solution (and name, and where to put it).

Signature · 1 line

Jens Axboe

Michael S. Tsirkin

maintainer

2012-03-29 12:46 UTC #5 lore raw

Quoted 18 lines · Jens Axboe

Quoted 17 lines · Ren Mingxin
Quoted 15 lines · Michael S. Tsirkin
Quoted 11 lines · Ren Mingxin
Hi,
The current virtblk's naming algorithm just supports 26^3 disks. If there are mass of virtblks(exceeding 26^3), there will be disks with the same name.
According to "sd_format_disk_name()", I add function "virtblk_name_format()" for virtblk to support mass of disks.
Signed-off-by Ren Mingxin<[email protected]>
Nod. This is basically what 3e1a7ff8a0a7b948f2684930166954f9e8e776fe did. Except, maybe we should move this function into block core instead of duplicating it? Where would be a good place to put it?

Quoted 8 lines

Quoted 5 lines
Yes, this was also what I thought.
How about placing the "sd_format_disk_name()"
as "disk_name_format()" into "block/genhd.c"
("include/linux/genhd.h")?
Yes, that sounds like the appropriate solution (and name, and where to put it).

OK, once this is posted I'll get your ack and
queue for 3.4 with other virtio fixups.

Quoted 2 lines

Signature · 1 line
Jens Axboe

Michael S. Tsirkin

maintainer

2012-03-29 12:46 UTC #6 lore raw

In reply to Jens Axboe

Quoted 18 lines · Jens Axboe

Quoted 17 lines · Ren Mingxin
Quoted 15 lines · Michael S. Tsirkin
Quoted 11 lines · Ren Mingxin
Hi,
The current virtblk's naming algorithm just supports 26^3 disks. If there are mass of virtblks(exceeding 26^3), there will be disks with the same name.
According to "sd_format_disk_name()", I add function "virtblk_name_format()" for virtblk to support mass of disks.
Signed-off-by Ren Mingxin<[email protected]>
Nod. This is basically what 3e1a7ff8a0a7b948f2684930166954f9e8e776fe did. Except, maybe we should move this function into block core instead of duplicating it? Where would be a good place to put it?

Quoted 8 lines

Quoted 5 lines
Yes, this was also what I thought.
How about placing the "sd_format_disk_name()"
as "disk_name_format()" into "block/genhd.c"
("include/linux/genhd.h")?
Yes, that sounds like the appropriate solution (and name, and where to put it).

OK, once this is posted I'll get your ack and
queue for 3.4 with other virtio fixups.

Quoted 2 lines

Signature · 1 line
Jens Axboe

Jens Axboe

maintainer

2012-03-29 12:03 UTC #7 lore raw

In reply to Ren Mingxin

Quoted 17 lines · Ren Mingxin

Quoted 7 lines · Michael S. Tsirkin
Quoted 6 lines · Ren Mingxin
Hi,
The current virtblk's naming algorithm just supports 26^3 disks. If there are mass of virtblks(exceeding 26^3), there will be disks with the same name.
Quoted 8 lines
Quoted 5 lines
According to "sd_format_disk_name()", I add function "virtblk_name_format()" for virtblk to support mass of disks.
Signed-off-by Ren Mingxin<[email protected]>
Nod. This is basically what 3e1a7ff8a0a7b948f2684930166954f9e8e776fe did. Except, maybe we should move this function into block core instead of duplicating it? Where would be a good place to put it?

Quoted 5 lines

Yes, this was also what I thought.
How about placing the "sd_format_disk_name()"
as "disk_name_format()" into "block/genhd.c"
("include/linux/genhd.h")?

Yes, that sounds like the appropriate solution (and name, and where to put it).

Signature · 1 line

Jens Axboe

Ren Mingxin

author

2012-03-29 05:36 UTC #8 lore raw

In reply to Michael S. Tsirkin

Quoted 7 lines · Michael S. Tsirkin

Quoted 6 lines · Ren Mingxin
Hi,
The current virtblk's naming algorithm just supports 26^3 disks. If there are mass of virtblks(exceeding 26^3), there will be disks with the same name.

Quoted 8 lines

Quoted 5 lines
According to "sd_format_disk_name()", I add function "virtblk_name_format()" for virtblk to support mass of disks.
Signed-off-by Ren Mingxin<[email protected]>
Nod. This is basically what 3e1a7ff8a0a7b948f2684930166954f9e8e776fe did. Except, maybe we should move this function into block core instead of duplicating it? Where would be a good place to put it?

Yes, this was also what I thought.

How about placing the "sd_format_disk_name()"
as "disk_name_format()" into "block/genhd.c"
("include/linux/genhd.h")?

Thanks.
Ren