Re: [Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for replace work

[Chunyan Liu]

2014-03-18 3:35 GMT+08:00 Leandro Dorileo <address@hidden>:

Hi,

On Mon, Mar 10, 2014 at 03:31:41PM +0800, Chunyan Liu wrote:
> Add some qemu_opt functions to replace the same functionality of
> QEMUOptionParameter handling.
>
> Signed-off-by: Dong Xu Wang <address@hidden>
> Signed-off-by: Chunyan Liu <address@hidden>
> ---
>  include/qemu/option.h |   9 +++
>  util/qemu-option.c    | 188 ++++++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 184 insertions(+), 13 deletions(-)
>
> diff --git a/include/qemu/option.h b/include/qemu/option.h
> index c3b0a91..de4912a 100644
> --- a/include/qemu/option.h
> +++ b/include/qemu/option.h
> @@ -111,6 +111,7 @@ struct QemuOptsList {0
>  };
>
>  const char *qemu_opt_get(QemuOpts *opts, const char *name);
> +char *qemu_opt_get_del(QemuOpts *opts, const char *name);
>  /**
>   * qemu_opt_has_help_opt:
>   * @opts: options to search for a help request
> @@ -126,6 +127,11 @@ bool qemu_opt_has_help_opt(QemuOpts *opts);
>  bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool 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);
> +bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval);
> +uint64_t qemu_opt_get_number_del(QemuOpts *opts, const char *name,
> +                                 uint64_t defval);
> +uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name,
> +                               uint64_t defval);

I have initially understood you would remove these _del() suffixed functions in your
final cleanup, but it seems you haven't. Am I missing something?

Neither the functions nor the callers have been cleanup, why?

No, it won't be removed. It replaces the existing handling of QEMUOptionParameter.

For each driver, it handles the options they are expected to handle, then remove those

options from the list; for the left options, they will be passed to proto_drv for 2nd phase

handling.

 

Regards..

--
Dorileo

>  int qemu_opt_unset(QemuOpts *opts, const char *name);
>  int qemu_opt_set(QemuOpts *opts, const char *name, const char *value);
>  void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value,
> @@ -161,4 +167,7 @@ void qemu_opts_print(QemuOpts *opts);
>  int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque,
>                        int abort_on_failure);
>
> +QemuOptsList *qemu_opts_append(QemuOptsList *dst, QemuOptsList *list);
> +void qemu_opts_free(QemuOptsList *list);
> +void qemu_opts_print_help(QemuOptsList *list);
>  #endif
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index df79235..2c450e0 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -35,6 +35,7 @@
>
>  static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc,
>                                              const char *name);
> +static void qemu_opt_del(QemuOpt *opt);
>
>  /*
>   * Extracts the name of an option from the parameter string (p points at the
> @@ -379,6 +380,74 @@ QEMUOptionParameter *append_option_parameters(QEMUOptionParameter *dest,
>      return dest;
>  }
>
> +static size_t count_opts_list(QemuOptsList *list)
> +{
> +    QemuOptDesc *desc = NULL;
> +    size_t num_opts = 0;
> +
> +    if (!list) {
> +        return 0;
> +    }
> +
> +    desc = list->desc;
> +    while (desc && desc->name) {
> +        num_opts++;
> +        desc++;
> +    }
> +
> +    return num_opts;
> +}
> +
> +/* Create a new QemuOptsList with a desc of the merge of the first
> + * and second. It will allocate space for one new QemuOptsList plus
> + * enough space for QemuOptDesc in first and second QemuOptsList.
> + * First argument's QemuOptDesc members take precedence over second's.
> + * The result's name and implied_opt_name are not copied from them.
> + * Both merge_lists should not be set. Both lists can be NULL.
> + */
> +QemuOptsList *qemu_opts_append(QemuOptsList *dst,
> +                               QemuOptsList *list)
> +{
> +    size_t num_opts, num_dst_opts;
> +    QemuOptsList *tmp;
> +    QemuOptDesc *desc;
> +
> +    if (!dst && !list) {
> +        return NULL;
> +    }
> +
> +    num_opts = count_opts_list(dst);
> +    num_opts += count_opts_list(list);
> +    tmp = g_malloc0(sizeof(QemuOptsList) +
> +                    (num_opts + 1) * sizeof(QemuOptDesc));
> +    QTAILQ_INIT(&tmp->head);
> +    num_dst_opts = 0;
> +
> +    /* copy dst->desc to new list */
> +    if (dst) {
> +        desc = dst->desc;
> +        while (desc && desc->name) {
> +            tmp->desc[num_dst_opts++] = *desc;
> +            tmp->desc[num_dst_opts].name = NULL;
> +            desc++;
> +        }
> +    }
> +
> +    /* add list->desc to new list */
> +    if (list) {
> +        desc = list->desc;
> +        while (desc && desc->name) {
> +            if (find_desc_by_name(tmp->desc, desc->name) == NULL) {
> +                tmp->desc[num_dst_opts++] = *desc;
> +                tmp->desc[num_dst_opts].name = NULL;
> +            }
> +            desc++;
> +        }
> +    }
> +
> +    return tmp;
> +}
> +
>  /*
>   * Parses a parameter string (param) into an option list (dest).
>   *
> @@ -574,6 +643,29 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name)
>      return opt ? opt->str : NULL;
>  }
>
> +char *qemu_opt_get_del(QemuOpts *opts, const char *name)
> +{
> +    QemuOpt *opt;
> +    const QemuOptDesc *desc;
> +    char *str = NULL;
> +
> +    if (opts == NULL) {
> +        return NULL;
> +    }
> +
> +    opt = qemu_opt_find(opts, name);
> +    if (!opt) {
> +        desc = find_desc_by_name(opts->list->desc, name);
> +        if (desc && desc->def_value_str) {
> +            str = g_strdup(desc->def_value_str);
> +        }
> +        return str;
> +    }
> +    str = g_strdup(opt->str);
> +    qemu_opt_del(opt);
> +    return str;
> +}
> +
>  bool qemu_opt_has_help_opt(QemuOpts *opts)
>  {
>      QemuOpt *opt;
> @@ -586,9 +678,11 @@ bool qemu_opt_has_help_opt(QemuOpts *opts)
>      return false;
>  }
>
> -bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval)
> +static bool _qemu_opt_get_bool(QemuOpts *opts, const char *name,
> +                               bool defval, bool del)
>  {
>      QemuOpt *opt;
> +    bool ret = defval;
>
>      if (opts == NULL) {
>          return defval;
> @@ -599,19 +693,35 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval)
>      if (opt == NULL) {
>          const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
>          if (desc && desc->def_value_str) {
> -            parse_option_bool(name, desc->def_value_str, &defval, &error_abort);
> +            parse_option_bool(name, desc->def_value_str, &ret, &error_abort);
>          }
> -        return defval;
> +        return ret;
>      }
>      if (opt->desc) {
>          assert(opt->desc->type == QEMU_OPT_BOOL);
>      }
> -    return opt->value.boolean;
> +    ret = opt->value.boolean;
> +    if (del) {
> +        qemu_opt_del(opt);
> +    }
> +    return ret;
>  }
>
> -uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval)
> +bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval)
> +{
> +    return _qemu_opt_get_bool(opts, name, defval, false);
> +}
> +
> +bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval)
> +{
> +    return _qemu_opt_get_bool(opts, name, defval, true);
> +}
> +
> +static uint64_t _qemu_opt_get_number(QemuOpts *opts, const char *name,
> +                                     uint64_t defval, bool del)
>  {
>      QemuOpt *opt;
> +    uint64_t ret = defval;
>
>      if (opts == NULL) {
>          return defval;
> @@ -622,20 +732,36 @@ uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval)
>      if (opt == NULL) {
>          const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
>          if (desc && desc->def_value_str) {
> -            parse_option_number(name, desc->def_value_str, &defval,
> -                                &error_abort);
> +            parse_option_number(name, desc->def_value_str, &ret, &error_abort);
>          }
> -        return defval;
> +        return ret;
>      }
>      if (opt->desc) {
>          assert(opt->desc->type == QEMU_OPT_NUMBER);
>      }
> -    return opt->value.uint;
> +    ret = opt->value.uint;
> +    if (del) {
> +        qemu_opt_del(opt);
> +    }
> +    return ret;
>  }
>
> -uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval)
> +uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval)
> +{
> +    return _qemu_opt_get_number(opts, name, defval, false);
> +}
> +
> +uint64_t qemu_opt_get_number_del(QemuOpts *opts, const char *name,
> +                                 uint64_t defval)
> +{
> +    return _qemu_opt_get_number(opts, name, defval, true);
> +}
> +
> +static uint64_t _qemu_opt_get_size(QemuOpts *opts, const char *name,
> +                                   uint64_t defval, bool del)
>  {
>      QemuOpt *opt;
> +    uint64_t ret = defval;
>
>      if (opts == NULL) {
>          return defval;
> @@ -645,14 +771,29 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval)
>      if (opt == NULL) {
>          const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
>          if (desc && desc->def_value_str) {
> -            parse_option_size(name, desc->def_value_str, &defval, &error_abort);
> +            parse_option_size(name, desc->def_value_str, &ret, &error_abort);
>          }
> -        return defval;
> +        return ret;
>      }
>      if (opt->desc) {
>          assert(opt->desc->type == QEMU_OPT_SIZE);
>      }
> -    return opt->value.uint;
> +    ret = opt->value.uint;
> +    if (del) {
> +        qemu_opt_del(opt);
> +    }
> +    return ret;
> +}
> +
> +uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval)
> +{
> +    return _qemu_opt_get_size(opts, name, defval, false);
> +}
> +
> +uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name,
> +                               uint64_t defval)
> +{
> +    return _qemu_opt_get_size(opts, name, defval, true);
>  }
>
>  static void qemu_opt_parse(QemuOpt *opt, Error **errp)
> @@ -1302,3 +1443,24 @@ int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque,
>      loc_pop(&loc);
>      return rc;
>  }
> +
> +/* free a QemuOptsList, can accept NULL as arguments */
> +void qemu_opts_free(QemuOptsList *list)
> +{
> +    if (!list) {
> +        return;
> +    }
> +
> +    g_free(list);
> +}
> +
> +void qemu_opts_print_help(QemuOptsList *list)
> +{
> +    int i;
> +    printf("Supported options:\n");
> +    for (i = 0; list && list->desc[i].name; i++) {
> +        printf("%-16s %s\n", list->desc[i].name,
> +               list->desc[i].help ?
> +               list->desc[i].help : "");
> +    }
> +}
> --
> 1.7.12.4
>
>