Re: [Qemu-devel] [PATCH v2 5/6] qemu-img: Allow -o help with incomplete argument list

[Jeff Cody]

On Thu, Feb 20, 2014 at 03:57:22PM +0100, Kevin Wolf wrote:
> This patch allows using 'qemu-img $subcmd -o help' for the create,
> convert and amend subcommands, without specifying the previously
> required filename arguments.
> 
> Note that it's still allowed and meaningful to specify a filename: An
> invocation like 'qemu-img create -o help sheepdog:foo' will also display
> options that are provided by the Sheepdog driver.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  qemu-img.c | 58 +++++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 35 insertions(+), 23 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index d6dc7ec..98ddbe7 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -250,16 +250,19 @@ static int print_block_option_help(const char 
> *filename, const char *fmt)
>          return 1;
>      }
>  
> -    proto_drv = bdrv_find_protocol(filename, true);
> -    if (!proto_drv) {
> -        error_report("Unknown protocol '%s'", filename);
> -        return 1;
> -    }
> -
>      create_options = append_option_parameters(create_options,
>                                                drv->create_options);
> -    create_options = append_option_parameters(create_options,
> -                                              proto_drv->create_options);
> +
> +    if (filename) {
> +        proto_drv = bdrv_find_protocol(filename, true);
> +        if (!proto_drv) {
> +            error_report("Unknown protocol '%s'", filename);
> +            return 1;
> +        }
> +        create_options = append_option_parameters(create_options,
> +                                                  proto_drv->create_options);
> +    }
> +
>      print_option_help(create_options);
>      free_option_parameters(create_options);
>      return 0;
> @@ -390,10 +393,16 @@ static int img_create(int argc, char **argv)
>      }
>  
>      /* Get the filename */
> +    filename = (optind < argc) ? argv[optind] : NULL;
> +    if (options && has_help_option(options)) {
> +        g_free(options);
> +        return print_block_option_help(filename, fmt);
> +    }
> +

Same comment as patch #2 with regards to common cleanup code in
img_create().

Reviewed-by: Jeff Cody <address@hidden>

>      if (optind >= argc) {
>          help();
>      }
> -    filename = argv[optind++];
> +    optind++;
>  
>      /* Get image size, if specified */
>      if (optind < argc) {
> @@ -417,11 +426,6 @@ static int img_create(int argc, char **argv)
>          help();
>      }
>  
> -    if (options && has_help_option(options)) {
> -        g_free(options);
> -        return print_block_option_help(filename, fmt);
> -    }
> -
>      bdrv_img_create(filename, fmt, base_filename, base_fmt,
>                      options, img_size, BDRV_O_FLAGS, &local_err, quiet);
>      if (error_is_set(&local_err)) {
> @@ -1260,17 +1264,18 @@ static int img_convert(int argc, char **argv)
>      }
>  
>      bs_n = argc - optind - 1;
> -    if (bs_n < 1) {
> -        help();
> -    }
> -
> -    out_filename = argv[argc - 1];
> +    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 (bs_n < 1) {
> +        help();
> +    }
> +
> +
>      if (bs_n > 1 && out_baseimg) {
>          error_report("-B makes no sense when concatenating multiple input "
>                       "images");
> @@ -2675,15 +2680,21 @@ static int img_amend(int argc, char **argv)
>          }
>      }
>  
> -    if (optind != argc - 1) {
> +    if (!options) {
>          help();
>      }
>  
> -    if (!options) {
> -        help();
> +    filename = (optind == argc - 1) ? argv[argc - 1] : NULL;
> +    if (fmt && has_help_option(options)) {
> +        /* If a format is explicitly specified (and possibly no filename is
> +         * given), print option help here */
> +        ret = print_block_option_help(filename, fmt);
> +        goto out;
>      }
>  
> -    filename = argv[argc - 1];
> +    if (optind != argc - 1) {
> +        help();
> +    }
>  
>      bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true, 
> quiet);
>      if (!bs) {
> @@ -2695,6 +2706,7 @@ static int img_amend(int argc, char **argv)
>      fmt = bs->drv->format_name;
>  
>      if (has_help_option(options)) {
> +        /* If the format was auto-detected, print option help here */
>          ret = print_block_option_help(filename, fmt);
>          goto out;
>      }
> -- 
> 1.8.1.4
> 
>