[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] qemu-img: Report bdrv_block_status failures
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] qemu-img: Report bdrv_block_status failures |
Date: |
Mon, 25 Mar 2019 11:10:13 +0000 |
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