From 83d4bf943e09cbcb011e255c872724e95fe4856e Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Mon, 15 May 2017 17:47:09 +0100 Subject: qemu-img: add support for --object with 'dd' command 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 Reviewed-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Daniel P. Berrange Message-id: 20170515164712.6643-2-berrange@redhat.com Signed-off-by: Max Reitz --- qemu-img.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'qemu-img.c') 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); -- cgit v1.1 From ea204ddac7340bfda60cb0b388dbc3ffd77e8be0 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Mon, 15 May 2017 17:47:10 +0100 Subject: qemu-img: fix --image-opts usage with dd command 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 Reviewed-by: Max Reitz Signed-off-by: Daniel P. Berrange Message-id: 20170515164712.6643-3-berrange@redhat.com Signed-off-by: Max Reitz --- qemu-img.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'qemu-img.c') 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; -- cgit v1.1 From 305b4c60f200ee8e6267ac75f3f5b5d09fda1079 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Mon, 15 May 2017 17:47:11 +0100 Subject: qemu-img: introduce --target-image-opts for 'convert' command 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 Message-id: 20170515164712.6643-4-berrange@redhat.com Reviewed-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- qemu-img.c | 84 ++++++++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 57 insertions(+), 27 deletions(-) (limited to 'qemu-img.c') 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. */ -- cgit v1.1 From 29cf933635a50a4f1c51b022b323089997471e38 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Mon, 15 May 2017 17:47:12 +0100 Subject: qemu-img: copy *key-secret opts when opening newly created files 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. Signed-off-by: Daniel P. Berrange Message-id: 20170515164712.6643-5-berrange@redhat.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- qemu-img.c | 42 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-) (limited to 'qemu-img.c') 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) { -- cgit v1.1 From adb998c12aa7aa22c78baaec5c1252721e89c3de Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Mon, 15 May 2017 22:10:14 +0800 Subject: qemu-img: Fix leakage of options on error Reported by Coverity. Signed-off-by: Fam Zheng Message-id: 20170515141014.25793-1-famz@redhat.com Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- qemu-img.c | 1 + 1 file changed, 1 insertion(+) (limited to 'qemu-img.c') diff --git a/qemu-img.c b/qemu-img.c index 0bf941b..5aef8ef 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -295,6 +295,7 @@ static BlockBackend *img_open_opts(const char *optstr, if (qdict_haskey(options, BDRV_OPT_FORCE_SHARE) && !qdict_get_bool(options, BDRV_OPT_FORCE_SHARE)) { error_report("--force-share/-U conflicts with image options"); + QDECREF(options); return NULL; } qdict_put(options, BDRV_OPT_FORCE_SHARE, qbool_from_bool(true)); -- cgit v1.1