[Qemu-devel] Re: [PATCH] Add serial number support for virtio_blk

[john cooper]

Anthony Liguori wrote:
> john cooper wrote:
>>
>>      s = (VirtIOBlock *)virtio_init_pci(bus, "virtio-blk",
>>                                         PCI_VENDOR_ID_REDHAT_QUMRANET,
>> @@ -369,6 +420,10 @@ void *virtio_blk_init(PCIBus *bus,
>> BlockDriverState *bs)
>>      s->vdev.reset = virtio_blk_reset;
>>      s->bs = bs;
>>      s->rq = NULL;
>> +    if (strlen(ps = (char *)drive_get_serial(bs)))
>> +        strncpy(s->serial_str, ps, sizeof(s->serial_str));
>> +    else
>> +        snprintf(s->serial_str, sizeof(s->serial_str), "0");
>>   
> 
> ps = drive_get_serial(bs);
> snprintf(s->serial_str, sizeof(s->serial_str), "%s", *ps ? ps : "0");
> 
> strncpy() doesn't do what you think it does.  It doesn't always null
> terminate.

In general yes, but here it is contrived to copy a
terminating nul.  The string is maintained as a 21
byte [BLOCK_SERIAL_STRLEN + 1] char[] and the
incoming cmdline serial string is hard null terminated
by get_opt_value().  Above, strncpy() into a sizeof(21)
byte s->serial_str, will copy the trailing nul.
However when this data is exported by the guest driver
in a char[20] sized structure the trailing nul may be
omitted.

The same logic may be found in IDE and SCSI counterparts.
I agree it's not the most obvious approach, and this
clause should ideally be factored out as common to
all cases once we have the patch under discussion
resolved.

> Doesn't serial_str need to be saved in the savevm format?

Possibly, but currently it isn't being captured in
IDE nor SCSI AFAICT.  I'll take a closer look.

-john

-- 
address@hidden