Re: [Qemu-devel] [PATCH 1/7] qemu-img create: Detect options specified more than once

[Fam Zheng <famz@redhat.com>]

On Wed, 02/19 16:12, Kevin Wolf wrote:
> If you specified multiple -o options for qemu-img create, it would
> silently ignore all but the last one. Similarly, for other options the
> last occurence wins (which is at least a bit less surprising). Error out
> instead.
> 
> The only exception is a -o help option, which may be added to any valid
> qemu-img create command and ignores all other options.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qemu-img.c | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index c989850..6a64fe1 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -341,13 +341,14 @@ static int img_create(int argc, char **argv)
>  {
>      int c;
>      uint64_t img_size = -1;
> -    const char *fmt = "raw";
> +    const char *fmt = NULL;
>      const char *base_fmt = NULL;
>      const char *filename;
>      const char *base_filename = NULL;
>      char *options = NULL;
>      Error *local_err = NULL;
>      bool quiet = false;
> +    bool options_help = false;
>  
>      for(;;) {
>          c = getopt(argc, argv, "F:b:f:he6o:q");
> @@ -360,12 +361,24 @@ static int img_create(int argc, char **argv)
>              help();
>              break;
>          case 'F':
> +            if (base_fmt) {
> +                error_report("-F may only be specified once");
> +                return 1;
> +            }
>              base_fmt = optarg;
>              break;
>          case 'b':
> +            if (base_filename) {
> +                error_report("-b may only be specified once");
> +                return 1;
> +            }
>              base_filename = optarg;
>              break;
>          case 'f':
> +            if (fmt) {
> +                error_report("-f may only be specified once");
> +                return 1;
> +            }
>              fmt = optarg;
>              break;
>          case 'e':
> @@ -377,7 +390,16 @@ static int img_create(int argc, char **argv)
>                    "compat6\' instead!");
>              return 1;
>          case 'o':
> -            options = optarg;
> +            if (is_help_option(optarg)) {
> +                options_help = true;
> +            } else if (!options) {
> +                options = optarg;
> +            } else {
> +                error_report("-o cannot be used multiple times. Please use a "
> +                             "single -o option with comma-separated settings "
> +                             "instead.");

Not consistent with ending "." but not a big problem.

Reviewed-by: Fam Zheng <famz@redhat.com>

> +                return 1;
> +            }
>              break;
>          case 'q':
>              quiet = true;
> @@ -385,6 +407,10 @@ static int img_create(int argc, char **argv)
>          }
>      }
>  
> +    if (!fmt) {
> +        fmt = "raw";
> +    }
> +
>      /* Get the filename */
>      if (optind >= argc) {
>          help();
> @@ -413,7 +439,7 @@ static int img_create(int argc, char **argv)
>          help();
>      }
>  
> -    if (options && is_help_option(options)) {
> +    if (options_help) {
>          return print_block_option_help(filename, fmt);
>      }
>  
> -- 
> 1.8.1.4
> 
>