Re: [Qemu-devel] [PATCH 1/1] scsi-hd: fix property unset case

[Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>]

On 03/02/2015 11:46 AM, Markus Armbruster wrote:
> Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> writes:
>
>> check conf.blk before calling blkconf_blocksizes
>>
>> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>> ---
>>   hw/scsi/scsi-disk.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
>> index 2921728..df5140e 100644
>> --- a/hw/scsi/scsi-disk.c
>> +++ b/hw/scsi/scsi-disk.c
>> @@ -2291,7 +2291,9 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
>>   static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
>>   {
>>       SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
>> -    blkconf_blocksizes(&s->qdev.conf);
>> +    if (s->qdev.conf.blk) {
>> +        blkconf_blocksizes(&s->qdev.conf);
>> +    }
>
> Looks suspicious on first glance, because block device model realize()
> methods are supposed to fail when the backend is missing.  But...
>

it will properly fail in scsi_realize

>>       s->qdev.blocksize = s->qdev.conf.logical_block_size;
>>       s->qdev.type = TYPE_DISK;
>>       if (!s->product) {
>             s->product = g_strdup("QEMU HARDDISK");
>         }
>         scsi_realize(&s->qdev, errp);
>
> ... scsi_realize() errors out then.  Worth a comment.  Or maybe call
> blkconf_blocksizes() only after scsi_realize().  Your choice.

can't call it later. conf.logical_block_size, which blkconf_blocksizes
sets it used earlier.