diff mbox

qemu-img: change opening method for the output in dd

Message ID 20160811040232.30905-1-fullmanet@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Reda Sallahi Aug. 11, 2016, 4:02 a.m. UTC 
dd was creating an output image regardless of whether there was one already
created. With this patch we try to open first the output image and resize it
if necessary.

Signed-off-by: Reda Sallahi <fullmanet@gmail.com>
---
Depends on:
[PATCH v2] qemu-img: add conv=notrunc option to dd

 qemu-img.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 76 insertions(+), 15 deletions(-)

Comments

Fam Zheng Aug. 11, 2016, 8:53 a.m. UTC | #1 
On Thu, 08/11 06:02, Reda Sallahi wrote:
> dd was creating an output image regardless of whether there was one already
> created. With this patch we try to open first the output image and resize it
> if necessary.
> 
> Signed-off-by: Reda Sallahi <fullmanet@gmail.com>
> ---
> Depends on:
> [PATCH v2] qemu-img: add conv=notrunc option to dd
> 
>  qemu-img.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 76 insertions(+), 15 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 7c546c1..dfa0e63 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -3919,6 +3919,8 @@ static int img_dd(int argc, char **argv)
>      char *tmp;
>      BlockDriver *drv = NULL, *proto_drv = NULL;
>      BlockBackend *blk1 = NULL, *blk2 = NULL;
> +    BlockDriverState *bs = NULL;
> +    QDict *qoptions = NULL;
>      QemuOpts *opts = NULL;
>      QemuOptsList *create_opts = NULL;
>      Error *local_err = NULL;
> @@ -4080,22 +4082,60 @@ static int img_dd(int argc, char **argv)
>          size = dd.count * in.bsz;
>      }
>  
> -    /* Overflow means the specified offset is beyond input image's size */
> -    if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||
> -                              size < in.bsz * in.offset)) {
> -        qemu_opt_set_number(opts, BLOCK_OPT_SIZE, 0, &error_abort);
> -    } else {
> -        qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
> -                            size - in.bsz * in.offset, &error_abort);
> -    }
> +    qoptions = qdict_new();
> +    qdict_put(qoptions, "driver", qstring_from_str(out_fmt));
>  
> -    ret = bdrv_create(drv, out.filename, opts, &local_err);
> -    if (ret < 0) {
> -        error_reportf_err(local_err,
> -                          "%s: error while creating output image: ",
> -                          out.filename);
> -        ret = -1;
> -        goto out;
> +    bs = bdrv_open(out.filename, NULL, qoptions, BDRV_O_RDWR, &local_err);

What about --image-opts? With that we need to open it like img_open.

> +
> +    if (!bs) {
> +        drv = bdrv_find_format(out_fmt);
> +        if (!drv) {
> +            error_report("Unknown file format");
> +            ret = -1;
> +            goto out;
> +        }
> +        proto_drv = bdrv_find_protocol(out.filename, true, &local_err);

local_err is likely set here, you cannot pass it again without freeing it
first.

> +
> +        if (!proto_drv) {
> +            error_report_err(local_err);
> +            ret = -1;
> +            goto out;
> +        }
> +        if (!drv->create_opts) {
> +            error_report("Format driver '%s' does not support image creation",
> +                         drv->format_name);
> +            ret = -1;
> +            goto out;
> +        }
> +        if (!proto_drv->create_opts) {
> +            error_report("Protocol driver '%s' does not support image creation",
> +                         proto_drv->format_name);
> +            ret = -1;
> +            goto out;
> +        }
> +        create_opts = qemu_opts_append(create_opts, drv->create_opts);
> +        create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
> +
> +        opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
> +
> +        /* Overflow means the specified offset is beyond input image's size */
> +        if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||
> +                                  size < in.offset * in.bsz)) {
> +            qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
> +                                0, &error_abort);
> +        } else {
> +            qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
> +                                size - in.offset * in.bsz, &error_abort);
> +        }
> +
> +        ret = bdrv_create(drv, out.filename, opts, &local_err);
> +        if (ret < 0) {
> +            error_reportf_err(local_err,
> +                              "%s: error while creating output image: ",
> +                              out.filename);
> +            ret = -1;
> +            goto out;
> +        }
>      }
>  
>      blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,

Isn't this duplicated to the above bdrv_open? Maybe they should be unified to
one.

