[Qemu-devel] [PATCH] qemu-img: implement copy offload (-C) for dd

* [Qemu-devel] [PATCH] qemu-img: implement copy offload (-C) for dd
@ 2019-02-21 17:37 Sergio Lopez
  2019-02-21 18:08 ` Eric Blake
  0 siblings, 1 reply; 7+ messages in thread
From: Sergio Lopez @ 2019-02-21 17:37 UTC (permalink / raw)
  To: qemu-block; +Cc: qemu-devel, kwolf, mreitz, Sergio Lopez

This parameter is analogous to convert's "-C", making use of
bdrv_co_copy_range().

Signed-off-by: Sergio Lopez <slp@redhat.com>
---
 qemu-img-cmds.hx |   4 +-
 qemu-img.c       | 146 ++++++++++++++++++++++++++++++++++++-----------
 qemu-img.texi    |   2 +-
 3 files changed, 117 insertions(+), 35 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 1526f327a5..7b85d183e5 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -56,9 +56,9 @@ STEXI
 ETEXI
 
 DEF("dd", img_dd,
-    "dd [--image-opts] [-U] [-f fmt] [-O output_fmt] [bs=block_size] [count=blocks] [skip=blocks] if=input of=output")
+    "dd [--image-opts] [-U] [-C] [-f fmt] [-O output_fmt] [bs=block_size] [count=blocks] [skip=blocks] if=input of=output")
 STEXI
-@item dd [--image-opts] [-U] [-f @var{fmt}] [-O @var{output_fmt}] [bs=@var{block_size}] [count=@var{blocks}] [skip=@var{blocks}] if=@var{input} of=@var{output}
+@item dd [--image-opts] [-U] [-C] [-f @var{fmt}] [-O @var{output_fmt}] [bs=@var{block_size}] [count=@var{blocks}] [skip=@var{blocks}] if=@var{input} of=@var{output}
 ETEXI
 
 DEF("info", img_info,
diff --git a/qemu-img.c b/qemu-img.c
index 25288c4d18..75ac0318a1 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4310,7 +4310,6 @@ struct DdInfo {
 struct DdIo {
     int bsz;    /* Block size */
     char *filename;
-    uint8_t *buf;
     int64_t offset;
 };
 
@@ -4320,6 +4319,18 @@ struct DdOpts {
     unsigned int flag;
 };
 
+typedef struct ImgDdState {
+    BlockBackend *src;
+    int64_t src_offset;
+    BlockBackend *dst;
+    int64_t dst_offset;
+    int bsz;
+    int64_t size;
+    bool copy_range;
+    int running_coroutines;
+    int ret;
+} ImgDdState;
+
 static int img_dd_bs(const char *arg,
                      struct DdIo *in, struct DdIo *out,
                      struct DdInfo *dd)
@@ -4383,6 +4394,86 @@ static int img_dd_skip(const char *arg,
     return 0;
 }
 
+static void coroutine_fn dd_co_do_copy(void *opaque)
+{
+    ImgDdState *s = opaque;
+    int64_t block_count, in_pos, out_pos;
+    uint8_t *buf;
+    int ret = 0;
+
+    s->running_coroutines++;
+
+    in_pos = s->src_offset;
+    out_pos = s->dst_offset;
+
+    if (s->copy_range) {
+        /*
+         * We don't impose any kind of alignment requirements nor
+         * adjustments here. It's up to the user to set the best
+         * parameters for a particular storage/driver.
+         */
+        for (; in_pos < s->size && ret == 0; block_count++) {
+            int bytes = s->bsz;
+
+            if (in_pos + bytes > s->size) {
+                bytes = s->size - in_pos;
+            }
+
+            ret = blk_co_copy_range(s->src, in_pos,
+                                    s->dst, out_pos,
+                                    bytes, 0, 0);
+            in_pos += bytes;
+            out_pos += bytes;
+        }
+
+        /*
+         * If copy_range succeded, jump to the end. Otherwise,
+         * fall through to try with the usual bouncing buffers.
+         */
+        if (ret == 0) {
+            goto out;
+        } else {
+            warn_report("copy_range failed, continuing with the conventional "
+                        "read/write approach.");
+        }
+    }
+
+    buf = g_new(uint8_t, s->bsz);
+
+    for (; in_pos < s->size; block_count++) {
+        int in_ret, out_ret;
+
+        if (in_pos + s->bsz > s->size) {
+            in_ret = blk_pread(s->src, in_pos, buf, s->size - in_pos);
+        } else {
+            in_ret = blk_pread(s->src, in_pos, buf, s->bsz);
+        }
+        if (in_ret < 0) {
+            error_report("error while reading from input image file: %s",
+                         strerror(-in_ret));
+            ret = -1;
+            goto free_out;
+        }
+        in_pos += in_ret;
+
+        out_ret = blk_pwrite(s->dst, out_pos, buf, in_ret, 0);
+
+        if (out_ret < 0) {
+            error_report("error while writing to output image file: %s",
+                         strerror(-out_ret));
+            ret = -1;
+            goto free_out;
+        }
+        out_pos += out_ret;
+    }
+
+ free_out:
+    g_free(buf);
+ out:
+    s->running_coroutines--;
+    s->ret = ret;
+}
+
 static int img_dd(int argc, char **argv)
 {
     int ret = 0;
@@ -4390,6 +4481,7 @@ static int img_dd(int argc, char **argv)
     char *tmp;
     BlockDriver *drv = NULL, *proto_drv = NULL;
     BlockBackend *blk1 = NULL, *blk2 = NULL;
+    Coroutine *co = NULL;
     QemuOpts *opts = NULL;
     QemuOptsList *create_opts = NULL;
     Error *local_err = NULL;
@@ -4398,8 +4490,9 @@ static int img_dd(int argc, char **argv)
     const char *out_fmt = "raw";
     const char *fmt = NULL;
     int64_t size = 0;
-    int64_t block_count = 0, out_pos, in_pos;
+    int64_t in_pos;
     bool force_share = false;
+    bool copy_range = false;
     struct DdInfo dd = {
         .flags = 0,
         .count = 0,
@@ -4407,13 +4500,11 @@ static int img_dd(int argc, char **argv)
     struct DdIo in = {
         .bsz = 512, /* Block size is by default 512 bytes */
         .filename = NULL,
-        .buf = NULL,
         .offset = 0
     };
     struct DdIo out = {
         .bsz = 512,
         .filename = NULL,
-        .buf = NULL,
         .offset = 0
     };
 
@@ -4433,7 +4524,7 @@ static int img_dd(int argc, char **argv)
         { 0, 0, 0, 0 }
     };
 
-    while ((c = getopt_long(argc, argv, ":hf:O:U", long_options, NULL))) {
+    while ((c = getopt_long(argc, argv, ":hf:O:UC", long_options, NULL))) {
         if (c == EOF) {
             break;
         }
@@ -4456,6 +4547,9 @@ static int img_dd(int argc, char **argv)
         case 'U':
             force_share = true;
             break;
+        case 'C':
+            copy_range = true;
+            break;
         case OPTION_OBJECT:
             if (!qemu_opts_parse_noisily(&qemu_object_opts, optarg, true)) {
                 ret = -1;
@@ -4606,35 +4700,25 @@ static int img_dd(int argc, char **argv)
         in_pos = in.offset * in.bsz;
     }
 
-    in.buf = g_new(uint8_t, in.bsz);
-
-    for (out_pos = 0; in_pos < size; block_count++) {
-        int in_ret, out_ret;
-
-        if (in_pos + in.bsz > size) {
-            in_ret = blk_pread(blk1, in_pos, in.buf, size - in_pos);
-        } else {
-            in_ret = blk_pread(blk1, in_pos, in.buf, in.bsz);
-        }
-        if (in_ret < 0) {
-            error_report("error while reading from input image file: %s",
-                         strerror(-in_ret));
-            ret = -1;
-            goto out;
-        }
-        in_pos += in_ret;
+    ImgDdState s = (ImgDdState) {
+        .src = blk1,
+        .src_offset = in_pos,
+        .dst = blk2,
+        .dst_offset = 0,
+        .bsz = in.bsz,
+        .size = size,
+        .copy_range = copy_range,
+    };
 
-        out_ret = blk_pwrite(blk2, out_pos, in.buf, in_ret, 0);
+    co = qemu_coroutine_create(dd_co_do_copy, &s);
+    qemu_coroutine_enter(co);
 
-        if (out_ret < 0) {
-            error_report("error while writing to output image file: %s",
-                         strerror(-out_ret));
-            ret = -1;
-            goto out;
-        }
-        out_pos += out_ret;
+    while (s.running_coroutines) {
+        main_loop_wait(false);
     }
 
+    ret = s.ret;
+
 out:
     g_free(arg);
     qemu_opts_del(opts);
@@ -4643,8 +4727,6 @@ out:
     blk_unref(blk2);
     g_free(in.filename);
     g_free(out.filename);
-    g_free(in.buf);
-    g_free(out.buf);
 
     if (ret) {
         return 1;
diff --git a/qemu-img.texi b/qemu-img.texi
index 3b6710a580..5c5079e3bc 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -390,7 +390,7 @@ way.
 The size can also be specified using the @var{size} option with @code{-o},
 it doesn't need to be specified separately in this case.
 
-@item dd [--image-opts] [-U] [-f @var{fmt}] [-O @var{output_fmt}] [bs=@var{block_size}] [count=@var{blocks}] [skip=@var{blocks}] if=@var{input} of=@var{output}
+@item dd [--image-opts] [-U] [-C] [-f @var{fmt}] [-O @var{output_fmt}] [bs=@var{block_size}] [count=@var{blocks}] [skip=@var{blocks}] if=@var{input} of=@var{output}
 
 Dd copies from @var{input} file to @var{output} file converting it from
 @var{fmt} format to @var{output_fmt} format.
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread