[libvirt] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option

[Eduardo Habkost]

On Wed, Feb 27, 2013 at 09:42:50AM -0600, Anthony Liguori wrote:
> Paolo Bonzini <pbonzini at redhat.com> writes:
> 
> > Il 26/02/2013 20:35, Anthony Liguori ha scritto:
> >>>> >> 
> >>>> >> See also discussion on multi-valued keys in command line option
> >>>> >> arguments and config files in v1 thread.  Hopefully we can reach a
> >>>> >> conclusion soon, and then we'll see whether this patch is what we want.
> >>> >
> >>> > Yeah, let's drop this patch by now. I am starting to be convinced that
> >>> > "cpus=A,cpus=B,cpus=C" is the best approach. It is not pretty, but at
> >>> > least it uses generic parser code instead of yet another ad-hoc
> >>> > parser.
> >> No, we cannot rely on this behavior.  We had to do it to support
> >> backwards compat with netdev but it should not be used anywhere else.
> >
> > We chose a config format (git's) because it was a good match for
> > QemuOpts, and multiple-valued operations are well supported by that
> > config format; see git-config(1).  There is no reason to consider it a
> > backwards-compatibility hack.
>
> There's such thing as list support in QemuOpts.
[skipping stuff about internal implementation details that don't matter]
> 
> If we want to have list syntax, we need to introduce first class support
> for it.

Absolutely. But how the syntax for it should look like?


> Here's a simple example of how to do this.  Obviously we would
> want to introduce some better error checking so we can catch if someone
> tries to access a list with a non-list accessor.  We also would need
> list accessor functions.
> 
> But it's really not hard at all.

Of course. Once we decide how the syntax will look like, implementing
it should be very easy. But as far as I can see, we were trying to
discuss what's the appropriate syntax, here.

I still don't see why "option=A:B:C" with no possibility of having list
items containing ":" (like your proposal below) is a better generic
syntax for lists than "option=A,option=B,option=C".


> From 4ee7ed36d597f0defd78baac7480ecb29e11e1c1 Mon Sep 17 00:00:00 2001
> From: Anthony Liguori <aliguori at us.ibm.com>
> Date: Wed, 27 Feb 2013 09:32:09 -0600
> Subject: [PATCH] qemu-opts: support lists
> 
> Signed-off-by: Anthony Liguori <aliguori at us.ibm.com>
> ---
>  include/qemu/option.h |  2 ++
>  util/qemu-option.c    | 12 ++++++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/include/qemu/option.h b/include/qemu/option.h
> index ba197cd..e4808c3 100644
> --- a/include/qemu/option.h
> +++ b/include/qemu/option.h
> @@ -41,6 +41,7 @@ enum QEMUOptionParType {
>  typedef struct QEMUOptionParameter {
>      const char *name;
>      enum QEMUOptionParType type;
> +    bool list;
>      union {
>          uint64_t n;
>          char* s;
> @@ -95,6 +96,7 @@ enum QemuOptType {
>  typedef struct QemuOptDesc {
>      const char *name;
>      enum QemuOptType type;
> +    bool list;
>      const char *help;
>  } QemuOptDesc;
>  
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index 5a1d03c..6b1ae6e 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -636,6 +636,18 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value,
>          return;
>      }
>  
> +    if (desc->list && strchr(value, ':')) {
> +        gchar **tokens = g_strsplit(value, ":", 0);
> +        int i;
> +        for (i = 0; tokens[i]; i++) {
> +            opt_set(opts, name, tokens[i], prepend, errp);
> +            if (error_is_set(errp)) {
> +                return;
> +            }
> +        }
> +        g_strfreev(tokens);
> +    }
> +
>      opt = g_malloc0(sizeof(*opt));
>      opt->name = g_strdup(name);
>      opt->opts = opts;
> -- 
> 1.8.0
> 

> 
> Regards,
> 
> Anthony Liguori
> 
> >
> > Paolo


-- 
Eduardo