[Qemu-devel] [PATCH 0/3] Add virtio-blk support to persistent-storage rules

[Qemu-devel] [PATCH 0/3] Add virtio-blk support to persistent-storage rules

[Ryan Harper]

This patch series provides updates to udev to allow the creation symlinks for
virtio-blk devices, specifically disk/by-id and disk/by-path.  This is most
useful for virtio-blk devices that do not yet have any filesystem for which a
UUID can be extracted (disk/by-uuid).  These patches (save the path_id fix)
require an updated[1] qemu (on the host) and virtio-blk (in the guest)  to
generate the by-id path; however if the guest or host qemu isn't capable
then no action is taken.

1. http://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01869.html

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>

[PATCH 0/3] Add virtio-blk support to persistent-storage rules

[Ryan Harper]

This patch series provides updates to udev to allow the creation symlinks for
virtio-blk devices, specifically disk/by-id and disk/by-path.  This is most
useful for virtio-blk devices that do not yet have any filesystem for which a
UUID can be extracted (disk/by-uuid).  These patches (save the path_id fix)
require an updated[1] qemu (on the host) and virtio-blk (in the guest)  to
generate the by-id path; however if the guest or host qemu isn't capable
then no action is taken.

1. http://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01869.html

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>

[PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers

[Ryan Harper]

Use the 'VBID' virtio-blk ioctl to extract drive serial numbers
to be used for building disk/by-id symlinks.  After extracting
the serial number of the device it prints out the minimum info
needed in a similar format to `scsi_id --export` so that the
persistent-storage rules can process the serial information.

This program depends on the virtio-blk serial device patches posted
here[1] being applied to qemu and linux-kernel.

Here is what the output looks like:

% ./virtioblk_id /dev/vdb
ID_VIRTIO=1
ID_TYPE=disk
ID_VIRTIO_SERIAL=QM00001

1. http://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01869.html

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
---
 Makefile.am                        |    7 +++
 extras/virtioblk_id/virtioblk_id.c |   81 ++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 0 deletions(-)
 create mode 100644 extras/virtioblk_id/virtioblk_id.c

diff --git a/Makefile.am b/Makefile.am
index b0eeaf1..d2d3036 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -250,6 +250,13 @@ extras_path_id_path_id_LDADD = libudev/libudev-private.la
 libexec_PROGRAMS += extras/path_id/path_id
 
 # ------------------------------------------------------------------------------
+# virtioblk_id - virtioblk inquery to extract disk serial numbers
+# ------------------------------------------------------------------------------
+extras_virtioblk_id_virtioblk_id_SOURCES = extras/virtioblk_id/virtioblk_id.c
+extras_virtioblk_id_virtioblk_id_LDADD = libudev/libudev-private.la
+libexec_PROGRAMS += extras/virtioblk_id/virtioblk_id
+
+# ------------------------------------------------------------------------------
 # fstab_import - import /etc/fstab entry for block device
 # ------------------------------------------------------------------------------
 extras_fstab_import_fstab_import_SOURCES = extras/fstab_import/fstab_import.c
diff --git a/extras/virtioblk_id/virtioblk_id.c b/extras/virtioblk_id/virtioblk_id.c
new file mode 100644
index 0000000..81afd03
--- /dev/null
+++ b/extras/virtioblk_id/virtioblk_id.c
@@ -0,0 +1,81 @@
+/*
+ * extract disk serial from virtio-blk devices
+ * and print enough values to allow udev to create disk/by-id/ symlinks
+ *
+ * Copyright IBM, Corp. 2010
+ * 
+ * Authors:
+ *  John Cooper <john.cooper@redhat.com>
+ *  Ryan Harper <ryanh@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <stdio.h>
+#include <strings.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <linux/hdreg.h>
+#include <getopt.h>
+
+#define IOCTL_CMD "VBID"
+
+int main(int argc, char **argv)
+{
+    int fd, rv;
+    const char *device;
+    char buf[255];
+
+    int opt_debug = 0;
+    static const char *option_string = "d";
+    static struct option options[] = {
+        {"debug", 0, NULL, 'd'},
+        {"help", 0, NULL, 'h'},
+        {0, 0, NULL, 0}
+    };
+
+    while (1) {
+        int option;
+        option = getopt_long(argc, argv, "dh", options, NULL);
+        if (option = -1)
+            break;
+        switch (option) {
+        case 'd':
+            opt_debug = 1;
+            break;
+        case 'h':
+            printf("Usage: virtioblk_id [--debug] [--help] <device>\n"
+                   "  --debug           print debugging information\n"
+                   "  --help            print this help text\n\n");
+        default:
+            rv = 1;
+            goto exit;
+        }
+    }
+
+    device = argv[optind];
+    if (device = NULL) {
+        fprintf(stderr, "no device specified\n");
+        rv = 1;
+        goto exit;
+    }
+
+
+    bzero(buf, sizeof (buf));
+    if ((fd = open(device, O_RDONLY)) < 0) {
+        if (opt_debug = 1) 
+            perror("open");
+    } else if ((rv = ioctl(fd, IOCTL_CMD, buf)) < 0) {
+        if (opt_debug = 1) 
+            perror("ioctl");
+    } else {
+        printf("ID_VIRTIO=1\n");
+        printf("ID_TYPE=disk\n");
+        printf("ID_VIRTIO_SERIAL=%s\n", buf);
+        rv = 0;
+    }
+
+ exit:
+    return rv;
+}
-- 
1.6.3.3

Re: [PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers

[Kay Sievers]

On Thu, Jun 3, 2010 at 21:07, Ryan Harper <ryanh@us.ibm.com> wrote:
> Use the 'VBID' virtio-blk ioctl to extract drive serial numbers
> to be used for building disk/by-id symlinks.  After extracting
> the serial number of the device it prints out the minimum info
> needed in a similar format to `scsi_id --export` so that the
> persistent-storage rules can process the serial information.
>
> This program depends on the virtio-blk serial device patches posted
> here[1] being applied to qemu and linux-kernel.
>
> Here is what the output looks like:
>
> % ./virtioblk_id /dev/vdb
> ID_VIRTIO=1
> ID_TYPE=disk
> ID_VIRTIO_SERIAL=QM00001

Yikes! An ioctl to copy a plain string, and an entire binary to call
that ioctl and print it. If we don't have enough problems we make new
ones? :)

What's the reason to drop the ATA identify, that would work out-of-the
box without any of this stuff. It could also support WWN, which is
what people are looking for these days.

Confused,
Kay

Re: [PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers

[Ryan Harper]

* Kay Sievers <kay.sievers@vrfy.org> [2010-06-03 14:53]:
> On Thu, Jun 3, 2010 at 21:07, Ryan Harper <ryanh@us.ibm.com> wrote:
> > Use the 'VBID' virtio-blk ioctl to extract drive serial numbers
> > to be used for building disk/by-id symlinks.  After extracting
> > the serial number of the device it prints out the minimum info
> > needed in a similar format to `scsi_id --export` so that the
> > persistent-storage rules can process the serial information.
> >
> > This program depends on the virtio-blk serial device patches posted
> > here[1] being applied to qemu and linux-kernel.
> >
> > Here is what the output looks like:
> >
> > % ./virtioblk_id /dev/vdb
> > ID_VIRTIO=1
> > ID_TYPE=disk
> > ID_VIRTIO_SERIAL=QM00001
> 
> Yikes! An ioctl to copy a plain string, and an entire binary to call
> that ioctl and print it. If we don't have enough problems we make new
> ones? :)
> 
> What's the reason to drop the ATA identify, that would work out-of-the
> box without any of this stuff. It could also support WWN, which is
> what people are looking for these days.

http://www.mail-archive.com/qemu-devel@nongnu.org/msg24321.html

1. Virtio-blk isn't an ATA device
2. The ATA identify page is too large to fit into the virtio config
space

I'm not finding the older threads where this was discussed in detail.
I'll keep looking if the above isn't a sufficient explaination.


-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com

Re: [PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers

[Kay Sievers]

On Thu, Jun 3, 2010 at 22:01, Ryan Harper <ryanh@us.ibm.com> wrote:
> * Kay Sievers <kay.sievers@vrfy.org> [2010-06-03 14:53]:
>> On Thu, Jun 3, 2010 at 21:07, Ryan Harper <ryanh@us.ibm.com> wrote:
>> > Use the 'VBID' virtio-blk ioctl to extract drive serial numbers
>> > to be used for building disk/by-id symlinks.  After extracting
>> > the serial number of the device it prints out the minimum info
>> > needed in a similar format to `scsi_id --export` so that the
>> > persistent-storage rules can process the serial information.
>> >
>> > This program depends on the virtio-blk serial device patches posted
>> > here[1] being applied to qemu and linux-kernel.
>> >
>> > Here is what the output looks like:
>> >
>> > % ./virtioblk_id /dev/vdb
>> > ID_VIRTIO=1
>> > ID_TYPE=disk
>> > ID_VIRTIO_SERIAL=QM00001
>>
>> Yikes! An ioctl to copy a plain string, and an entire binary to call
>> that ioctl and print it. If we don't have enough problems we make new
>> ones? :)
>>
>> What's the reason to drop the ATA identify, that would work out-of-the
>> box without any of this stuff. It could also support WWN, which is
>> what people are looking for these days.
>
> http://www.mail-archive.com/qemu-devel@nongnu.org/msg24321.html
>
> 1. Virtio-blk isn't an ATA device
> 2. The ATA identify page is too large to fit into the virtio config
> space
>
> I'm not finding the older threads where this was discussed in detail.
> I'll keep looking if the above isn't a sufficient explaination.

Well, if ATA doesn't fit, then put that string in sysfs like mmc block
devices are doing it. It looks really awkward to require a new binary
and a new ioctl to get a single string out of something that was just
invented.

Thanks,
Kay

Re: [PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers

[David Zeuthen]

Hey,

On Thu, Jun 3, 2010 at 3:07 PM, Ryan Harper <ryanh@us.ibm.com> wrote:
> % ./virtioblk_id /dev/vdb
> ID_VIRTIO=1
> ID_TYPE=disk
> ID_VIRTIO_SERIAL=QM00001

Why ID_VIRTIO_SERIAL? Please use ID_SERIAL (and ID_SERIAL_SHORT) like
ata_id, scsi_id and usb_id already does.

     David

Re: [PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers

[Ryan Harper]

* Kay Sievers <kay.sievers@vrfy.org> [2010-06-03 15:06]:
> On Thu, Jun 3, 2010 at 22:01, Ryan Harper <ryanh@us.ibm.com> wrote:
> > * Kay Sievers <kay.sievers@vrfy.org> [2010-06-03 14:53]:
> >> On Thu, Jun 3, 2010 at 21:07, Ryan Harper <ryanh@us.ibm.com> wrote:
> >> > Use the 'VBID' virtio-blk ioctl to extract drive serial numbers
> >> > to be used for building disk/by-id symlinks.  After extracting
> >> > the serial number of the device it prints out the minimum info
> >> > needed in a similar format to `scsi_id --export` so that the
> >> > persistent-storage rules can process the serial information.
> >> >
> >> > This program depends on the virtio-blk serial device patches posted
> >> > here[1] being applied to qemu and linux-kernel.
> >> >
> >> > Here is what the output looks like:
> >> >
> >> > % ./virtioblk_id /dev/vdb
> >> > ID_VIRTIO=1
> >> > ID_TYPE=disk
> >> > ID_VIRTIO_SERIAL=QM00001
> >>
> >> Yikes! An ioctl to copy a plain string, and an entire binary to call
> >> that ioctl and print it. If we don't have enough problems we make new
> >> ones? :)
> >>
> >> What's the reason to drop the ATA identify, that would work out-of-the
> >> box without any of this stuff. It could also support WWN, which is
> >> what people are looking for these days.
> >
> > http://www.mail-archive.com/qemu-devel@nongnu.org/msg24321.html
> >
> > 1. Virtio-blk isn't an ATA device
> > 2. The ATA identify page is too large to fit into the virtio config
> > space
> >
> > I'm not finding the older threads where this was discussed in detail.
> > I'll keep looking if the above isn't a sufficient explaination.
> 
> Well, if ATA doesn't fit, then put that string in sysfs like mmc block
> devices are doing it. It looks really awkward to require a new binary
> and a new ioctl to get a single string out of something that was just
> invented.

John,

you mentioned that you had a sys interface in-mind earlier.  I'm not
sure how to proceed given the virtio-blk kernel side patches are
already upstream[1][2].  Are we looking to revert and switch?


1.  http://repo.or.cz/w/linux-2.6.git/commitdiff/234f2725a5d03f78539f1d36cb32f2c4f9b1822c
2.  http://repo.or.cz/w/linux-2.6.git/commitdiff/4cb2ea28c55cf5e5ef83aec535099ffce3c583df


-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com

[Qemu-devel] Re: [PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers

[john cooper]

Ryan Harper wrote:
> * Kay Sievers <kay.sievers@vrfy.org> [2010-06-03 15:06]:
>> On Thu, Jun 3, 2010 at 22:01, Ryan Harper <ryanh@us.ibm.com> wrote:
>>> * Kay Sievers <kay.sievers@vrfy.org> [2010-06-03 14:53]:
>>>> On Thu, Jun 3, 2010 at 21:07, Ryan Harper <ryanh@us.ibm.com> wrote:
>>>>> Use the 'VBID' virtio-blk ioctl to extract drive serial numbers
>>>>> to be used for building disk/by-id symlinks.  After extracting
>>>>> the serial number of the device it prints out the minimum info
>>>>> needed in a similar format to `scsi_id --export` so that the
>>>>> persistent-storage rules can process the serial information.
>>>>>
>>>>> This program depends on the virtio-blk serial device patches posted
>>>>> here[1] being applied to qemu and linux-kernel.
>>>>>
>>>>> Here is what the output looks like:
>>>>>
>>>>> % ./virtioblk_id /dev/vdb
>>>>> ID_VIRTIO=1
>>>>> ID_TYPE=disk
>>>>> ID_VIRTIO_SERIAL=QM00001
>>>> Yikes! An ioctl to copy a plain string, and an entire binary to call
>>>> that ioctl and print it. If we don't have enough problems we make new
>>>> ones? :)

[Jumping in the thread here as I hadn't seen the comments
until now.  This issue has been discussed ad nauseam in the
context of qemu so here is a summary]

Packaging this up this data within a mock ATA_IDENTIFY was
proposed.  However that is a large bag of largely archaic
bits most of which are dummied up normally, and would be even
more so in the case here of virtio.  Putting a proposed
identify struct mock-up in qemu made the most sense in this
scenario as that's where the disk geometry information exists,
unfortunately doing so didn't fly with the maintainer.  Locating
this mock-up in the virtio driver makes less sense as the
geometry data for mock-up would need to be packaged up in yet
another structure and passed to the driver for formatting onto
the identify data.  No one even seriously considered doing this
as it combines the worst of both worlds.

>>>> What's the reason to drop the ATA identify, that would work out-of-the
>>>> box without any of this stuff. It could also support WWN, which is
>>>> what people are looking for these days.

That is the singular, very strong argument for ATA_IDENTIFY
and one which I'd made several times however it isn't the only
consideration in this multi-way trade off.

>>> http://www.mail-archive.com/qemu-devel@nongnu.org/msg24321.html
>>>
>>> 1. Virtio-blk isn't an ATA device
>>> 2. The ATA identify page is too large to fit into the virtio config
>>> space

Passing this data in PCI config made sense in the original
version of this patch when I was just shuffling 20 bytes of
id to the driver.  But when the ATA_IDENTIFY argument arose
it extended the size of the passed data to 512 bytes which caused
breakage of PCI config space due to the size now consumed to
quietly exceed the PCI specification.  Moving the identify
scheme to pass data by virtio request was proposed but by this
time the maintainer was strongly objecting to mock-up of this data
within qemu.

>>> I'm not finding the older threads where this was discussed in detail.
>>> I'll keep looking if the above isn't a sufficient explaination.
>> Well, if ATA doesn't fit, then put that string in sysfs like mmc block
>> devices are doing it. It looks really awkward to require a new binary
>> and a new ioctl to get a single string out of something that was just
>> invented.

There are two issues:

- Getting the serial/id string exported from qemu to the driver.
  This is the much less contentious portion unless we're still
  bent on using the identify scheme which will surely torpedo
  the effort once again.  Assuming we're satisfied with qemu
  exporting the serial/id string alone we should be quite close
  here.

- Presentation of the serial/id to the application by the
  virtio driver.  ATA_IDENTIFY would be nice if the above restrictions
  didn't exist, but they do.  So let's just get the job done
  once and for all.  No one wants to invent yet another ioctl
  interface but it is the best compromise available as the code
  in both the driver and application is trivial.  The other
  alternatives at their respective extremes (/sys: new but pretty
  interface, over powered for the job, more cruft in the driver;
  ATA_IDENTIFY: existing interface, ugliest possible code, even
  more cruft in the driver) have been proposed and abandoned for
  reasons cited above.
 
> John,
> 
> you mentioned that you had a sys interface in-mind earlier.  I'm not
> sure how to proceed given the virtio-blk kernel side patches are
> already upstream[1][2].  Are we looking to revert and switch?

No, we can use the interface as-is, however we have an opportunity
to make it more usage friendly.

> 1.  http://repo.or.cz/w/linux-2.6.git/commitdiff/234f2725a5d03f78539f1d36cb32f2c4f9b1822c

My slight modification to the above commit makes it possible
for the application to determine the data size the driver intends
to copy out, and allows the user to indicate the destination data
size for the copy operation.  Otherwise the caller needs to have
the size assumption built-in.

Your immediate use case here I believe to be the needed motivation
to finally drive this to a conclusion.  So folks lets just tie this
one off. 

Thanks,

-john


-- 
john.cooper@redhat.com

Re: [PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers

[john cooper]

Ryan Harper wrote:
> * Kay Sievers <kay.sievers@vrfy.org> [2010-06-03 15:06]:
>> On Thu, Jun 3, 2010 at 22:01, Ryan Harper <ryanh@us.ibm.com> wrote:
>>> * Kay Sievers <kay.sievers@vrfy.org> [2010-06-03 14:53]:
>>>> On Thu, Jun 3, 2010 at 21:07, Ryan Harper <ryanh@us.ibm.com> wrote:
>>>>> Use the 'VBID' virtio-blk ioctl to extract drive serial numbers
>>>>> to be used for building disk/by-id symlinks.  After extracting
>>>>> the serial number of the device it prints out the minimum info
>>>>> needed in a similar format to `scsi_id --export` so that the
>>>>> persistent-storage rules can process the serial information.
>>>>>
>>>>> This program depends on the virtio-blk serial device patches posted
>>>>> here[1] being applied to qemu and linux-kernel.
>>>>>
>>>>> Here is what the output looks like:
>>>>>
>>>>> % ./virtioblk_id /dev/vdb
>>>>> ID_VIRTIO=1
>>>>> ID_TYPE=disk
>>>>> ID_VIRTIO_SERIAL=QM00001
>>>> Yikes! An ioctl to copy a plain string, and an entire binary to call
>>>> that ioctl and print it. If we don't have enough problems we make new
>>>> ones? :)

[Jumping in the thread here as I hadn't seen the comments
until now.  This issue has been discussed ad nauseam in the
context of qemu so here is a summary]

Packaging this up this data within a mock ATA_IDENTIFY was
proposed.  However that is a large bag of largely archaic
bits most of which are dummied up normally, and would be even
more so in the case here of virtio.  Putting a proposed
identify struct mock-up in qemu made the most sense in this
scenario as that's where the disk geometry information exists,
unfortunately doing so didn't fly with the maintainer.  Locating
this mock-up in the virtio driver makes less sense as the
geometry data for mock-up would need to be packaged up in yet
another structure and passed to the driver for formatting onto
the identify data.  No one even seriously considered doing this
as it combines the worst of both worlds.

>>>> What's the reason to drop the ATA identify, that would work out-of-the
>>>> box without any of this stuff. It could also support WWN, which is
>>>> what people are looking for these days.

That is the singular, very strong argument for ATA_IDENTIFY
and one which I'd made several times however it isn't the only
consideration in this multi-way trade off.

>>> http://www.mail-archive.com/qemu-devel@nongnu.org/msg24321.html
>>>
>>> 1. Virtio-blk isn't an ATA device
>>> 2. The ATA identify page is too large to fit into the virtio config
>>> space

Passing this data in PCI config made sense in the original
version of this patch when I was just shuffling 20 bytes of
id to the driver.  But when the ATA_IDENTIFY argument arose
it extended the size of the passed data to 512 bytes which caused
breakage of PCI config space due to the size now consumed to
quietly exceed the PCI specification.  Moving the identify
scheme to pass data by virtio request was proposed but by this
time the maintainer was strongly objecting to mock-up of this data
within qemu.

>>> I'm not finding the older threads where this was discussed in detail.
>>> I'll keep looking if the above isn't a sufficient explaination.
>> Well, if ATA doesn't fit, then put that string in sysfs like mmc block
>> devices are doing it. It looks really awkward to require a new binary
>> and a new ioctl to get a single string out of something that was just
>> invented.

There are two issues:

- Getting the serial/id string exported from qemu to the driver.
  This is the much less contentious portion unless we're still
  bent on using the identify scheme which will surely torpedo
  the effort once again.  Assuming we're satisfied with qemu
  exporting the serial/id string alone we should be quite close
  here.

- Presentation of the serial/id to the application by the
  virtio driver.  ATA_IDENTIFY would be nice if the above restrictions
  didn't exist, but they do.  So let's just get the job done
  once and for all.  No one wants to invent yet another ioctl
  interface but it is the best compromise available as the code
  in both the driver and application is trivial.  The other
  alternatives at their respective extremes (/sys: new but pretty
  interface, over powered for the job, more cruft in the driver;
  ATA_IDENTIFY: existing interface, ugliest possible code, even
  more cruft in the driver) have been proposed and abandoned for
  reasons cited above.
 
> John,
> 
> you mentioned that you had a sys interface in-mind earlier.  I'm not
> sure how to proceed given the virtio-blk kernel side patches are
> already upstream[1][2].  Are we looking to revert and switch?

No, we can use the interface as-is, however we have an opportunity
to make it more usage friendly.

> 1.  http://repo.or.cz/w/linux-2.6.git/commitdiff/234f2725a5d03f78539f1d36cb32f2c4f9b1822c

My slight modification to the above commit makes it possible
for the application to determine the data size the driver intends
to copy out, and allows the user to indicate the destination data
size for the copy operation.  Otherwise the caller needs to have
the size assumption built-in.

Your immediate use case here I believe to be the needed motivation
to finally drive this to a conclusion.  So folks lets just tie this
one off. 

Thanks,

-john


-- 
john.cooper@redhat.com

[PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers

[Ryan Harper]

Use the 'VBID' virtio-blk ioctl to extract drive serial numbers
to be used for building disk/by-id symlinks.  After extracting
the serial number of the device it prints out the minimum info
needed in a similar format to `scsi_id --export` so that the
persistent-storage rules can process the serial information.

This program depends on the virtio-blk serial device patches posted
here[1] being applied to qemu and linux-kernel.

Here is what the output looks like:

% ./virtioblk_id /dev/vdb
ID_VIRTIO=1
ID_TYPE=disk
ID_SERIAL=QM00001
ID_SERIAL_SHORT=QM00001

1. http://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01869.html

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
---
 Makefile.am                        |    7 +++
 extras/virtioblk_id/virtioblk_id.c |   83 ++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 0 deletions(-)
 create mode 100644 extras/virtioblk_id/virtioblk_id.c

diff --git a/Makefile.am b/Makefile.am
index bafe4c7..6b8f41b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -251,6 +251,13 @@ extras_path_id_path_id_LDADD = libudev/libudev-private.la
 libexec_PROGRAMS += extras/path_id/path_id
 
 # ------------------------------------------------------------------------------
+# virtioblk_id - virtioblk inquery to extract disk serial numbers
+# ------------------------------------------------------------------------------
+extras_virtioblk_id_virtioblk_id_SOURCES = extras/virtioblk_id/virtioblk_id.c
+extras_virtioblk_id_virtioblk_id_LDADD = libudev/libudev-private.la
+libexec_PROGRAMS += extras/virtioblk_id/virtioblk_id
+
+# ------------------------------------------------------------------------------
 # fstab_import - import /etc/fstab entry for block device
 # ------------------------------------------------------------------------------
 extras_fstab_import_fstab_import_SOURCES = extras/fstab_import/fstab_import.c
diff --git a/extras/virtioblk_id/virtioblk_id.c b/extras/virtioblk_id/virtioblk_id.c
new file mode 100644
index 0000000..be9ff2e
--- /dev/null
+++ b/extras/virtioblk_id/virtioblk_id.c
@@ -0,0 +1,83 @@
+/*
+ * extract disk serial from virtio-blk devices
+ * and print enough values to allow udev to create disk/by-id/ symlinks
+ *
+ * Copyright IBM, Corp. 2010
+ * 
+ * Authors:
+ *  John Cooper <john.cooper@redhat.com>
+ *  Ryan Harper <ryanh@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <stdio.h>
+#include <strings.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <linux/hdreg.h>
+#include <getopt.h>
+
+/* virtio-blk VBID ioctl */
+#define IOCTL_CMD 0x56424944
+
+int main(int argc, char **argv)
+{
+    int fd, rv;
+    const char *device;
+    char buf[255];
+
+    int opt_debug = 0;
+    static const char *option_string = "d";
+    static struct option options[] = {
+        {"debug", 0, NULL, 'd'},
+        {"help", 0, NULL, 'h'},
+        {0, 0, NULL, 0}
+    };
+
+    while (1) {
+        int option;
+        option = getopt_long(argc, argv, "dh", options, NULL);
+        if (option = -1)
+            break;
+        switch (option) {
+        case 'd':
+            opt_debug = 1;
+            break;
+        case 'h':
+            printf("Usage: virtioblk_id [--debug] [--help] <device>\n"
+                   "  --debug           print debugging information\n"
+                   "  --help            print this help text\n\n");
+        default:
+            rv = 1;
+            goto exit;
+        }
+    }
+
+    device = argv[optind];
+    if (device = NULL) {
+        fprintf(stderr, "no device specified\n");
+        rv = 1;
+        goto exit;
+    }
+
+
+    bzero(buf, sizeof (buf));
+    if ((fd = open(device, O_RDONLY)) < 0) {
+        if (opt_debug = 1) 
+            perror("open");
+    } else if ((rv = ioctl(fd, IOCTL_CMD, buf)) < 0) {
+        if (opt_debug = 1) 
+            perror("ioctl");
+    } else {
+        printf("ID_VIRTIO=1\n");
+        printf("ID_TYPE=disk\n");
+        printf("ID_SERIAL=%s\n", buf);
+        printf("ID_SERIAL_SHORT=%s\n", buf);
+        rv = 0;
+    }
+
+ exit:
+    return rv;
+}
-- 
1.6.3.3

Re: [PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers

[Kay Sievers]

On Thu, Jun 10, 2010 at 21:16, Ryan Harper <ryanh@us.ibm.com> wrote:
> Use the 'VBID' virtio-blk ioctl to extract drive serial numbers
> to be used for building disk/by-id symlinks.  After extracting
> the serial number of the device it prints out the minimum info
> needed in a similar format to `scsi_id --export` so that the
> persistent-storage rules can process the serial information.
>
> This program depends on the virtio-blk serial device patches posted
> here[1] being applied to qemu and linux-kernel.
>
> Here is what the output looks like:
>
> % ./virtioblk_id /dev/vdb
> ID_VIRTIO=1
> ID_TYPE=disk
> ID_SERIAL=QM00001
> ID_SERIAL_SHORT=QM00001

As requested in the ealier mail. Please provide a good reason why the
kernel code can not create a "serial" file (or whatever fits your
needs) in sysfs at the block device.

Other subsystems are doing the same thing like:
  http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/mmc/core/mmc.c;h‰f7a25b7ac12ab17f7baa9a18e0d980fe1c50eb;hb=HEAD#l270

We don't like to accumulate special purpose tools based on rather
weird ioctls for newly developed stuff, which can be replaced with a
few obviously and generally useful lines of code in the kernel driver.

As I think the general direction of solving the problem you describe
is fundamentally wrong, I'll apply this only, if there are valid
reasons not to add this to sysfs.

Thanks,
Kay

Re: [PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers

[Ryan Harper]

* Kay Sievers <kay.sievers@vrfy.org> [2010-06-14 03:55]:
> On Thu, Jun 10, 2010 at 21:16, Ryan Harper <ryanh@us.ibm.com> wrote:
> > Use the 'VBID' virtio-blk ioctl to extract drive serial numbers
> > to be used for building disk/by-id symlinks.  After extracting
> > the serial number of the device it prints out the minimum info
> > needed in a similar format to `scsi_id --export` so that the
> > persistent-storage rules can process the serial information.
> >
> > This program depends on the virtio-blk serial device patches posted
> > here[1] being applied to qemu and linux-kernel.
> >
> > Here is what the output looks like:
> >
> > % ./virtioblk_id /dev/vdb
> > ID_VIRTIO=1
> > ID_TYPE=disk
> > ID_SERIAL=QM00001
> > ID_SERIAL_SHORT=QM00001
> 
> As requested in the ealier mail. Please provide a good reason why the
> kernel code can not create a "serial" file (or whatever fits your
> needs) in sysfs at the block device.

I didn't see you respond John's email:

http://www.spinics.net/lists/hotplug/msg03869.html

which included quite  bit of the gory history on this topic.

> 
> Other subsystems are doing the same thing like:
>   http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/mmc/core/mmc.c;h‰f7a25b7ac12ab17f7baa9a18e0d980fe1c50eb;hb=HEAD#l270
> 
> We don't like to accumulate special purpose tools based on rather
> weird ioctls for newly developed stuff, which can be replaced with a
> few obviously and generally useful lines of code in the kernel driver.
> 
> As I think the general direction of solving the problem you describe
> is fundamentally wrong, I'll apply this only, if there are valid
> reasons not to add this to sysfs.
> 
> Thanks,
> Kay

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com

Re: [PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers

[Kay Sievers]

On Mon, Jun 14, 2010 at 15:15, Ryan Harper <ryanh@us.ibm.com> wrote:
> * Kay Sievers <kay.sievers@vrfy.org> [2010-06-14 03:55]:
>> On Thu, Jun 10, 2010 at 21:16, Ryan Harper <ryanh@us.ibm.com> wrote:
>> > Use the 'VBID' virtio-blk ioctl to extract drive serial numbers
>> > to be used for building disk/by-id symlinks.  After extracting
>> > the serial number of the device it prints out the minimum info
>> > needed in a similar format to `scsi_id --export` so that the
>> > persistent-storage rules can process the serial information.
>> >
>> > This program depends on the virtio-blk serial device patches posted
>> > here[1] being applied to qemu and linux-kernel.
>> >
>> > Here is what the output looks like:
>> >
>> > % ./virtioblk_id /dev/vdb
>> > ID_VIRTIO=1
>> > ID_TYPE=disk
>> > ID_SERIAL=QM00001
>> > ID_SERIAL_SHORT=QM00001
>>
>> As requested in the ealier mail. Please provide a good reason why the
>> kernel code can not create a "serial" file (or whatever fits your
>> needs) in sysfs at the block device.
>
> I didn't see you respond John's email:
>
> http://www.spinics.net/lists/hotplug/msg03869.html
>
> which included quite  bit of the gory history on this topic.

Yeah, and I don't find any reason there, why a sysfs attribute will not work.

Kay

Re: [PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers

[Ryan Harper]

* Kay Sievers <kay.sievers@vrfy.org> [2010-06-14 08:30]:
> On Mon, Jun 14, 2010 at 15:15, Ryan Harper <ryanh@us.ibm.com> wrote:
> > * Kay Sievers <kay.sievers@vrfy.org> [2010-06-14 03:55]:
> >> On Thu, Jun 10, 2010 at 21:16, Ryan Harper <ryanh@us.ibm.com> wrote:
> >> > Use the 'VBID' virtio-blk ioctl to extract drive serial numbers
> >> > to be used for building disk/by-id symlinks.  After extracting
> >> > the serial number of the device it prints out the minimum info
> >> > needed in a similar format to `scsi_id --export` so that the
> >> > persistent-storage rules can process the serial information.
> >> >
> >> > This program depends on the virtio-blk serial device patches posted
> >> > here[1] being applied to qemu and linux-kernel.
> >> >
> >> > Here is what the output looks like:
> >> >
> >> > % ./virtioblk_id /dev/vdb
> >> > ID_VIRTIO=1
> >> > ID_TYPE=disk
> >> > ID_SERIAL=QM00001
> >> > ID_SERIAL_SHORT=QM00001
> >>
> >> As requested in the ealier mail. Please provide a good reason why the
> >> kernel code can not create a "serial" file (or whatever fits your
> >> needs) in sysfs at the block device.
> >
> > I didn't see you respond John's email:
> >
> > http://www.spinics.net/lists/hotplug/msg03869.html
> >
> > which included quite  bit of the gory history on this topic.
> 
> Yeah, and I don't find any reason there, why a sysfs attribute will not work.

You were quite right; the sysfs attribute route was rather simple.
Thanks for pointing me in the right direction.  We've got the
kernel-side fix picked up.  I've posted v3 of this series that uses the
sysfs attribute.

Thanks!

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com