Am 20.06.2011 11:35, schrieb Markus Armbruster: > It needs to be a qdev property, because it belongs to the drive's > guest part. Precedence: commit a0fef654 and 6ced55a5. > > Bonus: info qtree now shows the serial number. > > Signed-off-by: Markus Armbruster <arm...@redhat.com> > --- > hw/s390-virtio-bus.c | 4 +++- > hw/s390-virtio-bus.h | 1 + > hw/virtio-blk.c | 29 +++++++++++++++++++---------- > hw/virtio-blk.h | 2 ++ > hw/virtio-pci.c | 4 +++- > hw/virtio-pci.h | 1 + > hw/virtio.h | 3 ++- > 7 files changed, 31 insertions(+), 13 deletions(-) > > diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c > index d4a12f7..2bf4821 100644 > --- a/hw/s390-virtio-bus.c > +++ b/hw/s390-virtio-bus.c > @@ -128,7 +128,8 @@ static int s390_virtio_blk_init(VirtIOS390Device *dev) > { > VirtIODevice *vdev; > > - vdev = virtio_blk_init((DeviceState *)dev, &dev->block); > + vdev = virtio_blk_init((DeviceState *)dev, &dev->block, > + &dev->block_serial); > if (!vdev) { > return -1; > } > @@ -355,6 +356,7 @@ static VirtIOS390DeviceInfo s390_virtio_blk = { > .qdev.size = sizeof(VirtIOS390Device), > .qdev.props = (Property[]) { > DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, block), > + DEFINE_PROP_STRING("serial", VirtIOS390Device, block_serial), > DEFINE_PROP_END_OF_LIST(), > }, > }; > diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h > index 0c412d0..f1bece7 100644 > --- a/hw/s390-virtio-bus.h > +++ b/hw/s390-virtio-bus.h > @@ -42,6 +42,7 @@ typedef struct VirtIOS390Device { > uint8_t feat_len; > VirtIODevice *vdev; > BlockConf block; > + char *block_serial; > NICConf nic; > uint32_t host_features; > virtio_serial_conf serial; > diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > index 91e0394..6471ac8 100644 > --- a/hw/virtio-blk.c > +++ b/hw/virtio-blk.c > @@ -28,8 +28,8 @@ typedef struct VirtIOBlock > void *rq; > QEMUBH *bh; > BlockConf *conf; > + char *serial; > unsigned short sector_mask; > - char sn[BLOCK_SERIAL_STRLEN]; > DeviceState *qdev; > } VirtIOBlock; > > @@ -362,8 +362,13 @@ static void virtio_blk_handle_request(VirtIOBlockReq > *req, > } else if (type & VIRTIO_BLK_T_GET_ID) { > VirtIOBlock *s = req->dev; > > - memcpy(req->elem.in_sg[0].iov_base, s->sn, > - MIN(req->elem.in_sg[0].iov_len, sizeof(s->sn))); > + /* > + * NB: per existing s/n string convention the string is > + * terminated by '\0' only when shorter than buffer. > + */ > + strncpy(req->elem.in_sg[0].iov_base, > + s->serial ? s->serial : "", > + MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
Not sure what you're trying to do with VIRTIO_BLK_ID_BYTES here. s->string either is dinfo->serial, in which case it happens to be the same as BLOCK_SERIAL_STRLEN and as such makes some sense. Or it may be a qdev property, in which case the string just has the length it has. Or it's the empty string. So I think in two of three cases you're potentially reading beyond the end of the buffer. Kevin