Skip to contentSkip to conversation

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
#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);
maintainer
#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);
#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
author
#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

maintainer
#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
maintainer
#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
maintainer
#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
maintainer
#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
author
#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