Re: [Qemu-block] [PATCH 1/2] qemu-img: Report bdrv_block_status failures

[Vladimir Sementsov-Ogievskiy]

25.03.2019 13:27, Vladimir Sementsov-Ogievskiy wrote:
> 24.03.2019 0:26, Eric Blake wrote:
>> If bdrv_block_status_above() fails, we are aborting the convert
>> process but failing to print an error message.  Broken in commit
>> 690c7301 (v2.4) when rewriting convert's logic.
>>
>> Discovered when teaching nbdkit to support NBD_CMD_BLOCK_STATUS, and
>> accidentally violating the protocol by returning more than one extent
>> in spite of qemu asking for NBD_CMD_FLAG_REQ_ONE.  The qemu NBD code
>> should probably handle the server's non-compliance more gracefully
>> than failing with EINVAL, but qemu-img shouldn't be silently
>> squelching any block status failures. It doesn't help that qemu 3.1
>> masks the qemu-img bug with extra noise that the nbd code is dumping
>> to stderr (that noise was cleaned up in d8b4bad8).
>>
>> Reported-by: Richard W.M. Jones <address@hidden>
>> Signed-off-by: Eric Blake <address@hidden>
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> 
>> ---
>>   qemu-img.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 5fac8407428..a67c28e9ae5 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -1630,6 +1630,8 @@ static int convert_iteration_sectors(ImgConvertState 
>> *s, int64_t sector_num)
>>                                             count, &count, NULL, NULL);
>>           }
>>           if (ret < 0) {
>> +            error_report("Could not read sector %" PRId64 " metadata: %s",
>> +                         sector_num, strerror(-ret));
> 
> hmm first time I see that is called "metadata", more common pattern is just 
> s/ metadata:/:/


Oops, it's OK, "metadata" is about "sector".

> 
>>               return ret;
>>           }
>>           n = DIV_ROUND_UP(count, BDRV_SECTOR_SIZE);
>>
> 
> 


-- 
Best regards,
Vladimir