[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 3/4] qemu-img: introduce --target-image-opts
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v4 3/4] qemu-img: introduce --target-image-opts for 'convert' command |
Date: |
Thu, 13 Apr 2017 21:46:07 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0 |
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 <address@hidden>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
> 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.
> +
> address@hidden --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