[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 5/5] qcow2-refcount: don't mask corruptions u
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v4 5/5] qcow2-refcount: don't mask corruptions under internal errors |
Date: |
Wed, 27 Feb 2019 13:48:41 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 |
On 27.02.19 13:47, Vladimir Sementsov-Ogievskiy wrote:
> 27.02.2019 15:42, Max Reitz wrote:
>> On 14.12.18 14:42, Vladimir Sementsov-Ogievskiy wrote:
>>> No reasons for not reporting found corruptions as corruptions in case
>>> of some internal errors, especially in case of just failed to fix l2
>>> entry (and in this case, missed corruptions may influence comparing
>>> logic, when we calculate difference between corruptions fields of two
>>> results)
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>> ---
>>> block/qcow2-refcount.c | 31 ++++++++++++++-----------------
>>> 1 file changed, 14 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
>>> index 8da0e91dd3..b0e006e628 100644
>>> --- a/block/qcow2-refcount.c
>>> +++ b/block/qcow2-refcount.c
>>
>> [...]
>>
>>> @@ -1899,19 +1899,16 @@ static int check_oflag_copied(BlockDriverState *bs,
>>> BdrvCheckResult *res,
>>> fprintf(stderr, "%s OFLAG_COPIED data cluster: "
>>> "l2_entry=%" PRIx64 " refcount=%" PRIu64 "\n",
>>> repair ? "Repairing" : "ERROR", l2_entry,
>>> refcount);
>>> - if (repair) {
>>> - l2_table[j] = cpu_to_be64(refcount == 1
>>> - ? l2_entry | QCOW_OFLAG_COPIED
>>> - : l2_entry & ~QCOW_OFLAG_COPIED);
>>> - l2_dirty++;
>>> - } else {
>>> - res->corruptions++;
>>> - }
>>> + l2_table[j] = cpu_to_be64(refcount == 1
>>> + ? l2_entry | QCOW_OFLAG_COPIED
>>> + : l2_entry & ~QCOW_OFLAG_COPIED);
>>> + l2_dirty++;
>>
>> I found the old logic to be easier to understand, actually. It made it
>> clear that if !repair, the L2 table is not going to be touched.
>>
>> Max
>>
>
> so,
>
> if (repair) {
> keep as is
> }
> res->corruptions++;
>
> ok?
Yep, that works for me. Or above the fprintf() as you did it in other
places in this patch.
Max
signature.asc
Description: OpenPGP digital signature