Improve convert and dd commands

The '--image-opts' flags 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.

Quoted 1 line · Daniel P. Berrange

The '--image-opts' flags indicates whether the source filename

s/flags/flag/ or s/indicates/indicate/, I think?

Quoted 6 lines

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.

The '--image-opts' flags 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.

Reviewed-by Eric Blake <[email protected]>

Signed-off-by Daniel P. Berrange <[email protected]>

---
 qemu-img-cmds.hx |  4 +--
 qemu-img.c       | 86 ++++++++++++++++++++++++++++++++++++++++----------------
 qemu-img.texi    | 12 ++++++--
 3 files changed, 73 insertions(+), 29 deletions(-)

qemu-img-cmds.hx +0 -0

index 8ac7822..93b50ef 100644

qemu-img-cmds.hx +3 -0

+        goto fail_getopt;
+    }
+
     /* Initialize before goto out */
     if (quiet) {
         progress = 0;

@@ -2100,8 +2116,14 @@ static int img_convert(int argc, char **argv)
     out_filename = bs_n >= 1 ? argv[argc - 1] : NULL;

qemu-img-cmds.hx +2 -0

+            goto fail_getopt;
+        }
     }
 
     if (bs_n < 1) {

Quoted 8 lines · Daniel P. Berrange

The '--image-opts' flags 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.

Quoted 7 lines

Reviewed-by Eric Blake <[email protected]>

Signed-off-by Daniel P. Berrange <[email protected]>

---
 qemu-img-cmds.hx |  4 +--
 qemu-img.c       | 86 ++++++++++++++++++++++++++++++++++++++++----------------
 qemu-img.texi    | 12 ++++++--
 3 files changed, 73 insertions(+), 29 deletions(-)

I'm afraid this will need a rebase onto block-next now (because of Peter's "qemu-img: simplify img_convert" patch).

Besides the obvious rebase conflicts, there are some less obvious things that may/should thus be changed:

Quoted 4 lines

qemu-img-cmds.hx +0 -0

index 8ac7822..93b50ef 100644

[...]

Quoted 4 lines

qemu-img-cmds.hx +0 -0

@@ -2090,6 +2100,12 @@ static int img_convert(int argc, char **argv)
         goto fail_getopt;
     }
 

qemu-img-cmds.hx static int img_convert(int argc, char **argv) · line 2105

Quoted 3 lines

qemu-img-cmds.hx +3 -0

+    if (tgt_image_opts && !skip_create) {
+        error_report("--target-image-opts requires use of -n flag");
+        ret = -1;

Depending on whether you decide to put this below bdrv_parse_cache_mode() or above, you might actually no longer have to set ret anymore.

Quoted 9 lines

qemu-img-cmds.hx +3 -0

+        goto fail_getopt;
+    }
+
     /* Initialize before goto out */
     if (quiet) {
         progress = 0;

@@ -2100,8 +2116,14 @@ static int img_convert(int argc, char **argv)
     out_filename = bs_n >= 1 ? argv[argc - 1] : NULL;

qemu-img-cmds.hx static int img_convert(int argc, char **argv) · line 2124

Quoted 10 lines

qemu-img-cmds.hx +6 -2

 
     if (options && has_help_option(options)) {
-        ret = print_block_option_help(out_filename, out_fmt);
-        goto out;
+        if (out_fmt) {
+            ret = print_block_option_help(out_filename, out_fmt);
+            goto out;
+        } else {
+            error_report("Option help requires a format be specified");
+            ret = -1;

Since this is most likely above bdrv_parse_cache_mode() (and may thus end up above the previous hunk?), you probably don't have to set ret here either (Sorry...).

The changes from v4 look good, but the rebase will result in non-trivial changes, so I giving an R-b would not be of any real use, I'm afraid...

The qemu-img dd/convert commands will create a 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.

The qemu-img dd/convert commands will create a 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 Eric Blake <[email protected]>

Signed-off-by Daniel P. Berrange <[email protected]>

---
 qemu-img.c | 41 +++++++++++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 6 deletions(-)

qemu-img.c +0 -0

index 2344e64..cee28db 100644

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

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