On 12.04.2017 18:44, Daniel P. Berrange wrote: > 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 <ebl...@redhat.com> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > --- > qemu-img-cmds.hx | 4 +-- > qemu-img.c | 84 > +++++++++++++++++++++++++++++++++++++++----------------- > qemu-img.texi | 12 ++++++-- > 3 files changed, 71 insertions(+), 29 deletions(-)
Looks good apart from some minor issues.
[...]
> diff --git a/qemu-img.c b/qemu-img.c
> index 83aff5e..31c4923 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
[...]
> @@ -2090,6 +2100,11 @@ 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;
ret should be set to -1.
> + }
> +
> /* Initialize before goto out */
> if (quiet) {
> progress = 0;
> @@ -2100,8 +2115,13 @@ static int img_convert(int argc, char **argv)
> out_filename = bs_n >= 1 ? argv[argc - 1] : NULL;
>
> 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");
> + goto fail_getopt;
Same here.
> + }
> }
>
> if (bs_n < 1) {
[...]
> diff --git a/qemu-img.texi b/qemu-img.texi
> index c81db3e..63f597d 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 target @var{filename} parameter(s) are to be interpreted a
s/a$/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.
By the way, since you're referring to -n, you could just call it
"@var{output_filename}" instead of "target @var{filename}", because
that's how convert's target filename is described in this man page.
Max
>
> @item fmt
> is the disk image format. It is guessed automatically in most cases. See
> below
>
signature.asc
Description: OpenPGP digital signature
