Re: [Qemu-devel] [PATCH v4 5/5] qcow2-refcount: don't mask corruptions under internal errors

[Vladimir Sementsov-Ogievskiy]

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?


-- 
Best regards,
Vladimir