diff mbox

[V4,01/10] NUMA: Support multiple CPU ranges on -numa option

Message ID 1372931597-28115-2-git-send-email-gaowanlong@cn.fujitsu.com
State New
Headers show

Commit Message

Wanlong Gao July 4, 2013, 9:53 a.m. UTC
From: Bandan Das <bsd@redhat.com>

This allows us to use the "cpus" property multiple times
to specify multiple cpu (ranges) to the -numa option :

-numa node,cpus=1,cpus=2,cpus=4
or
-numa node,cpus=1-3,cpus=5

Note that after this patch, the defalut suffix of "-numa node,mem=N"
will no longer be "M". So we must add the suffix "M" like "-numa node,mem=NM"
when assigning "N MB" of node memory size.

Signed-off-by: Bandan Das <bsd@redhat.com>
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
---
 qemu-options.hx |   3 +-
 vl.c            | 108 ++++++++++++++++++++++++++++++++++----------------------
 2 files changed, 67 insertions(+), 44 deletions(-)

Comments

Eduardo Habkost July 5, 2013, 6:41 p.m. UTC | #1
On Thu, Jul 04, 2013 at 05:53:08PM +0800, Wanlong Gao wrote:
> From: Bandan Das <bsd@redhat.com>
> 
> This allows us to use the "cpus" property multiple times
> to specify multiple cpu (ranges) to the -numa option :
> 
> -numa node,cpus=1,cpus=2,cpus=4
> or
> -numa node,cpus=1-3,cpus=5
> 
> Note that after this patch, the defalut suffix of "-numa node,mem=N"
> will no longer be "M". So we must add the suffix "M" like "-numa node,mem=NM"
> when assigning "N MB" of node memory size.

Such an incompatible change is not acceptable, as it would break
existing configurations. libvirt doesn't specify any suffix and expects
it to always mean "MB".

