On 02.05.2017 16:47, 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. > > Reviewed-by: Fam Zheng <f...@redhat.com> > Reviewed-by: Eric Blake <ebl...@redhat.com>
Sure you want to keep this, considering that there are quite some
changes since v5?
> Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> ---
> qemu-img-cmds.hx | 4 +--
> qemu-img.c | 77
> +++++++++++++++++++++++++++++++++++++-------------------
> qemu-img.texi | 12 +++++++--
> 3 files changed, 63 insertions(+), 30 deletions(-)
[...]
> diff --git a/qemu-img.c b/qemu-img.c
> index d8fdcb1..94c8cea 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
[...]
> @@ -1900,7 +1901,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;
Not sure about the indentation here. (I personally don't like spanning
the declaration over multiple lines in the first place, but that's a
different topic.) Indenting consecutive lines by four spaces is
standard, but the indentation by five spaces had a reason.
I guess I'd personally rather keep the five-space indentation...
> int64_t ret = -EINVAL;
>
> ImgConvertState s = (ImgConvertState) {
[...]
> @@ -2047,12 +2056,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 out;
Shouldn't this remain goto fail_getopt;?
> + } else {
> + error_report("Option help requires a format be specified");
> + goto fail_getopt;
> + }
> }
>
> if (s.src_num < 1) {
[...]
Why did you remove the compress &&
!out_bs->drv->bdrv_co_pwritev_compressed check? I liked it. :-(
Max
signature.asc
Description: OpenPGP digital signature
