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

[Vladimir Sementsov-Ogievskiy]

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:/:/

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


-- 
Best regards,
Vladimir