[Qemu-devel] [PATCH v2 1/7] block/qcow2-refcount: fix check_oflag_copied

Vladimir Sementsov-Ogievskiy posted 7 patches 7 years, 2 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v2 1/7] block/qcow2-refcount: fix check_oflag_copied
Posted by Vladimir Sementsov-Ogievskiy 7 years, 2 months ago
Increase corruptions_fixed only after successful fix.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2-refcount.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 3c539f02e5..615847eb09 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1816,7 +1816,7 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
     for (i = 0; i < s->l1_size; i++) {
         uint64_t l1_entry = s->l1_table[i];
         uint64_t l2_offset = l1_entry & L1E_OFFSET_MASK;
-        bool l2_dirty = false;
+        int l2_fixed_entries = 0;
 
         if (!l2_offset) {
             continue;
@@ -1878,8 +1878,7 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
                         l2_table[j] = cpu_to_be64(refcount == 1
                                     ? l2_entry |  QCOW_OFLAG_COPIED
                                     : l2_entry & ~QCOW_OFLAG_COPIED);
-                        l2_dirty = true;
-                        res->corruptions_fixed++;
+                        l2_fixed_entries++;
                     } else {
                         res->corruptions++;
                     }
@@ -1887,7 +1886,7 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
             }
         }
 
-        if (l2_dirty) {
+        if (l2_fixed_entries > 0) {
             ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L2,
                                                 l2_offset, s->cluster_size);
             if (ret < 0) {
@@ -1905,6 +1904,7 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
                 res->check_errors++;
                 goto fail;
             }
+            res->corruptions_fixed += l2_fixed_entries;
         }
     }
 
-- 
2.11.1


Re: [Qemu-devel] [PATCH v2 1/7] block/qcow2-refcount: fix check_oflag_copied
Posted by Max Reitz 7 years ago
On 17.08.18 14:22, Vladimir Sementsov-Ogievskiy wrote:
> Increase corruptions_fixed only after successful fix.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/qcow2-refcount.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 3c539f02e5..615847eb09 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -1816,7 +1816,7 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
>      for (i = 0; i < s->l1_size; i++) {
>          uint64_t l1_entry = s->l1_table[i];
>          uint64_t l2_offset = l1_entry & L1E_OFFSET_MASK;
> -        bool l2_dirty = false;
> +        int l2_fixed_entries = 0;

I understand that this new variable kind of supersedes the other one,
but I think for the sake of readability it would be better to keep l2_dirty.

Max

>          if (!l2_offset) {
>              continue;
> @@ -1878,8 +1878,7 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
>                          l2_table[j] = cpu_to_be64(refcount == 1
>                                      ? l2_entry |  QCOW_OFLAG_COPIED
>                                      : l2_entry & ~QCOW_OFLAG_COPIED);
> -                        l2_dirty = true;
> -                        res->corruptions_fixed++;
> +                        l2_fixed_entries++;
>                      } else {
>                          res->corruptions++;
>                      }
> @@ -1887,7 +1886,7 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
>              }
>          }
>  
> -        if (l2_dirty) {
> +        if (l2_fixed_entries > 0) {
>              ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L2,
>                                                  l2_offset, s->cluster_size);
>              if (ret < 0) {
> @@ -1905,6 +1904,7 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
>                  res->check_errors++;
>                  goto fail;
>              }
> +            res->corruptions_fixed += l2_fixed_entries;
>          }
>      }
>  
>