| Message ID | 20100602014854.GB16406@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
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
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
* 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()
* 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.
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
* 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 <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
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);
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(-)