> @@ -4117,6 +4157,26 @@ static int img_dd(int argc, char **argv)
>          in_pos = in.offset * in.bsz;
>      }
>  
> +    if (bs) {
> +        int64_t blk2sz = blk_getlength(blk2);
> +        if (blk2sz < 0) {
> +            error_report("Failed to get size for '%s'", in.filename);
> +            ret = -1;
> +            goto out;
> +        }
> +
> +        if (!(dd.conv & C_NOTRUNC)) {
> +            blk_truncate(blk2, 0);
> +        }
> +        if (!(dd.flags & C_SKIP) || (in.offset <= INT64_MAX / in.bsz &&
> +                                  size >= in.offset * in.bsz)) {
> +            if (!(dd.conv & C_NOTRUNC) ||
> +                blk2sz < size - in.offset * in.bsz) {
> +                blk_truncate(blk2, size - in.offset * in.bsz);
> +            }
> +        }
> +    }
> +
>      in.buf = g_new(uint8_t, in.bsz);
>  
>      for (out_pos = 0; in_pos < size; block_count++) {
> @@ -4152,6 +4212,7 @@ out:
>      qemu_opts_free(create_opts);
>      blk_unref(blk1);
>      blk_unref(blk2);
> +    bdrv_unref(bs);
>      g_free(in.filename);
>      g_free(out.filename);
>      g_free(in.buf);
> -- 
> 2.9.2
> 

Fam
Stefan Hajnoczi Aug. 11, 2016, 9 a.m. UTC | #2 
On Thu, Aug 11, 2016 at 06:02:32AM +0200, Reda Sallahi wrote:
> +    bs = bdrv_open(out.filename, NULL, qoptions, BDRV_O_RDWR, &local_err);

Why are bdrv_*() functions used instead of blk_*()?
diff mbox

Patch

diff --git a/qemu-img.c b/qemu-img.c
index 7c546c1..dfa0e63 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3919,6 +3919,8 @@  static int img_dd(int argc, char **argv)
     char *tmp;
     BlockDriver *drv = NULL, *proto_drv = NULL;
     BlockBackend *blk1 = NULL, *blk2 = NULL;
+    BlockDriverState *bs = NULL;
+    QDict *qoptions = NULL;
     QemuOpts *opts = NULL;
     QemuOptsList *create_opts = NULL;
     Error *local_err = NULL;
@@ -4080,22 +4082,60 @@  static int img_dd(int argc, char **argv)
         size = dd.count * in.bsz;
     }
 
-    /* Overflow means the specified offset is beyond input image's size */
-    if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||
-                              size < in.bsz * in.offset)) {
-        qemu_opt_set_number(opts, BLOCK_OPT_SIZE, 0, &error_abort);
-    } else {
-        qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
-                            size - in.bsz * in.offset, &error_abort);
-    }
+    qoptions = qdict_new();
+    qdict_put(qoptions, "driver", qstring_from_str(out_fmt));
 
-    ret = bdrv_create(drv, out.filename, opts, &local_err);
-    if (ret < 0) {
-        error_reportf_err(local_err,
-                          "%s: error while creating output image: ",
-                          out.filename);
-        ret = -1;
-        goto out;
+    bs = bdrv_open(out.filename, NULL, qoptions, BDRV_O_RDWR, &local_err);
+
+    if (!bs) {
+        drv = bdrv_find_format(out_fmt);
+        if (!drv) {
+            error_report("Unknown file format");
+            ret = -1;
+            goto out;
+        }
+        proto_drv = bdrv_find_protocol(out.filename, true, &local_err);
+
+        if (!proto_drv) {
+            error_report_err(local_err);
+            ret = -1;
+            goto out;
+        }
+        if (!drv->create_opts) {
+            error_report("Format driver '%s' does not support image creation",
+                         drv->format_name);
+            ret = -1;
+            goto out;
+        }
+        if (!proto_drv->create_opts) {
+            error_report("Protocol driver '%s' does not support image creation",
+                         proto_drv->format_name);
+            ret = -1;
+            goto out;
+        }
+        create_opts = qemu_opts_append(create_opts, drv->create_opts);
+        create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
+
+        opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
+
+        /* Overflow means the specified offset is beyond input image's size */
+        if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||
+                                  size < in.offset * in.bsz)) {
+            qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
+                                0, &error_abort);
+        } else {
+            qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
+                                size - in.offset * in.bsz, &error_abort);
+        }
+
+        ret = bdrv_create(drv, out.filename, opts, &local_err);
+        if (ret < 0) {
+            error_reportf_err(local_err,
+                              "%s: error while creating output image: ",
+                              out.filename);
+            ret = -1;
+            goto out;
+        }
     }
 
     blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,
@@ -4117,6 +4157,26 @@  static int img_dd(int argc, char **argv)
         in_pos = in.offset * in.bsz;
     }
 
+    if (bs) {
+        int64_t blk2sz = blk_getlength(blk2);
+        if (blk2sz < 0) {
+            error_report("Failed to get size for '%s'", in.filename);
+            ret = -1;
+            goto out;
+        }
+
+        if (!(dd.conv & C_NOTRUNC)) {
+            blk_truncate(blk2, 0);
+        }
+        if (!(dd.flags & C_SKIP) || (in.offset <= INT64_MAX / in.bsz &&
+                                  size >= in.offset * in.bsz)) {
+            if (!(dd.conv & C_NOTRUNC) ||
+                blk2sz < size - in.offset * in.bsz) {
+                blk_truncate(blk2, size - in.offset * in.bsz);
+            }
+        }
+    }
+
     in.buf = g_new(uint8_t, in.bsz);
 
     for (out_pos = 0; in_pos < size; block_count++) {
@@ -4152,6 +4212,7 @@  out:
     qemu_opts_free(create_opts);
     blk_unref(blk1);
     blk_unref(blk2);
+    bdrv_unref(bs);
     g_free(in.filename);
     g_free(out.filename);
     g_free(in.buf);