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

[Eric Blake]

On 3/25/19 6:53 AM, Kevin Wolf wrote:
> Am 25.03.2019 um 11:27 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> 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).

>>> @@ -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:/:/
> 
> I think that would be misleading, because I would understand that actual
> data I/O (bdrv_co_preadv) has failed.
> 
> Actually, before sending my R-b, for a moment I thought of suggesting to
> make the message something like "Could not get block status for sector
> ..." to make it less likely that someone just reads the start and
> misinterprets the message. But then I decided that the colour of this
> specific bike shed isn't important enough to me to request a respin.

Prior to commit 690c7301, it was "error while reading block status of
sector % "PRId64 " %s"

I can change that while queuing through my NBD tree.

Thanks; applying to my queue for 4.0.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature