[03/54] mirror: Resize active commit base in mirror_run()

Commit Message

Kevin Wolf Feb. 21, 2017, 2:57 p.m. UTC

This is more consistent with the commit block job, and it moves the code
to a place where we already have the necessary BlockBackends to resize
the base image when bdrv_truncate() is changed to require a BdrvChild.

Signed-off-by: Kevin Wolf <[e-mail]>
---
 block/mirror.c | 50 ++++++++++++++++++++++----------------------------
 1 file changed, 22 insertions(+), 28 deletions(-)

Comments

Max Reitz Feb. 22, 2017, 12:34 p.m. UTC | #1

On 21.02.2017 15:57, Kevin Wolf wrote:
> This is more consistent with the commit block job, and it moves the code
> to a place where we already have the necessary BlockBackends to resize
> the base image when bdrv_truncate() is changed to require a BdrvChild.
> 
> Signed-off-by: Kevin Wolf <[e-mail]>
> ---
>  block/mirror.c | 50 ++++++++++++++++++++++----------------------------
>  1 file changed, 22 insertions(+), 28 deletions(-)

Reviewed-by: Max Reitz <[e-mail]>

Fam Zheng Feb. 23, 2017, 11:31 a.m. UTC | #2

On Tue, 02/21 15:57, Kevin Wolf wrote:
> diff --git a/block/mirror.c b/block/mirror.c
> index 698a54e..13f793e 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -657,7 +657,28 @@ static void coroutine_fn mirror_run(void *opaque)
>      if (s->bdev_length < 0) {
>          ret = s->bdev_length;
>          goto immediate_exit;
> -    } else if (s->bdev_length == 0) {
> +    }
> +
> +    /* Active commit must resize the base image if its size differs from the
> +     * active layer. */
> +    if (s->base == blk_bs(s->target)) {
> +        int64_t base_length;
> +
> +        base_length = blk_getlength(s->target);
> +        if (base_length < 0) {
> +            ret = base_length;
> +            goto immediate_exit;
> +        }
> +
> +        if (s->bdev_length > base_length) {
> +            ret = blk_truncate(s->target, s->bdev_length);
> +            if (ret < 0) {
> +                goto immediate_exit;
> +            }
> +        }
> +    }
> +
> +    if (s->bdev_length == 0) {
>          /* Report BLOCK_JOB_READY and wait for complete. */
>          block_job_event_ready(&s->common);
>          s->synced = true;

This has a slight behavior change of commit job that previously upon resize
failure the job wouldn't start, but now it will, then immediately end with an
error. Not an problem, though.

Fam

Patch

diff --git a/block/mirror.c b/block/mirror.c
index 698a54e..13f793e 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -657,7 +657,28 @@  static void coroutine_fn mirror_run(void *opaque)
     if (s->bdev_length < 0) {
         ret = s->bdev_length;
         goto immediate_exit;
-    } else if (s->bdev_length == 0) {
+    }
+
+    /* Active commit must resize the base image if its size differs from the
+     * active layer. */
+    if (s->base == blk_bs(s->target)) {
+        int64_t base_length;
+
+        base_length = blk_getlength(s->target);
+        if (base_length < 0) {
+            ret = base_length;
+            goto immediate_exit;
+        }
+
+        if (s->bdev_length > base_length) {
+            ret = blk_truncate(s->target, s->bdev_length);
+            if (ret < 0) {
+                goto immediate_exit;
+            }
+        }
+    }
+
+    if (s->bdev_length == 0) {
         /* Report BLOCK_JOB_READY and wait for complete. */
         block_job_event_ready(&s->common);
         s->synced = true;
@@ -1058,9 +1079,7 @@  void commit_active_start(const char *job_id, BlockDriverState *bs,
                          BlockCompletionFunc *cb, void *opaque, Error **errp,
                          bool auto_complete)
 {
-    int64_t length, base_length;
     int orig_base_flags;
-    int ret;
     Error *local_err = NULL;
 
     orig_base_flags = bdrv_get_flags(base);
@@ -1069,31 +1088,6 @@  void commit_active_start(const char *job_id, BlockDriverState *bs,
         return;
     }
 
-    length = bdrv_getlength(bs);
-    if (length < 0) {
-        error_setg_errno(errp, -length,
-                         "Unable to determine length of %s", bs->filename);
-        goto error_restore_flags;
-    }
-
-    base_length = bdrv_getlength(base);
-    if (base_length < 0) {
-        error_setg_errno(errp, -base_length,
-                         "Unable to determine length of %s", base->filename);
-        goto error_restore_flags;
-    }
-
-    if (length > base_length) {
-        ret = bdrv_truncate(base, length);
-        if (ret < 0) {
-            error_setg_errno(errp, -ret,
-                            "Top image %s is larger than base image %s, and "
-                             "resize of base image failed",
-                             bs->filename, base->filename);
-            goto error_restore_flags;
-        }
-    }
-
     mirror_start_job(job_id, bs, creation_flags, base, NULL, speed, 0, 0,
                      MIRROR_LEAVE_BACKING_CHAIN,
                      on_error, on_error, true, cb, opaque, &local_err,