[Qemu-devel] [PATCH] virtio-blk: assign a default serial number if none provided

[Qemu-devel] [PATCH] virtio-blk: assign a default serial number if none provided

[Ryan Harper]

This patch applies on-top of John's virtio-blk serial patches.

Generate default serial numbers for virtio drives based on DriveInfo.unit which is
incremented for each additional virtio-blk device.  This provides a
per-virtio-blk number to use in the default string: QM%05d that is used in
hw/ide/core.c.  The resulting serial number looks like: QM00001, etc.

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
---
 hw/virtio-blk.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 98c62f2..e5c6e7c 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -518,6 +518,9 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf)
     bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
 
     strncpy(s->sn, drive_get_serial(s->bs), sizeof (s->sn));
+    if (strlen(s->sn) == 0) {
+        snprintf(s->sn, sizeof(s->sn), "QM%05d", conf->dinfo->unit);
+    }
 
     s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
 
-- 
1.6.3.3


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

Re: [Qemu-devel] [PATCH] virtio-blk: assign a default serial number if none provided

[Michael Tokarev]

02.06.2010 05:48, Ryan Harper wrote:
[]
>   hw/virtio-blk.c |    3 +++
> +    if (strlen(s->sn) == 0) {

Just out of curiocity (not that it is wrong or inefficient):
why
   strlen(s->sn)
and not, say,
   !s->sn[0]
?

/mjt

Re: [Qemu-devel] [PATCH] virtio-blk: assign a default serial number if none provided

[Ryan Harper]

* Michael Tokarev <mjt@tls.msk.ru> [2010-06-02 01:57]:
> 02.06.2010 05:48, Ryan Harper wrote:
> []
> >  hw/virtio-blk.c |    3 +++
> >+    if (strlen(s->sn) == 0) {
> 
> Just out of curiocity (not that it is wrong or inefficient):
> why
>   strlen(s->sn)
> and not, say,
>   !s->sn[0]
> ?

Just matching how it's done in hw/ide/core.c:ide_init_drive()


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

Re: [Qemu-devel] [PATCH] virtio-blk: assign a default serial number if none provided

[Markus Armbruster]

Ryan Harper <ryanh@us.ibm.com> writes:

> * Michael Tokarev <mjt@tls.msk.ru> [2010-06-02 01:57]:
>> 02.06.2010 05:48, Ryan Harper wrote:
>> []
>> >  hw/virtio-blk.c |    3 +++
>> >+    if (strlen(s->sn) == 0) {
>> 
>> Just out of curiocity (not that it is wrong or inefficient):
>> why
>>   strlen(s->sn)
>> and not, say,
>>   !s->sn[0]
>> ?
>
> Just matching how it's done in hw/ide/core.c:ide_init_drive()

I cleaned it up there the other day.

[PATCH 11/14] ide: Turn drive serial into a qdev property ide-drive.serial
http://thread.gmane.org/gmane.comp.emulators.qemu/72441/focus=72432
http://repo.or.cz/w/qemu/kevin.git/commit/ebf5f0ff455a794faca404e62e5dd6d5bdf2fb9b

[Qemu-devel] Re: [PATCH] virtio-blk: assign a default serial number if none provided

[Michael S. Tsirkin]

On Tue, Jun 01, 2010 at 08:48:54PM -0500, Ryan Harper wrote:
> This patch applies on-top of John's virtio-blk serial patches.
> 
> Generate default serial numbers for virtio drives based on DriveInfo.unit which is
> incremented for each additional virtio-blk device.  This provides a
> per-virtio-blk number to use in the default string: QM%05d that is used in
> hw/ide/core.c.  The resulting serial number looks like: QM00001, etc.
> 
> Signed-off-by: Ryan Harper <ryanh@us.ibm.com>


I think that there's a problem with this approach in that hot plug A,
hot plug B, hot unplug A is not the same as hot plug B.
So you might get guest boot failures and no easy way to
figure out why. For guests that need S/N, I think they
really must be persistent.

> ---
>  hw/virtio-blk.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index 98c62f2..e5c6e7c 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -518,6 +518,9 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf)
>      bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
>  
>      strncpy(s->sn, drive_get_serial(s->bs), sizeof (s->sn));
> +    if (strlen(s->sn) == 0) {
> +        snprintf(s->sn, sizeof(s->sn), "QM%05d", conf->dinfo->unit);
> +    }
>  
>      s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
>  
> -- 
> 1.6.3.3
> 
> 
> -- 
> Ryan Harper
> Software Engineer; Linux Technology Center
> IBM Corp., Austin, Tx
> ryanh@us.ibm.com

Re: [Qemu-devel] Re: [PATCH] virtio-blk: assign a default serial number if none provided

[Ryan Harper]

* Michael S. Tsirkin <mst@redhat.com> [2010-06-02 04:08]:
> On Tue, Jun 01, 2010 at 08:48:54PM -0500, Ryan Harper wrote:
> > This patch applies on-top of John's virtio-blk serial patches.
> > 
> > Generate default serial numbers for virtio drives based on DriveInfo.unit which is
> > incremented for each additional virtio-blk device.  This provides a
> > per-virtio-blk number to use in the default string: QM%05d that is used in
> > hw/ide/core.c.  The resulting serial number looks like: QM00001, etc.
> > 
> > Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> 
> 
> I think that there's a problem with this approach in that hot plug A,
> hot plug B, hot unplug A is not the same as hot plug B.
> So you might get guest boot failures and no easy way to
> figure out why. For guests that need S/N, I think they
> really must be persistent.

That's true; though I think most boot drives boot via either LVM or UUID
which will remain persistent.  That said, if you are relying on the
by-id; then of course the user will need to specify serial versus having
one auto-generated.

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

Re: [Qemu-devel] Re: [PATCH] virtio-blk: assign a default serial number if none provided

[Michael S. Tsirkin]

On Wed, Jun 02, 2010 at 06:45:46AM -0500, Ryan Harper wrote:
> * Michael S. Tsirkin <mst@redhat.com> [2010-06-02 04:08]:
> > On Tue, Jun 01, 2010 at 08:48:54PM -0500, Ryan Harper wrote:
> > > This patch applies on-top of John's virtio-blk serial patches.
> > > 
> > > Generate default serial numbers for virtio drives based on DriveInfo.unit which is
> > > incremented for each additional virtio-blk device.  This provides a
> > > per-virtio-blk number to use in the default string: QM%05d that is used in
> > > hw/ide/core.c.  The resulting serial number looks like: QM00001, etc.
> > > 
> > > Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> > 
> > 
> > I think that there's a problem with this approach in that hot plug A,
> > hot plug B, hot unplug A is not the same as hot plug B.
> > So you might get guest boot failures and no easy way to
> > figure out why. For guests that need S/N, I think they
> > really must be persistent.
> 
> That's true; though I think most boot drives boot via either LVM or UUID
> which will remain persistent.  That said, if you are relying on the
> by-id; then of course the user will need to specify serial versus having
> one auto-generated.

I guess the question then would be, if you don't rely on the S/N, why
do you want to set it?

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

Re: [Qemu-devel] Re: [PATCH] virtio-blk: assign a default serial number if none provided

[Ryan Harper]

* Michael S. Tsirkin <mst@redhat.com> [2010-06-02 06:59]:
> On Wed, Jun 02, 2010 at 06:45:46AM -0500, Ryan Harper wrote:
> > * Michael S. Tsirkin <mst@redhat.com> [2010-06-02 04:08]:
> > > On Tue, Jun 01, 2010 at 08:48:54PM -0500, Ryan Harper wrote:
> > > > This patch applies on-top of John's virtio-blk serial patches.
> > > > 
> > > > Generate default serial numbers for virtio drives based on DriveInfo.unit which is
> > > > incremented for each additional virtio-blk device.  This provides a
> > > > per-virtio-blk number to use in the default string: QM%05d that is used in
> > > > hw/ide/core.c.  The resulting serial number looks like: QM00001, etc.
> > > > 
> > > > Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> > > 
> > > 
> > > I think that there's a problem with this approach in that hot plug A,
> > > hot plug B, hot unplug A is not the same as hot plug B.
> > > So you might get guest boot failures and no easy way to
> > > figure out why. For guests that need S/N, I think they
> > > really must be persistent.
> > 
> > That's true; though I think most boot drives boot via either LVM or UUID
> > which will remain persistent.  That said, if you are relying on the
> > by-id; then of course the user will need to specify serial versus having
> > one auto-generated.
> 
> I guess the question then would be, if you don't rely on the S/N, why
> do you want to set it?

Functional similarity; we have default serial numbers for ide and scsi.
scsi is hotpluggable and would suffer the same issue; and of course ide
doesn't do hotplug.  I don't think most folks will encounter the above
scenario and that having the same default serial numbers being generated
like we do ide and scsi is reasonable.


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