Anthony Liguori wrote: > On 03/25/2010 12:32 AM, john cooper wrote: >> Add virtio-blk device id (s/n) support via virtio request. >> Remove artifacts of pci and ATA_IDENTIFY implementation >> relative to prior versions. >> >> Signed-off-by: john cooper<john.coo...@redhat.com> >> --- >> >> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c >> index 9915840..358b0af 100644 >> --- a/hw/virtio-blk.c >> +++ b/hw/virtio-blk.c >> @@ -19,6 +19,8 @@ >> # include<scsi/sg.h> >> #endif >> >> +#define min(a,b) ((a)< (b) ? (a) : (b)) >> > > We already have MIN(). > >> + >> typedef struct VirtIOBlock >> { >> VirtIODevice vdev; >> @@ -28,6 +30,7 @@ typedef struct VirtIOBlock >> QEMUBH *bh; >> BlockConf *conf; >> unsigned short sector_mask; >> + char sn[BLOCK_SERIAL_STRLEN]; >> } VirtIOBlock; >> >> static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev) >> @@ -317,6 +320,12 @@ static void >> virtio_blk_handle_request(VirtIOBlockReq *req, >> virtio_blk_handle_flush(req); >> } else if (req->out->type& VIRTIO_BLK_T_SCSI_CMD) { >> virtio_blk_handle_scsi(req); >> + } else if (req->out->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))); >> + virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); >> } else if (req->out->type& VIRTIO_BLK_T_OUT) { >> qemu_iovec_init_external(&req->qiov,&req->elem.out_sg[1], >> req->elem.out_num - 1); >> @@ -496,6 +505,8 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, >> BlockConf *conf) >> bdrv_guess_geometry(s->bs,&cylinders,&heads,&secs); >> bdrv_set_geometry_hint(s->bs, cylinders, heads, secs); >> >> + strncpy(s->sn, drive_get_serial(s->bs), sizeof (s->sn)); >> + >> > > Friends don't let friends use strncpy(). > > This actually will result in a non-NULL terminated string if > drive_get_serial() returns a string larger than s->sn. Use snprintf() > instead.
That actually is the desired behavior here as a serial string is of BLOCK_SERIAL_STRLEN bytes length maximum and not assured to be nul terminated (legacy ATA convention). snprintf() would cause us to lose the last string character in the case the full BLOCK_SERIAL_STRLEN bytes were in use. There are existing storage allocations of BLOCK_SERIAL_STRLEN + 1 in some cases but this appears as an internal convenience and is not part of the serial string data. -john -- john.coo...@redhat.com