[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



Paolo Bonzini <pbonzini 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.  The only way
QemuOptsVisitor was able to implement it was to expose QemuOpts publicly
via options_int.h and rely on a implementation detail.

There are fixed types supported by QemuOpts.  It just so happens that
whenever qemu_opt_set() is called, instead of replacing the last
instance, the value is either prepended or appended in order to
implement a replace or set-if-unset behavior.

All values are treated this way.  So the above "list" would only be:

{
        .name = "cpus",
        .type = QEMU_OPT_STRING
}

Which is indistinguishable from a straight string property.  This means
it's impossible to introspect because the type is context-sensitive.

What's more, there is no API outside of QemuOptsVisitor that can
actually work with "lists" of QemuOpts values.

If we want to have list syntax, we need to introduce first class support
for it.  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.

(Only compile tested)

>From 4ee7ed36d597f0defd78baac7480ecb29e11e1c1 Mon Sep 17 00:00:00 2001
From: Anthony Liguori <aliguori us ibm com>
Date: Wed, 27 Feb 2013 09:32:09 -0600
Subject: [PATCH] qemu-opts: support lists

Signed-off-by: Anthony Liguori <aliguori 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

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]