> 
> Signed-off-by: Bandan Das <bsd@redhat.com>
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> ---
>  qemu-options.hx |   3 +-
>  vl.c            | 108 ++++++++++++++++++++++++++++++++++----------------------
>  2 files changed, 67 insertions(+), 44 deletions(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 137a39b..449cf36 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -100,7 +100,8 @@ 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. The "-cpus" property may be specified multiple times

The option is not named "-cpus", but just "cpus". And I believe it is
normally called an "option", not a "property".

> +to denote multiple cpus or cpu ranges.
>  ETEXI
>  
>  DEF("add-fd", HAS_ARG, QEMU_OPTION_add_fd,
> diff --git a/vl.c b/vl.c
> index 6d9fd7d..6f2e17a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -516,6 +516,32 @@ static QemuOptsList qemu_realtime_opts = {
>      },
>  };
>  
> +static QemuOptsList qemu_numa_opts = {
> +    .name = "numa",
> +    .implied_opt_name = "type",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_numa_opts.head),
> +    .desc = {
> +        {
> +            .name = "type",
> +            .type = QEMU_OPT_STRING,
> +            .help = "node type"
> +        },{
> +            .name = "nodeid",
> +            .type = QEMU_OPT_NUMBER,
> +            .help = "node ID"
> +        },{
> +            .name = "mem",
> +            .type = QEMU_OPT_SIZE,
> +            .help = "memory size"
> +        },{
> +            .name = "cpus",
> +            .type = QEMU_OPT_STRING,
> +            .help = "cpu number or range"
> +        },
> +        { /* end of list */ }
> +    },
> +};
> +
>  const char *qemu_get_vm_name(void)
>  {
>      return qemu_name;
> @@ -1349,56 +1375,37 @@ error:
>      exit(1);
>  }
>  
> -static void numa_add(const char *optarg)
> +
> +static int numa_add_cpus(const char *name, const char *value, void *opaque)
>  {
> -    char option[128];
> -    char *endptr;
> -    unsigned long long nodenr;
> +    int *nodenr = opaque;
>  
> -    optarg = get_opt_name(option, 128, optarg, ',');
> -    if (*optarg == ',') {
> -        optarg++;
> +    if (!strcmp(name, "cpu")) {
> +        numa_node_parse_cpus(*nodenr, value);
>      }
> -    if (!strcmp(option, "node")) {
> -
> -        if (nb_numa_nodes >= MAX_NODES) {
> -            fprintf(stderr, "qemu: too many NUMA nodes\n");
> -            exit(1);
> -        }
> +    return 0;
> +}
>  
> -        if (get_param_value(option, 128, "nodeid", optarg) == 0) {
> -            nodenr = nb_numa_nodes;
> -        } else {
> -            if (parse_uint_full(option, &nodenr, 10) < 0) {
> -                fprintf(stderr, "qemu: Invalid NUMA nodeid: %s\n", option);
> -                exit(1);
> -            }
> -        }
> +static int numa_init_func(QemuOpts *opts, void *opaque)
> +{
> +    uint64_t nodenr, mem_size;
>  
> -        if (nodenr >= MAX_NODES) {
> -            fprintf(stderr, "qemu: invalid NUMA nodeid: %llu\n", nodenr);
> -            exit(1);
> -        }
> +    nodenr = qemu_opt_get_number(opts, "nodeid", nb_numa_nodes++);
>  
> -        if (get_param_value(option, 128, "mem", optarg) == 0) {
> -            node_mem[nodenr] = 0;
> -        } else {
> -            int64_t sval;
> -            sval = strtosz(option, &endptr);
> -            if (sval < 0 || *endptr) {
> -                fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg);
> -                exit(1);
> -            }
> -            node_mem[nodenr] = sval;
> -        }
> -        if (get_param_value(option, 128, "cpus", optarg) != 0) {
> -            numa_node_parse_cpus(nodenr, option);
> -        }
> -        nb_numa_nodes++;
> -    } else {
> -        fprintf(stderr, "Invalid -numa option: %s\n", option);
> +    if (nodenr >= MAX_NODES) {
> +        fprintf(stderr, "qemu: Max number of NUMA nodes reached : %d\n",
> +                (int)nodenr);
>          exit(1);
>      }
> +
> +    mem_size = qemu_opt_get_size(opts, "mem", 0);
> +    node_mem[nodenr] = mem_size;
> +
> +    if (qemu_opt_foreach(opts, numa_add_cpus, &nodenr, 1) < 0) {
> +        return -1;
> +    }
> +
> +    return 0;
>  }
>  
>  static QemuOptsList qemu_smp_opts = {
> @@ -2933,6 +2940,7 @@ int main(int argc, char **argv, char **envp)
>      qemu_add_opts(&qemu_object_opts);
>      qemu_add_opts(&qemu_tpmdev_opts);
>      qemu_add_opts(&qemu_realtime_opts);
> +    qemu_add_opts(&qemu_numa_opts);
>  
>      runstate_init();
>  
> @@ -3119,7 +3127,16 @@ int main(int argc, char **argv, char **envp)
>                  }
>                  break;
>              case QEMU_OPTION_numa:
> -                numa_add(optarg);
> +                olist = qemu_find_opts("numa");
> +                opts = qemu_opts_parse(olist, optarg, 1);
> +                if (!opts) {
> +                    exit(1);
> +                }
> +                optarg = qemu_opt_get(opts, "type");
> +                if (!optarg || strcmp(optarg, "node")) {
> +                    fprintf(stderr, "qemu: Incorrect format for numa option\n");

Why not do this inside numa_init_func()?

> +                    exit(1);
> +                }
>                  break;
>              case QEMU_OPTION_display:
>                  display_type = select_display(optarg);
> @@ -4195,6 +4212,11 @@ int main(int argc, char **argv, char **envp)
>  
>      register_savevm_live(NULL, "ram", 0, 4, &savevm_ram_handlers, NULL);
>  
> +    if (qemu_opts_foreach(qemu_find_opts("numa"), numa_init_func,
> +                          NULL, 1) != 0) {
> +        exit(1);
> +    }
> +
>      if (nb_numa_nodes > 0) {
>          int i;
>  
> -- 
> 1.8.3.2.634.g7a3187e
> 
>
Eric Blake July 8, 2013, 7:02 p.m. UTC | #2
On 07/05/2013 12:41 PM, Eduardo Habkost wrote:
> On Thu, Jul 04, 2013 at 05:53:08PM +0800, Wanlong Gao wrote:
>> From: Bandan Das <bsd@redhat.com>
>>
>> This allows us to use the "cpus" property multiple times
>> to specify multiple cpu (ranges) to the -numa option :
>>
>> -numa node,cpus=1,cpus=2,cpus=4
>> or
>> -numa node,cpus=1-3,cpus=5
>>
>> Note that after this patch, the defalut suffix of "-numa node,mem=N"
>> will no longer be "M". So we must add the suffix "M" like "-numa node,mem=NM"
>> when assigning "N MB" of node memory size.
> 
> Such an incompatible change is not acceptable, as it would break
> existing configurations. libvirt doesn't specify any suffix and expects
> it to always mean "MB".

Newer libvirt can be taught to append 'M' when it detects it is talking
to newer qemu.  While you have a point that it is annoying to force
users to upgrade to a newer libvirt merely because they upgraded qemu,
the libvirt point of view is that the following are supported:

old libvirt -> old qemu
new libvirt -> old qemu
new libvirt -> new qemu

but that this combination is always best effort and not required to work:

old libvirt -> new qemu
Eduardo Habkost July 8, 2013, 7:25 p.m. UTC | #3
On Mon, Jul 08, 2013 at 01:02:41PM -0600, Eric Blake wrote:
> On 07/05/2013 12:41 PM, Eduardo Habkost wrote:
> > On Thu, Jul 04, 2013 at 05:53:08PM +0800, Wanlong Gao wrote:
> >> From: Bandan Das <bsd@redhat.com>
> >>
> >> This allows us to use the "cpus" property multiple times
> >> to specify multiple cpu (ranges) to the -numa option :
> >>
> >> -numa node,cpus=1,cpus=2,cpus=4
> >> or
> >> -numa node,cpus=1-3,cpus=5
> >>
> >> Note that after this patch, the defalut suffix of "-numa node,mem=N"
> >> will no longer be "M". So we must add the suffix "M" like "-numa node,mem=NM"
> >> when assigning "N MB" of node memory size.
> > 
> > Such an incompatible change is not acceptable, as it would break
> > existing configurations. libvirt doesn't specify any suffix and expects
> > it to always mean "MB".
> 
> Newer libvirt can be taught to append 'M' when it detects it is talking
> to newer qemu.  While you have a point that it is annoying to force
> users to upgrade to a newer libvirt merely because they upgraded qemu,
> the libvirt point of view is that the following are supported:
> 
> old libvirt -> old qemu
> new libvirt -> old qemu
> new libvirt -> new qemu
> 
> but that this combination is always best effort and not required to work:
> 
> old libvirt -> new qemu

I assume the rules above apply only if "new libvirt" gets released
before "new qemu", right? Otherwise people won't be able to use latest
released libvirt with latest released qemu until "new libvirt" gets
released.
Anthony Liguori July 8, 2013, 7:25 p.m. UTC | #4
Eric Blake <eblake@redhat.com> writes:

> On 07/05/2013 12:41 PM, Eduardo Habkost wrote:
>> On Thu, Jul 04, 2013 at 05:53:08PM +0800, Wanlong Gao wrote:
>>> From: Bandan Das <bsd@redhat.com>
>>>
>>> This allows us to use the "cpus" property multiple times
>>> to specify multiple cpu (ranges) to the -numa option :
>>>
>>> -numa node,cpus=1,cpus=2,cpus=4
>>> or
>>> -numa node,cpus=1-3,cpus=5
>>>
>>> Note that after this patch, the defalut suffix of "-numa node,mem=N"
>>> will no longer be "M". So we must add the suffix "M" like "-numa node,mem=NM"
>>> when assigning "N MB" of node memory size.
>> 
>> Such an incompatible change is not acceptable, as it would break
>> existing configurations. libvirt doesn't specify any suffix and expects
>> it to always mean "MB".
>
> Newer libvirt can be taught to append 'M' when it detects it is talking
> to newer qemu.  While you have a point that it is annoying to force
> users to upgrade to a newer libvirt merely because they upgraded qemu,
> the libvirt point of view is that the following are supported:
>
> old libvirt -> old qemu
> new libvirt -> old qemu
> new libvirt -> new qemu
>
> but that this combination is always best effort and not required to work:
>
> old libvirt -> new qemu

That's fine for libvirt, but we don't break command line compatibility
in QEMU.  So this patch needs to change.

Regards,

Anthony Liguori

>
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
Wanlong Gao July 9, 2013, 3:28 a.m. UTC | #5
On 07/09/2013 03:25 AM, Anthony Liguori wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 07/05/2013 12:41 PM, Eduardo Habkost wrote:
>>> On Thu, Jul 04, 2013 at 05:53:08PM +0800, Wanlong Gao wrote:
>>>> From: Bandan Das <bsd@redhat.com>
>>>>
>>>> This allows us to use the "cpus" property multiple times
>>>> to specify multiple cpu (ranges) to the -numa option :
>>>>
>>>> -numa node,cpus=1,cpus=2,cpus=4
>>>> or
>>>> -numa node,cpus=1-3,cpus=5
>>>>
>>>> Note that after this patch, the defalut suffix of "-numa node,mem=N"
>>>> will no longer be "M". So we must add the suffix "M" like "-numa node,mem=NM"
>>>> when assigning "N MB" of node memory size.
>>>
>>> Such an incompatible change is not acceptable, as it would break
>>> existing configurations. libvirt doesn't specify any suffix and expects
>>> it to always mean "MB".
>>
>> Newer libvirt can be taught to append 'M' when it detects it is talking
>> to newer qemu.  While you have a point that it is annoying to force
>> users to upgrade to a newer libvirt merely because they upgraded qemu,
>> the libvirt point of view is that the following are supported:
>>
>> old libvirt -> old qemu
>> new libvirt -> old qemu
>> new libvirt -> new qemu
>>
>> but that this combination is always best effort and not required to work:
>>
>> old libvirt -> new qemu
> 
> That's fine for libvirt, but we don't break command line compatibility
> in QEMU.  So this patch needs to change.

But if we follow Paolo's suggestion like:
    -numa node,nodeid=0,cpus=0 -numa node,nodeid=1,cpus=1 \
    -numa mem,nodeid=0,size=1G,policy=membind,hostnode=0-1
    -numa mem,nodeid=1,size=2G,policy=interleave,hostnode=1

We already break the command line compatibility.
Why not change it be a really "size" options without default suffix?

Thanks,
Wanlong Gao

> 
> Regards,
> 
> Anthony Liguori
> 
>>
>> -- 
>> Eric Blake   eblake redhat com    +1-919-301-3266
>> Libvirt virtualization library http://libvirt.org
> 
>
Eric Blake July 9, 2013, 3:34 a.m. UTC | #6
On 07/08/2013 09:28 PM, Wanlong Gao wrote:

>>>>>
>>>>> Note that after this patch, the defalut suffix of "-numa node,mem=N"
>>>>> will no longer be "M". So we must add the suffix "M" like "-numa node,mem=NM"
>>>>> when assigning "N MB" of node memory size.
>>>>

> 
> But if we follow Paolo's suggestion like:
>     -numa node,nodeid=0,cpus=0 -numa node,nodeid=1,cpus=1 \
>     -numa mem,nodeid=0,size=1G,policy=membind,hostnode=0-1
>     -numa mem,nodeid=1,size=2G,policy=interleave,hostnode=1
> 
> We already break the command line compatibility.

New command options can have whatever syntax makes sense.  The worry
here is that if you use the old command line without any new options,
the old syntax must behave the same.

> Why not change it be a really "size" options without default suffix?

Because users are annoyed when a command line that worked with qemu 1.5
needlessly fails to work with qemu 1.6.
Paolo Bonzini July 14, 2013, 11:34 a.m. UTC | #7
Il 08/07/2013 21:02, Eric Blake ha scritto:
> On 07/05/2013 12:41 PM, Eduardo Habkost wrote:
>> On Thu, Jul 04, 2013 at 05:53:08PM +0800, Wanlong Gao wrote:
>>> From: Bandan Das <bsd@redhat.com>
>>>
>>> This allows us to use the "cpus" property multiple times
>>> to specify multiple cpu (ranges) to the -numa option :
>>>
>>> -numa node,cpus=1,cpus=2,cpus=4
>>> or
>>> -numa node,cpus=1-3,cpus=5
>>>
>>> Note that after this patch, the defalut suffix of "-numa node,mem=N"
>>> will no longer be "M". So we must add the suffix "M" like "-numa node,mem=NM"
>>> when assigning "N MB" of node memory size.
>>
>> Such an incompatible change is not acceptable, as it would break
>> existing configurations. libvirt doesn't specify any suffix and expects
>> it to always mean "MB".
> 
> Newer libvirt can be taught to append 'M' when it detects it is talking
> to newer qemu.  While you have a point that it is annoying to force
> users to upgrade to a newer libvirt merely because they upgraded qemu,
> the libvirt point of view is that the following are supported:
> 
> old libvirt -> old qemu
> new libvirt -> old qemu
> new libvirt -> new qemu
> 
> but that this combination is always best effort and not required to work:
> 
> old libvirt -> new qemu

I don't think this is the case, unless you're talking of *very* old
libvirt (e.g. pre-QMP).

Paolo
Eric Blake July 15, 2013, 9:33 p.m. UTC | #8
On 07/14/2013 05:34 AM, Paolo Bonzini wrote:

>>> Such an incompatible change is not acceptable, as it would break
>>> existing configurations. libvirt doesn't specify any suffix and expects
>>> it to always mean "MB".
>>
>> Newer libvirt can be taught to append 'M' when it detects it is talking
>> to newer qemu.  While you have a point that it is annoying to force
>> users to upgrade to a newer libvirt merely because they upgraded qemu,
>> the libvirt point of view is that the following are supported:
>>
>> old libvirt -> old qemu
>> new libvirt -> old qemu
>> new libvirt -> new qemu
>>
>> but that this combination is always best effort and not required to work:
>>
>> old libvirt -> new qemu
> 
> I don't think this is the case, unless you're talking of *very* old
> libvirt (e.g. pre-QMP).

As a counter-example, I can recall a case where a qemu release that used
just two digits (was that 1.2?) broke operation under older libvirt that
assumed versions would always be three digits; but it definitely
occurred after 0.15.x which is the point at which libvirt started
favoring QMP.  That is, we had a case in Fedora where if you upgraded
qemu, you HAD to also update libvirt to be able to keep your guests running.

But yes, the goal of having command line compatibility, so that any
application using the same command line it always uses will get the same
guest, regardless of a qemu upgrade in the meantime, should be our
default mode of operation, even if newer apps should prefer newer
(better) command line interfaces.
Paolo Bonzini July 16, 2013, 6:24 a.m. UTC | #9
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 15/07/2013 23:33, Eric Blake ha scritto:
>>>>> Newer libvirt can be taught to append 'M' when it detects
>>>>> it is talking to newer qemu.  While you have a point that
>>>>> it is annoying to force users to upgrade to a newer libvirt
>>>>> merely because they upgraded qemu, the libvirt point of
>>>>> view is that the following are supported:
>>>>> 
>>>>> old libvirt -> old qemu new libvirt -> old qemu new libvirt
>>>>> -> new qemu
>>>>> 
>>>>> but that this combination is always best effort and not
>>>>> required to work:
>>>>> 
>>>>> old libvirt -> new qemu
>>> 
>>> I don't think this is the case, unless you're talking of *very*
>>> old libvirt (e.g. pre-QMP).
> As a counter-example, I can recall a case where a qemu release that
> used just two digits (was that 1.2?) broke operation under older
> libvirt that assumed versions would always be three digits; but it
> definitely occurred after 0.15.x which is the point at which
> libvirt started favoring QMP.  That is, we had a case in Fedora
> where if you upgraded qemu, you HAD to also update libvirt to be
> able to keep your guests running.

Right, I remember that now.  So, better: "we have some interfaces
which are considered API, and old libvirt -> new QEMU should not break
for things that use those interfaces".  QMP and the command line are
definitely one.

The case you mentioned was about -help, if I remember correctly, which
was indeed quite brittle (like HMP).

> But yes, the goal of having command line compatibility, so that
> any application using the same command line it always uses will get
> the same guest, regardless of a qemu upgrade in the meantime,
> should be our default mode of operation, even if newer apps should
> prefer newer (better) command line interfaces.

Yes, the command line *is* part of the API.

Paolo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJR5OcaAAoJEBvWZb6bTYbyGQgQAJVrX6z8/0hozhjAz81G7tuX
cVmnC5cw+TgfYspf73yoBLBYZPUY/Ydb7WiabKrSfweMFX848WVhQr7rkwp0DVKQ
X0WSbEKrVIGRMCjtvEMkzw1fmXintPLsaoxaLqYZs2MFgEsEP1eEG2MT/2JwpFd/
iDkqVmQ9fPxCEm8beoJXN8HV4Mwz5YY5E04tSqCktJzPh9+sGwB4cPy7PPiPjvHK
I8nIdLHtOqFs4SwX1ic6HEZbeBE71swxr5QKhSg3/v6MzjZbK9/IU0RBcY69ftek
3fRJV8/hs8mHhfT7LsvB7XCNOxYq8jD1Bzy4oMJ/3LcAOyTLt1QJzFW4yaRSNGBK
6V/pDSWlghefulZu/aZASMh/IyxuCJRJ0uMVUEi20FeaIs96Bq5QBEInN/1JIYdH
Qkek7C6dTrP1EdfbZFRa8+RzYEIDL0XmJFce8oicPZLGbhr/Jg1tZAkcUGr9gaeh
z9bOTgAI98z29ZSHm4Bb3rb1WWSJY7BBRAgIDDxZuf34wuVUGWEvJOHRkB2iRa87
d6howw9eqWogVNNYHKYoTQCxEaTe7/PB0wXdWX5+AAZ29C0ETPZFOBYVwh9QLuCD
V9WxGutqlXohbTgOk8rERHcUMJLlNblJg/i0tOMDU2Me4Uv+nW8UawEWUJClZiZS
5emSnaCr7UwPS9qUz1n1
=LQs8
-----END PGP SIGNATURE-----
diff mbox

Patch

diff --git a/qemu-options.hx b/qemu-options.hx
index 137a39b..449cf36 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -100,7 +100,8 @@  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. The "-cpus" property may be specified multiple times
+to denote multiple cpus or cpu ranges.
 ETEXI
 
 DEF("add-fd", HAS_ARG, QEMU_OPTION_add_fd,
diff --git a/vl.c b/vl.c
index 6d9fd7d..6f2e17a 100644
--- a/vl.c
+++ b/vl.c
@@ -516,6 +516,32 @@  static QemuOptsList qemu_realtime_opts = {
     },
 };
 
+static QemuOptsList qemu_numa_opts = {
+    .name = "numa",
+    .implied_opt_name = "type",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_numa_opts.head),
+    .desc = {
+        {
+            .name = "type",
+            .type = QEMU_OPT_STRING,
+            .help = "node type"
+        },{
+            .name = "nodeid",
+            .type = QEMU_OPT_NUMBER,
+            .help = "node ID"
+        },{
+            .name = "mem",
+            .type = QEMU_OPT_SIZE,
+            .help = "memory size"
+        },{
+            .name = "cpus",
+            .type = QEMU_OPT_STRING,
+            .help = "cpu number or range"
+        },
+        { /* end of list */ }
+    },
+};
+
 const char *qemu_get_vm_name(void)
 {
     return qemu_name;
@@ -1349,56 +1375,37 @@  error:
     exit(1);
 }
 
-static void numa_add(const char *optarg)
+
+static int numa_add_cpus(const char *name, const char *value, void *opaque)
 {
-    char option[128];
-    char *endptr;
-    unsigned long long nodenr;
+    int *nodenr = opaque;
 
-    optarg = get_opt_name(option, 128, optarg, ',');
-    if (*optarg == ',') {
-        optarg++;
+    if (!strcmp(name, "cpu")) {
+        numa_node_parse_cpus(*nodenr, value);
     }
-    if (!strcmp(option, "node")) {
-
-        if (nb_numa_nodes >= MAX_NODES) {
-            fprintf(stderr, "qemu: too many NUMA nodes\n");
-            exit(1);
-        }
+    return 0;
+}
 
-        if (get_param_value(option, 128, "nodeid", optarg) == 0) {
-            nodenr = nb_numa_nodes;
-        } else {
-            if (parse_uint_full(option, &nodenr, 10) < 0) {
-                fprintf(stderr, "qemu: Invalid NUMA nodeid: %s\n", option);
-                exit(1);
-            }
-        }
+static int numa_init_func(QemuOpts *opts, void *opaque)
+{
+    uint64_t nodenr, mem_size;
 
-        if (nodenr >= MAX_NODES) {
-            fprintf(stderr, "qemu: invalid NUMA nodeid: %llu\n", nodenr);
-            exit(1);
-        }
+    nodenr = qemu_opt_get_number(opts, "nodeid", nb_numa_nodes++);
 
-        if (get_param_value(option, 128, "mem", optarg) == 0) {
-            node_mem[nodenr] = 0;
-        } else {
-            int64_t sval;
-            sval = strtosz(option, &endptr);
-            if (sval < 0 || *endptr) {
-                fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg);
-                exit(1);
-            }
-            node_mem[nodenr] = sval;
-        }
-        if (get_param_value(option, 128, "cpus", optarg) != 0) {
-            numa_node_parse_cpus(nodenr, option);
-        }
-        nb_numa_nodes++;
-    } else {
-        fprintf(stderr, "Invalid -numa option: %s\n", option);
+    if (nodenr >= MAX_NODES) {
+        fprintf(stderr, "qemu: Max number of NUMA nodes reached : %d\n",
+                (int)nodenr);
         exit(1);
     }
+
+    mem_size = qemu_opt_get_size(opts, "mem", 0);
+    node_mem[nodenr] = mem_size;
+
+    if (qemu_opt_foreach(opts, numa_add_cpus, &nodenr, 1) < 0) {
+        return -1;
+    }
+
+    return 0;
 }
 
 static QemuOptsList qemu_smp_opts = {
@@ -2933,6 +2940,7 @@  int main(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_object_opts);
     qemu_add_opts(&qemu_tpmdev_opts);
     qemu_add_opts(&qemu_realtime_opts);
+    qemu_add_opts(&qemu_numa_opts);
 
     runstate_init();
 
@@ -3119,7 +3127,16 @@  int main(int argc, char **argv, char **envp)
                 }
                 break;
             case QEMU_OPTION_numa:
-                numa_add(optarg);
+                olist = qemu_find_opts("numa");
+                opts = qemu_opts_parse(olist, optarg, 1);
+                if (!opts) {
+                    exit(1);
+                }
+                optarg = qemu_opt_get(opts, "type");
+                if (!optarg || strcmp(optarg, "node")) {
+                    fprintf(stderr, "qemu: Incorrect format for numa option\n");
+                    exit(1);
+                }
                 break;
             case QEMU_OPTION_display:
                 display_type = select_display(optarg);
@@ -4195,6 +4212,11 @@  int main(int argc, char **argv, char **envp)
 
     register_savevm_live(NULL, "ram", 0, 4, &savevm_ram_handlers, NULL);
 
+    if (qemu_opts_foreach(qemu_find_opts("numa"), numa_init_func,
+                          NULL, 1) != 0) {
+        exit(1);
+    }
+
     if (nb_numa_nodes > 0) {
         int i;