Re: [Qemu-devel] [PATCH 04/19] Add support for a option parameter as an enum

[Markus Armbruster]

"Daniel P. Berrange" <address@hidden> writes:

> This adds a new option parameter QEMU_OPT_ENUM. The user
> provides the value in its string representation. The parser
> validates this and converts it to integer representation
> for internal use. If the user supplies an invalid value
> it will report the precise allowed list of values.
>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
>  qemu-option.c |   31 +++++++++++++++++++++++++++++++
>  qemu-option.h |   10 ++++++++++
>  2 files changed, 41 insertions(+), 0 deletions(-)
>
> diff --git a/qemu-option.c b/qemu-option.c
> index acd74f9..bb9cc43 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -238,6 +238,24 @@ static int parse_option_size(const char *name, const 
> char *value, uint64_t *ret)
>      return 0;
>  }
>  
> +static int parse_option_enum(const char *name, const char *value, int *ret, 
> const QemuOptDesc *desc)
> +{
> +    if (!value) {
> +        qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a enumerated 
> string");
> +        return -1;
> +    }
> +
> +    *ret = (desc->validate.optEnum.from_string)(value);
> +    if (*ret < 0) {
> +        char *valid = (desc->validate.optEnum.to_string_list)();
> +        qerror_report(QERR_INVALID_PARAMETER_VALUE, name, valid);
> +        qemu_free(valid);
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +
>  /*
>   * Sets the value of a parameter in a given option list. The parsing of the
>   * value depends on the type of option:
> @@ -516,6 +534,7 @@ struct QemuOpt {
>      union {
>          int      boolean;
>          uint64_t uint;
> +        int      enumVal;
>      } value;
>  
>      QemuOpts     *opts;
> @@ -578,6 +597,16 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char 
> *name, uint64_t defval)
>      return opt->value.uint;
>  }
>  
> +int qemu_opt_get_enum(QemuOpts *opts, const char *name, int defval)
> +{
> +    QemuOpt *opt = qemu_opt_find(opts, name);
> +
> +    if (opt == NULL)
> +        return defval;
> +    assert(opt->desc && opt->desc->type == QEMU_OPT_ENUM);
> +    return opt->value.enumVal;
> +}
> +
>  static int qemu_opt_parse(QemuOpt *opt)
>  {
>      if (opt->desc == NULL)
> @@ -592,6 +621,8 @@ static int qemu_opt_parse(QemuOpt *opt)
>          return parse_option_number(opt->name, opt->str, &opt->value.uint);
>      case QEMU_OPT_SIZE:
>          return parse_option_size(opt->name, opt->str, &opt->value.uint);
> +    case QEMU_OPT_ENUM:
> +        return parse_option_enum(opt->name, opt->str, &opt->value.enumVal, 
> opt->desc);
>      default:
>          abort();
>      }
> diff --git a/qemu-option.h b/qemu-option.h
> index 4823219..3540a9b 100644
> --- a/qemu-option.h
> +++ b/qemu-option.h
> @@ -89,12 +89,21 @@ enum QemuOptType {
>      QEMU_OPT_BOOL,        /* on/off                                          
>      */
>      QEMU_OPT_NUMBER,      /* simple number                                   
>      */
>      QEMU_OPT_SIZE,        /* size, accepts (K)ilo, (M)ega, (G)iga, (T)era 
> postfix */
> +    QEMU_OPT_ENUM,        /* int, from user string validated against an enum 
>      */
>  };

Could replace QEMU_OPT_BOOL by QEMU_OPT_ENUM.

>  
>  typedef struct QemuOptDesc {
>      const char *name;
>      enum QemuOptType type;
>      const char *help;
> +    union {
> +        struct {
> +            int (*from_string)(const char *);
> +            const char *(*to_string)(int);
> +            char *(*to_string_list)(void);
> +            int last;
> +        } optEnum;
> +    } validate;
>  } QemuOptDesc;
>  
>  struct QemuOptsList {

I think this enum stuff would be useful for qdev's PropertyInfo as well.
Maybe we should try to share more conversions to and from strings with
it.

> @@ -108,6 +117,7 @@ const char *qemu_opt_get(QemuOpts *opts, const char 
> *name);
>  int qemu_opt_get_bool(QemuOpts *opts, const char *name, int defval);
>  uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t 
> defval);
>  uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t 
> defval);
> +int qemu_opt_get_enum(QemuOpts *opts, const char *name, int defval);
>  int qemu_opt_set(QemuOpts *opts, const char *name, const char *value);
>  typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void 
> *opaque);
>  int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,