All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v10 0/4] Improve convert and dd commands
@ 2017-05-15 16:47 Daniel P. Berrange
  2017-05-15 16:47 ` [Qemu-devel] [PATCH v10 1/4] qemu-img: add support for --object with 'dd' command Daniel P. Berrange
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Daniel P. Berrange @ 2017-05-15 16:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Eric Blake, Kevin Wolf, Fam Zheng, Max Reitz,
	Daniel P. Berrange

Update to

  v1: https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05699.html
  v2: https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00728.html
  v3: https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg04391.html
  v4: https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg02153.html
  v5: https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg04109.html
  v6: https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg00215.html
  v7: https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg00242.html
  v8: https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg02028.html
  v9: https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03504.html

This series is in response to Max pointing out that you cannot
use 'convert' for an encrypted target image.

The 'convert' and 'dd' commands need to first create the image
and then open it. The bdrv_create() method takes a set of options
for creating the image, which let us provide a key-secret for the
encryption key. When the commands then open the new image, they
don't provide any options, so the image is unable to be opened
due to lack of encryption key. It is also not possible to use
the --image-opts argument to provide structured options in the
target image name - it must be a plain filename to satisfy the
bdrv_create() API contract.

This series addresses these problems to some extent

 - Adds a new --target-image-opts flag which is used to say
   that the target filename is using structured options.
   It is *only* permitted to use this when -n is also set.
   ie the target image must be pre-created so convert/dd
   don't need to run bdrv_create().

 - When --target-image-opts is not used, add special case
   code that identifies options passed to bdrv_create()
   named "*key-secret" and adds them to the options used
   to open the new image

In future it is desirable to make --target-image-opts work even when -n is
*not* given. This requires considerable work to create a new bdrv_create()
API impl.

The first patch fixes a bug in the 'dd' command while the second adds support
for the missing '--object' arg to 'dd', allowing it to reference secrets when
opening files.  The last two patches implement the new features described above
for the 'convert' command.

Changed in v10:

 - Fixed crash in qemu-img when locking flag is set, without
   fmt being set (seen when running iotest 153).

Changed in v9:

 - Rebased against Kevin's "block" branch with Fam's patch
   "qemu-img: Fix documentation of convert" applied first

Changed in v8:

 - Readd accidentally dropped check for compression (Max)
 - Fix indentation of variable declaration (Max)
 - Fix goto jump target (Max)

Changed in v7:

 - Drop the (accidentally included) revert patch (Eric)

Changed in v6:

 - Fix misc typos (Fam)
 - Resolve messy conflicts wrt max/block-next (Max)

Changed in v5:

 - Fix return value (Max)
 - Misc doc changes (Max)
 - Use error_abort (Max)

Changed in v4:

 - Refactor img_open_new_file in terms of img_open_file (Kevin)

Changed in v3:

 - Drop all patches affecting the 'dd' command except for the clear bug fix
   and the --object support. They can be re-considered once dd is rewritten
   to run ontop of convert.
 - Use consistent return/goto style in dd command (Max)
 - Fix error reporting when using compressed image and skip-create (Max)
 - Unconditionally create QDict when open files (Max)

Changed in v2:

 - Replace dd -n flag with support for conv=nocreat,notrunc
 - Misc typos (Eric, Fam)


Daniel P. Berrange (4):
  qemu-img: add support for --object with 'dd' command
  qemu-img: fix --image-opts usage with dd command
  qemu-img: introduce --target-image-opts for 'convert' command
  qemu-img: copy *key-secret opts when opening newly created files

 qemu-img-cmds.hx |   4 +-
 qemu-img.c       | 147 +++++++++++++++++++++++++++++++++++++++++++------------
 qemu-img.texi    |  12 ++++-
 3 files changed, 128 insertions(+), 35 deletions(-)

-- 
2.9.3

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

* [Qemu-devel] [PATCH v10 1/4] qemu-img: add support for --object with 'dd' command
  2017-05-15 16:47 [Qemu-devel] [PATCH v10 0/4] Improve convert and dd commands Daniel P. Berrange
@ 2017-05-15 16:47 ` Daniel P. Berrange
  2017-05-15 16:47 ` [Qemu-devel] [PATCH v10 2/4] qemu-img: fix --image-opts usage with dd command Daniel P. Berrange
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Daniel P. Berrange @ 2017-05-15 16:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Eric Blake, Kevin Wolf, Fam Zheng, Max Reitz,
	Daniel P. Berrange

The qemu-img dd command added --image-opts support, but missed
the corresponding --object support. This prevented passing
secrets (eg auth passwords) needed by certain disk images.

Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 qemu-img.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index b506839..181f499 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4158,6 +4158,7 @@ static int img_dd(int argc, char **argv)
     };
     const struct option long_options[] = {
         { "help", no_argument, 0, 'h'},
+        { "object", required_argument, 0, OPTION_OBJECT},
         { "image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
         { "force-share", no_argument, 0, 'U'},
         { 0, 0, 0, 0 }
@@ -4186,6 +4187,15 @@ static int img_dd(int argc, char **argv)
         case 'U':
             force_share = true;
             break;
+        case OPTION_OBJECT: {
+            QemuOpts *opts;
+            opts = qemu_opts_parse_noisily(&qemu_object_opts,
+                                           optarg, true);
+            if (!opts) {
+                ret = -1;
+                goto out;
+            }
+        }   break;
         case OPTION_IMAGE_OPTS:
             image_opts = true;
             break;
@@ -4230,6 +4240,14 @@ static int img_dd(int argc, char **argv)
         ret = -1;
         goto out;
     }
+
+    if (qemu_opts_foreach(&qemu_object_opts,
+                          user_creatable_add_opts_foreach,
+                          NULL, NULL)) {
+        ret = -1;
+        goto out;
+    }
+
     blk1 = img_open(image_opts, in.filename, fmt, 0, false, false,
                     force_share);
 
-- 
2.9.3

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

* [Qemu-devel] [PATCH v10 2/4] qemu-img: fix --image-opts usage with dd command
  2017-05-15 16:47 [Qemu-devel] [PATCH v10 0/4] Improve convert and dd commands Daniel P. Berrange
  2017-05-15 16:47 ` [Qemu-devel] [PATCH v10 1/4] qemu-img: add support for --object with 'dd' command Daniel P. Berrange
