From: john cooper <john.cooper@redhat.com> To: Ryan Harper <ryanh@us.ibm.com> Cc: Kay Sievers <kay.sievers@vrfy.org>, john.cooper@redhat.com, Rusty Russell <rusty@rustcorp.com.au>, linux-hotplug@vger.kernel.org, qemu-devel@nongnu.org, Marc Haber <mh+qemu-devel@zugschlus.de> Subject: [Qemu-devel] Re: [PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers Date: Fri, 04 Jun 2010 11:06:26 -0400 [thread overview] Message-ID: <4C091672.9070700@redhat.com> (raw) In-Reply-To: <20100603210359.GP19185@us.ibm.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: john cooper <john.cooper@redhat.com> To: Ryan Harper <ryanh@us.ibm.com> Cc: Kay Sievers <kay.sievers@vrfy.org>, john.cooper@redhat.com, Rusty Russell <rusty@rustcorp.com.au>, linux-hotplug@vger.kernel.org, qemu-devel@nongnu.org, Marc Haber <mh+qemu-devel@zugschlus.de> Subject: Re: [PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers Date: Fri, 04 Jun 2010 15:06:26 +0000 [thread overview] Message-ID: <4C091672.9070700@redhat.com> (raw) In-Reply-To: <20100603210359.GP19185@us.ibm.com> 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
next prev parent reply other threads:[~2010-06-04 15:20 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2010-06-03 19:07 [Qemu-devel] [PATCH 0/3] Add virtio-blk support to persistent-storage rules Ryan Harper 2010-06-03 19:07 ` Ryan Harper 2010-06-03 19:07 ` [PATCH 1/3] Add virtioblk_id tool to extract drive serial numbers Ryan Harper 2010-06-03 19:52 ` Kay Sievers 2010-06-03 20:01 ` Ryan Harper 2010-06-03 20:04 ` Kay Sievers 2010-06-03 20:13 ` David Zeuthen 2010-06-03 21:03 ` Ryan Harper 2010-06-04 15:06 ` john cooper [this message] 2010-06-04 15:06 ` john cooper 2010-06-10 19:16 ` Ryan Harper 2010-06-14 8:54 ` Kay Sievers 2010-06-14 13:15 ` Ryan Harper 2010-06-14 13:29 ` Kay Sievers 2010-06-29 14:48 ` Ryan Harper
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=4C091672.9070700@redhat.com \ --to=john.cooper@redhat.com \ --cc=kay.sievers@vrfy.org \ --cc=linux-hotplug@vger.kernel.org \ --cc=mh+qemu-devel@zugschlus.de \ --cc=qemu-devel@nongnu.org \ --cc=rusty@rustcorp.com.au \ --cc=ryanh@us.ibm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.