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

login
register
mail settings
Submitter Eduardo Habkost
Date Feb. 21, 2013, 9:44 p.m.
Message ID <1361483057-20503-1-git-send-email-ehabkost@redhat.com>
Download mbox | patch
Permalink /patch/222423/
State New
Headers show

Comments

Eduardo Habkost - Feb. 21, 2013, 9:44 p.m.
This allows ":" to be used a separator between each CPU range, so the
command-line may look like:

  -numa node,cpus=A-B:C-D

Note that the following format, currently used by libvirt:

  -numa nodes,cpus=A-B,C-D

will _not_ work, as "," is the option separator for the command-line
option parser, and it would require changing the -numa option parsing
code to handle "cpus" as a special case.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v2:
 - Use ":" as separator
 - Document the new format
---
 qemu-options.hx | 10 ++++++++--
 vl.c            | 14 +++++++++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)
Markus Armbruster - Feb. 26, 2013, 10:50 a.m.
Eduardo Habkost <ehabkost@redhat.com> writes:

> This allows ":" to be used a separator between each CPU range, so the
> command-line may look like:
>
>   -numa node,cpus=A-B:C-D
>
> Note that the following format, currently used by libvirt:
>
>   -numa nodes,cpus=A-B,C-D
>
> will _not_ work, as "," is the option separator for the command-line
> option parser, and it would require changing the -numa option parsing
> code to handle "cpus" as a special case.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v2:
>  - Use ":" as separator
>  - Document the new format

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.
Eduardo Habkost - Feb. 26, 2013, 2:05 p.m.
On Tue, Feb 26, 2013 at 11:50:08AM +0100, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > This allows ":" to be used a separator between each CPU range, so the
> > command-line may look like:
> >
> >   -numa node,cpus=A-B:C-D
> >
> > Note that the following format, currently used by libvirt:
> >
> >   -numa nodes,cpus=A-B,C-D
> >
> > will _not_ work, as "," is the option separator for the command-line
> > option parser, and it would require changing the -numa option parsing
> > code to handle "cpus" as a special case.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > Changes v2:
> >  - Use ":" as separator
> >  - Document the new format
> 
> 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.
Anthony Liguori - Feb. 26, 2013, 7:35 p.m.
Eduardo Habkost <ehabkost@redhat.com> writes:

> On Tue, Feb 26, 2013 at 11:50:08AM +0100, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>> > This allows ":" to be used a separator between each CPU range, so the
>> > command-line may look like:
>> >
>> >   -numa node,cpus=A-B:C-D
>> >
>> > Note that the following format, currently used by libvirt:
>> >
>> >   -numa nodes,cpus=A-B,C-D
>> >
>> > will _not_ work, as "," is the option separator for the command-line
>> > option parser, and it would require changing the -numa option parsing
>> > code to handle "cpus" as a special case.
>> >
>> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> > ---
>> > Changes v2:
>> >  - Use ":" as separator
>> >  - Document the new format
>> 
>> 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.

Regards,

Anthony Liguori

>
> -- 
> Eduardo
Eduardo Habkost - Feb. 26, 2013, 7:51 p.m.
On Tue, Feb 26, 2013 at 01:35:14PM -0600, Anthony Liguori wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > On Tue, Feb 26, 2013 at 11:50:08AM +0100, Markus Armbruster wrote:
> >> Eduardo Habkost <ehabkost@redhat.com> writes:
> >> 
> >> > This allows ":" to be used a separator between each CPU range, so the
> >> > command-line may look like:
> >> >
> >> >   -numa node,cpus=A-B:C-D
> >> >
> >> > Note that the following format, currently used by libvirt:
> >> >
> >> >   -numa nodes,cpus=A-B,C-D
> >> >
> >> > will _not_ work, as "," is the option separator for the command-line
> >> > option parser, and it would require changing the -numa option parsing
> >> > code to handle "cpus" as a special case.
> >> >
> >> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> >> > ---
> >> > Changes v2:
> >> >  - Use ":" as separator
> >> >  - Document the new format
> >> 
> >> 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.

Why? What should be the proper and generic way to represent and parse
lists, then?

There are many arguments being exposed at the v1 thread. See
 Message-ID: <512CC6D6.4060703@redhat.com>.
Paolo Bonzini - Feb. 26, 2013, 8:20 p.m.
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.

Paolo
Markus Armbruster - Feb. 26, 2013, 8:32 p.m.
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.

Seconded.

Patch

diff --git a/qemu-options.hx b/qemu-options.hx
index 4bc9c85..b135044 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -95,12 +95,18 @@  specifies the maximum number of hotpluggable CPUs.
 ETEXI
 
 DEF("numa", HAS_ARG, QEMU_OPTION_numa,
-    "-numa node[,mem=size][,cpus=cpu[-cpu]][,nodeid=node]\n", QEMU_ARCH_ALL)
+    "-numa node[,mem=size][,cpus=cpu[-cpu][:...]][,nodeid=node]\n", QEMU_ARCH_ALL)
 STEXI
 @item -numa @var{opts}
 @findex -numa
 Simulate a multi node NUMA system. If mem and cpus are omitted, resources
-are split equally.
+are split equally. Multiple CPU ranges in the "cpus" option may be separated
+by colons. e.g.:
+
+@example
+-numa node,mem=1024,cpus=0-1:4-5:8-9
+-numa node,mem=1024,cpus=2-3:6-7:10-11
+@end example
 ETEXI
 
 DEF("add-fd", HAS_ARG, QEMU_OPTION_add_fd,
diff --git a/vl.c b/vl.c
index 955d2ff..fe632db 100644
--- a/vl.c
+++ b/vl.c
@@ -1244,7 +1244,7 @@  char *get_boot_devices_list(size_t *size)
     return list;
 }
 
-static void numa_node_parse_cpus(int nodenr, const char *cpus)
+static void numa_node_parse_cpu_range(int nodenr, const char *cpus)
 {
     char *endptr;
     unsigned long long value, endvalue;
@@ -1288,6 +1288,18 @@  error:
     exit(1);
 }
 
+static void numa_node_parse_cpus(int nodenr, const char *option)
+{
+    char **parts;
+    int i;
+
+    parts = g_strsplit(option, ":", 0);
+    for (i = 0; parts[i]; i++) {
+        numa_node_parse_cpu_range(nodenr, parts[i]);
+    }
+    g_strfreev(parts);
+}
+
 static void numa_add(const char *optarg)
 {
     char option[128];