@ 2017-05-15 16:47 ` Daniel P. Berrange
  2017-05-15 16:47 ` [Qemu-devel] [PATCH v10 3/4] qemu-img: introduce --target-image-opts for 'convert' command Daniel P. Berrange
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Daniel P. Berrange @ 2017-05-15 16:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Eric Blake, Kevin Wolf, Fam Zheng, Max Reitz,
	Daniel P. Berrange

The --image-opts flag can only be used to affect the parsing
of the source image. The target image has to be specified in
the traditional style regardless, since it needs to be passed
to the bdrv_create() API which does not support the new style
opts.

Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 qemu-img.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 181f499..4dc1d56 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4316,8 +4316,13 @@ static int img_dd(int argc, char **argv)
         goto out;
     }
 
-    blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,
-                    false, false, false);
+    /* TODO, we can't honour --image-opts for the target,
+     * since it needs to be given in a format compatible
+     * with the bdrv_create() call above which does not
+     * support image-opts style.
+     */
+    blk2 = img_open_file(out.filename, out_fmt, BDRV_O_RDWR,
+                         false, false, false);
 
     if (!blk2) {
         ret = -1;
-- 
2.9.3

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

* [Qemu-devel] [PATCH v10 3/4] qemu-img: introduce --target-image-opts for 'convert' command
  2017-05-15 16:47 [Qemu-devel] [PATCH v10 0/4] Improve convert and dd commands Daniel P. Berrange
  2017-05-15 16:47 ` [Qemu-devel] [PATCH v10 1/4] qemu-img: add support for --object with 'dd' command Daniel P. Berrange
  2017-05-15 16:47 ` [Qemu-devel] [PATCH v10 2/4] qemu-img: fix --image-opts usage with dd command Daniel P. Berrange
@ 2017-05-15 16:47 ` Daniel P. Berrange
  2017-05-15 17:49   ` Eric Blake
  2017-05-15 16:47 ` [Qemu-devel] [PATCH v10 4/4] qemu-img: copy *key-secret opts when opening newly created files Daniel P. Berrange
  2017-05-15 17:50 ` [Qemu-devel] [PATCH v10 0/4] Improve convert and dd commands Max Reitz
  4 siblings, 1 reply; 7+ messages in thread
From: Daniel P. Berrange @ 2017-05-15 16:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Eric Blake, Kevin Wolf, Fam Zheng, Max Reitz,
	Daniel P. Berrange

The '--image-opts' flag indicates whether the source filename
includes options. The target filename has to remain in the
plain filename format though, since it needs to be passed to
bdrv_create().  When using --skip-create though, it would be
possible to use image-opts syntax. This adds --target-image-opts
to indicate that the target filename includes options. Currently
this mandates use of the --skip-create flag too.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 qemu-img-cmds.hx |  4 +--
 qemu-img.c       | 84 ++++++++++++++++++++++++++++++++++++++------------------
 qemu-img.texi    | 12 ++++++--
 3 files changed, 69 insertions(+), 31 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 469ec12..c298331 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -40,9 +40,9 @@ STEXI
 ETEXI
 
 DEF("convert", img_convert,
-    "convert [--object objectdef] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
+    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
 STEXI
-@item convert [--object @var{objectdef}] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}][-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}][-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("dd", img_dd,
diff --git a/qemu-img.c b/qemu-img.c
index 4dc1d56..e0e3d31 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -60,6 +60,7 @@ enum {
     OPTION_PATTERN = 260,
     OPTION_FLUSH_INTERVAL = 261,
     OPTION_NO_DRAIN = 262,
+    OPTION_TARGET_IMAGE_OPTS = 263,
 };
 
 typedef enum OutputFormat {
@@ -1913,10 +1914,10 @@ static int convert_do_copy(ImgConvertState *s)
 static int img_convert(int argc, char **argv)
 {
     int c, bs_i, flags, src_flags = 0;
-    const char *fmt = NULL, *out_fmt = "raw", *cache = "unsafe",
+    const char *fmt = NULL, *out_fmt = NULL, *cache = "unsafe",
                *src_cache = BDRV_DEFAULT_CACHE, *out_baseimg = NULL,
                *out_filename, *out_baseimg_param, *snapshot_name = NULL;
-    BlockDriver *drv, *proto_drv;
+    BlockDriver *drv = NULL, *proto_drv = NULL;
     BlockDriverInfo bdi;
     BlockDriverState *out_bs;
     QemuOpts *opts = NULL, *sn_opts = NULL;
@@ -1924,7 +1925,7 @@ static int img_convert(int argc, char **argv)
     char *options = NULL;
     Error *local_err = NULL;
     bool writethrough, src_writethrough, quiet = false, image_opts = false,
-         skip_create = false, progress = false;
+         skip_create = false, progress = false, tgt_image_opts = false;
     int64_t ret = -EINVAL;
     bool force_share = false;
 
@@ -1942,6 +1943,7 @@ static int img_convert(int argc, char **argv)
             {"object", required_argument, 0, OPTION_OBJECT},
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
             {"force-share", no_argument, 0, 'U'},
+            {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, ":hf:O:B:ce6o:s:l:S:pt:T:qnm:WU",
@@ -2062,9 +2064,16 @@ static int img_convert(int argc, char **argv)
         case OPTION_IMAGE_OPTS:
             image_opts = true;
             break;
+        case OPTION_TARGET_IMAGE_OPTS:
+            tgt_image_opts = true;
+            break;
         }
     }
 
+    if (!out_fmt && !tgt_image_opts) {
+        out_fmt = "raw";
+    }
+
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
                           NULL, NULL)) {
@@ -2076,12 +2085,22 @@ static int img_convert(int argc, char **argv)
         goto fail_getopt;
     }
 
+    if (tgt_image_opts && !skip_create) {
+        error_report("--target-image-opts requires use of -n flag");
+        goto fail_getopt;
+    }
+
     s.src_num = argc - optind - 1;
     out_filename = s.src_num >= 1 ? argv[argc - 1] : NULL;
 
     if (options && has_help_option(options)) {
-        ret = print_block_option_help(out_filename, out_fmt);
-        goto fail_getopt;
+        if (out_fmt) {
+            ret = print_block_option_help(out_filename, out_fmt);
+            goto fail_getopt;
+        } else {
+            error_report("Option help requires a format be specified");
+            goto fail_getopt;
+        }
     }
 
     if (s.src_num < 1) {
@@ -2146,22 +2165,22 @@ static int img_convert(int argc, char **argv)
         goto out;
     }
 
-    /* Find driver and parse its options */
-    drv = bdrv_find_format(out_fmt);
-    if (!drv) {
-        error_report("Unknown file format '%s'", out_fmt);
-        ret = -1;
-        goto out;
-    }
+    if (!skip_create) {
+        /* Find driver and parse its options */
+        drv = bdrv_find_format(out_fmt);
+        if (!drv) {
+            error_report("Unknown file format '%s'", out_fmt);
+            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;
-    }
+        proto_drv = bdrv_find_protocol(out_filename, true, &local_err);
+        if (!proto_drv) {
+            error_report_err(local_err);
+            ret = -1;
+            goto out;
+        }
 
-    if (!skip_create) {
         if (!drv->create_opts) {
             error_report("Format driver '%s' does not support image creation",
                          drv->format_name);
@@ -2218,7 +2237,7 @@ static int img_convert(int argc, char **argv)
         const char *preallocation =
             qemu_opt_get(opts, BLOCK_OPT_PREALLOC);
 
-        if (!drv->bdrv_co_pwritev_compressed) {
+        if (drv && !drv->bdrv_co_pwritev_compressed) {
             error_report("Compression not supported for this file format");
             ret = -1;
             goto out;
@@ -2258,19 +2277,30 @@ static int img_convert(int argc, char **argv)
         goto out;
     }
 
-    /* XXX we should allow --image-opts to trigger use of
-     * img_open() here, but then we have trouble with
-     * the bdrv_create() call which takes different params.
-     * Not critical right now, so fix can wait...
-     */
-    s.target = img_open_file(out_filename, out_fmt, flags, writethrough, quiet,
-                             false);
+    if (skip_create) {
+        s.target = img_open(tgt_image_opts, out_filename, out_fmt,
+                            flags, writethrough, quiet, false);
+    } else {
+        /* TODO ultimately we should allow --target-image-opts
+         * to be used even when -n is not given.
+         * That has to wait for bdrv_create to be improved
+         * to allow filenames in option syntax
+         */
+        s.target = img_open_file(out_filename, out_fmt, flags,
+                                 writethrough, quiet, false);
+    }
     if (!s.target) {
         ret = -1;
         goto out;
     }
     out_bs = blk_bs(s.target);
 
+    if (s.compressed && !out_bs->drv->bdrv_co_pwritev_compressed) {
+        error_report("Compression not supported for this file format");
+        ret = -1;
+        goto out;
+    }
+
     /* increase bufsectors from the default 4096 (2M) if opt_transfer
      * or discard_alignment of the out_bs is greater. Limit to 32768 (16MB)
      * as maximum. */
diff --git a/qemu-img.texi b/qemu-img.texi
index 50a2364..5b925ec 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -45,9 +45,17 @@ keys.
 
 @item --image-opts
 
-Indicates that the @var{filename} parameter is to be interpreted as a
+Indicates that the source @var{filename} parameter is to be interpreted as a
 full option string, not a plain filename. This parameter is mutually
-exclusive with the @var{-f} and @var{-F} parameters.
+exclusive with the @var{-f} parameter.
+
+@item --target-image-opts
+
+Indicates that the @var{output_filename} parameter(s) are to be interpreted as
+a full option string, not a plain filename. This parameter is mutually
+exclusive with the @var{-O} parameters. It is currently required to also use
+the @var{-n} parameter to skip image creation. This restriction may be relaxed
+in a future release.
 
 @item fmt
 is the disk image format. It is guessed automatically in most cases. See below
-- 
2.9.3

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

* [Qemu-devel] [PATCH v10 4/4] qemu-img: copy *key-secret opts when opening newly created files
  2017-05-15 16:47 [Qemu-devel] [PATCH v10 0/4] Improve convert and dd commands Daniel P. Berrange
                   ` (2 preceding siblings ...)
  2017-05-15 16:47 ` [Qemu-devel] [PATCH v10 3/4] qemu-img: introduce --target-image-opts for 'convert' command Daniel P. Berrange
@ 2017-05-15 16:47 ` Daniel P. Berrange
  2017-05-15 17:50 ` [Qemu-devel] [PATCH v10 0/4] Improve convert and dd commands Max Reitz
  4 siblings, 0 replies; 7+ messages in thread
From: Daniel P. Berrange @ 2017-05-15 16:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Eric Blake, Kevin Wolf, Fam Zheng, Max Reitz,
	Daniel P. Berrange

The qemu-img dd/convert commands will create an image file and
then try to open it. Historically it has been possible to open
new files without passing any options. With encrypted files
though, the *key-secret options are mandatory, so we need to
provide those options when opening the newly created file.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 qemu-img.c | 42 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index e0e3d31..0bf941b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -314,14 +314,17 @@ static BlockBackend *img_open_opts(const char *optstr,
 }
 
 static BlockBackend *img_open_file(const char *filename,
+                                   QDict *options,
                                    const char *fmt, int flags,
                                    bool writethrough, bool quiet,
                                    bool force_share)
 {
     BlockBackend *blk;
     Error *local_err = NULL;
-    QDict *options = qdict_new();
 
+    if (!options) {
+        options = qdict_new();
+    }
     if (fmt) {
         qdict_put_str(options, "driver", fmt);
     }
@@ -344,6 +347,35 @@ static BlockBackend *img_open_file(const char *filename,
 }
 
 
+static int img_add_key_secrets(void *opaque,
+                               const char *name, const char *value,
+                               Error **errp)
+{
+    QDict *options = opaque;
+
+    if (g_str_has_suffix(name, "key-secret")) {
+        qdict_put(options, name, qstring_from_str(value));
+    }
+
+    return 0;
+}
+
+static BlockBackend *img_open_new_file(const char *filename,
+                                       QemuOpts *create_opts,
+                                       const char *fmt, int flags,
+                                       bool writethrough, bool quiet,
+                                       bool force_share)
+{
+    QDict *options = NULL;
+
+    options = qdict_new();
+    qemu_opt_foreach(create_opts, img_add_key_secrets, options, &error_abort);
+
+    return img_open_file(filename, options, fmt, flags, writethrough, quiet,
+                         force_share);
+}
+
+
 static BlockBackend *img_open(bool image_opts,
                               const char *filename,
                               const char *fmt, int flags, bool writethrough,
@@ -364,7 +396,7 @@ static BlockBackend *img_open(bool image_opts,
         blk = img_open_opts(filename, opts, flags, writethrough, quiet,
                             force_share);
     } else {
-        blk = img_open_file(filename, fmt, flags, writethrough, quiet,
+        blk = img_open_file(filename, NULL, fmt, flags, writethrough, quiet,
                             force_share);
     }
     return blk;
@@ -2286,8 +2318,8 @@ static int img_convert(int argc, char **argv)
          * That has to wait for bdrv_create to be improved
          * to allow filenames in option syntax
          */
-        s.target = img_open_file(out_filename, out_fmt, flags,
-                                 writethrough, quiet, false);
+        s.target = img_open_new_file(out_filename, opts, out_fmt,
+                                     flags, writethrough, quiet, false);
     }
     if (!s.target) {
         ret = -1;
@@ -4351,7 +4383,7 @@ static int img_dd(int argc, char **argv)
      * with the bdrv_create() call above which does not
      * support image-opts style.
      */
-    blk2 = img_open_file(out.filename, out_fmt, BDRV_O_RDWR,
+    blk2 = img_open_file(out.filename, NULL, out_fmt, BDRV_O_RDWR,
                          false, false, false);
 
     if (!blk2) {
-- 
2.9.3

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

* Re: [Qemu-devel] [PATCH v10 3/4] qemu-img: introduce --target-image-opts for 'convert' command
  2017-05-15 16:47 ` [Qemu-devel] [PATCH v10 3/4] qemu-img: introduce --target-image-opts for 'convert' command Daniel P. Berrange
@ 2017-05-15 17:49   ` Eric Blake
  0 siblings, 0 replies; 7+ messages in thread
From: Eric Blake @ 2017-05-15 17:49 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel
  Cc: qemu-block, Kevin Wolf, Fam Zheng, Max Reitz

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

On 05/15/2017 11:47 AM, Daniel P. Berrange wrote:
> The '--image-opts' flag indicates whether the source filename
> includes options. The target filename has to remain in the
> plain filename format though, since it needs to be passed to
> bdrv_create().  When using --skip-create though, it would be
> possible to use image-opts syntax. This adds --target-image-opts
> to indicate that the target filename includes options. Currently
> this mandates use of the --skip-create flag too.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  qemu-img-cmds.hx |  4 +--
>  qemu-img.c       | 84 ++++++++++++++++++++++++++++++++++++++------------------
>  qemu-img.texi    | 12 ++++++--
>  3 files changed, 69 insertions(+), 31 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

* Re: [Qemu-devel] [PATCH v10 0/4] Improve convert and dd commands
  2017-05-15 16:47 [Qemu-devel] [PATCH v10 0/4] Improve convert and dd commands Daniel P. Berrange
                   ` (3 preceding siblings ...)
  2017-05-15 16:47 ` [Qemu-devel] [PATCH v10 4/4] qemu-img: copy *key-secret opts when opening newly created files Daniel P. Berrange
@ 2017-05-15 17:50 ` Max Reitz
  4 siblings, 0 replies; 7+ messages in thread
From: Max Reitz @ 2017-05-15 17:50 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel
  Cc: qemu-block, Eric Blake, Kevin Wolf, Fam Zheng

[-- Attachment #1: Type: text/plain, Size: 2609 bytes --]

On 2017-05-15 18:47, Daniel P. Berrange wrote:
> Update to
> 
>   v1: https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05699.html
>   v2: https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00728.html
>   v3: https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg04391.html
>   v4: https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg02153.html
>   v5: https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg04109.html
>   v6: https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg00215.html
>   v7: https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg00242.html
>   v8: https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg02028.html
>   v9: https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03504.html
> 
> This series is in response to Max pointing out that you cannot
> use 'convert' for an encrypted target image.
> 
> The 'convert' and 'dd' commands need to first create the image
> and then open it. The bdrv_create() method takes a set of options
> for creating the image, which let us provide a key-secret for the
> encryption key. When the commands then open the new image, they
> don't provide any options, so the image is unable to be opened
> due to lack of encryption key. It is also not possible to use
> the --image-opts argument to provide structured options in the
> target image name - it must be a plain filename to satisfy the
> bdrv_create() API contract.
> 
> This series addresses these problems to some extent
> 
>  - Adds a new --target-image-opts flag which is used to say
>    that the target filename is using structured options.
>    It is *only* permitted to use this when -n is also set.
>    ie the target image must be pre-created so convert/dd
>    don't need to run bdrv_create().
> 
>  - When --target-image-opts is not used, add special case
>    code that identifies options passed to bdrv_create()
>    named "*key-secret" and adds them to the options used
>    to open the new image
> 
> In future it is desirable to make --target-image-opts work even when -n is
> *not* given. This requires considerable work to create a new bdrv_create()
> API impl.
> 
> The first patch fixes a bug in the 'dd' command while the second adds support
> for the missing '--object' arg to 'dd', allowing it to reference secrets when
> opening files.  The last two patches implement the new features described above
> for the 'convert' command.

Thanks,

Reviewed-by: Max Reitz <mreitz@redhat.com>

...and applied to my block branch:

https://github.com/XanClic/qemu/commits/block

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

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

end of thread, other threads:[~2017-05-15 17:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-15 16:47 [Qemu-devel] [PATCH v10 0/4] Improve convert and dd commands Daniel P. Berrange
2017-05-15 16:47 ` [Qemu-devel] [PATCH v10 1/4] qemu-img: add support for --object with 'dd' command Daniel P. Berrange
2017-05-15 16:47 ` [Qemu-devel] [PATCH v10 2/4] qemu-img: fix --image-opts usage with dd command Daniel P. Berrange
2017-05-15 16:47 ` [Qemu-devel] [PATCH v10 3/4] qemu-img: introduce --target-image-opts for 'convert' command Daniel P. Berrange
2017-05-15 17:49   ` Eric Blake
2017-05-15 16:47 ` [Qemu-devel] [PATCH v10 4/4] qemu-img: copy *key-secret opts when opening newly created files Daniel P. Berrange
2017-05-15 17:50 ` [Qemu-devel] [PATCH v10 0/4] Improve convert and dd commands Max Reitz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.