[v10,09/12] parallels: Move check of leaks to a separate function

Commit Message

Alexander Ivanov Feb. 3, 2023, 9:18 a.m. UTC

We will add more and more checks so we need a better code structure
in parallels_co_check. Let each check performs in a separate loop
in a separate helper.

Signed-off-by: Alexander Ivanov <[e-mail]>
---
 block/parallels.c | 80 ++++++++++++++++++++++++++++-------------------
 1 file changed, 48 insertions(+), 32 deletions(-)

Comments

Hanna Czenczek March 7, 2023, 12:17 p.m. UTC | #1

On 03.02.23 10:18, Alexander Ivanov wrote:
> We will add more and more checks so we need a better code structure
> in parallels_co_check. Let each check performs in a separate loop
> in a separate helper.
>
> Signed-off-by: Alexander Ivanov <[e-mail]>
> ---
>   block/parallels.c | 80 ++++++++++++++++++++++++++++-------------------
>   1 file changed, 48 insertions(+), 32 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 994528d93c..c7d37c4580 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -480,21 +480,57 @@ static int parallels_check_outside_image(BlockDriverState *bs,
>       return 0;
>   }
>   
> +static int parallels_check_leak(BlockDriverState *bs,
> +                                BdrvCheckResult *res,
> +                                BdrvCheckMode fix)

Like parallels_check_outside_image(), I think ideally this should be 
marked as `coroutine_fn GRAPH_RDLOCK` because of the block layer 
functions it calls.

Hanna

Patch

diff --git a/block/parallels.c b/block/parallels.c
index 994528d93c..c7d37c4580 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -480,21 +480,57 @@  static int parallels_check_outside_image(BlockDriverState *bs,
     return 0;
 }
 
+static int parallels_check_leak(BlockDriverState *bs,
+                                BdrvCheckResult *res,
+                                BdrvCheckMode fix)
+{
+    BDRVParallelsState *s = bs->opaque;
+    int64_t size;
+    int ret;
+
+    size = bdrv_getlength(bs->file->bs);
+    if (size < 0) {
+        res->check_errors++;
+        return size;
+    }
+
+    if (size > res->image_end_offset) {
+        int64_t count;
+        count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size);
+        fprintf(stderr, "%s space leaked at the end of the image %" PRId64 "\n",
+                fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR",
+                size - res->image_end_offset);
+        res->leaks += count;
+        if (fix & BDRV_FIX_LEAKS) {
+            Error *local_err = NULL;
+
+            /*
+             * In order to really repair the image, we must shrink it.
+             * That means we have to pass exact=true.
+             */
+            ret = bdrv_co_truncate(bs->file, res->image_end_offset, true,
+                                   PREALLOC_MODE_OFF, 0, &local_err);
+            if (ret < 0) {
+                error_report_err(local_err);
+                res->check_errors++;
+                return ret;
+            }
+            res->leaks_fixed += count;
+        }
+    }
+
+    return 0;
+}
+
 static int coroutine_fn parallels_co_check(BlockDriverState *bs,
                                            BdrvCheckResult *res,
                                            BdrvCheckMode fix)
 {
     BDRVParallelsState *s = bs->opaque;
-    int64_t size, prev_off;
+    int64_t prev_off;
     int ret;
     uint32_t i;
 
-    size = bdrv_getlength(bs->file->bs);
-    if (size < 0) {
-        res->check_errors++;
-        return size;
-    }
-
     qemu_co_mutex_lock(&s->lock);
 
     parallels_check_unclean(bs, res, fix);
@@ -504,6 +540,11 @@  static int coroutine_fn parallels_co_check(BlockDriverState *bs,
         goto out;
     }
 
+    ret = parallels_check_leak(bs, res, fix);
+    if (ret < 0) {
+        goto out;
+    }
+
     res->bfi.total_clusters = s->bat_size;
     res->bfi.compressed_clusters = 0; /* compression is not supported */
 
@@ -527,31 +568,6 @@  static int coroutine_fn parallels_co_check(BlockDriverState *bs,
         prev_off = off;
     }
 
-    if (size > res->image_end_offset) {
-        int64_t count;
-        count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size);
-        fprintf(stderr, "%s space leaked at the end of the image %" PRId64 "\n",
-                fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR",
-                size - res->image_end_offset);
-        res->leaks += count;
-        if (fix & BDRV_FIX_LEAKS) {
-            Error *local_err = NULL;
-
-            /*
-             * In order to really repair the image, we must shrink it.
-             * That means we have to pass exact=true.
-             */
-            ret = bdrv_co_truncate(bs->file, res->image_end_offset, true,
-                                   PREALLOC_MODE_OFF, 0, &local_err);
-            if (ret < 0) {
-                error_report_err(local_err);
-                res->check_errors++;
-                goto out;
-            }
-            res->leaks_fixed += count;
-        }
-    }
-
 out:
     qemu_co_mutex_unlock(&s->lock);