[Qemu-devel] [PATCH v14 15/19] numa: convert to use QObjectInputVisitor for -numa

["Daniel P. Berrange" <berrange@redhat.com>]

Switch away from using OptsVisitor to parse the -numa
argument processing. This enables use of the modern
list syntax for specifying CPUs. e.g. the old syntax

  -numa node,nodeid=0,cpus=0-3,cpus=8-11,mem=107

is equivalent to

  -numa node,nodeid=0,cpus.0=0,cpus.1=1,cpus.2=2,cpus.3=3,\
        cpus.4=8,cpus.5=9,cpus.6=10,cpus.7=11,mem=107

Furthermore, the cli arg can now follow the QAPI schema
nesting, so the above is equivalent to the canonical
syntax:

  -numa type=node,data.nodeid=0,data.cpus.0=0,data.cpus.1=1,\
        data.cpus.2=2,data.cpus.3=3,data.cpus.4=8,data.cpus.5=9,\
	data.cpus.6=10,data.cpus.7=11,data.mem=107

A test case is added to cover argument parsing to validate
that both the old and new syntax is correctly handled.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/sysemu/numa_int.h |  11 +++++
 numa.c                    |  36 +++++++++-----
 stubs/Makefile.objs       |   5 ++
 stubs/exec.c              |   6 +++
 stubs/hostmem.c           |  14 ++++++
 stubs/memory.c            |  41 ++++++++++++++++
 stubs/qdev.c              |   8 ++++
 stubs/vl.c                |   8 ++++
 stubs/vmstate.c           |   4 ++
 tests/Makefile.include    |   2 +
 tests/test-numa.c         | 116 ++++++++++++++++++++++++++++++++++++++++++++++
 11 files changed, 240 insertions(+), 11 deletions(-)
 create mode 100644 include/sysemu/numa_int.h
 create mode 100644 stubs/exec.c
 create mode 100644 stubs/hostmem.c
 create mode 100644 stubs/memory.c
 create mode 100644 stubs/qdev.c
 create mode 100644 stubs/vl.c
 create mode 100644 tests/test-numa.c

diff --git a/include/sysemu/numa_int.h b/include/sysemu/numa_int.h
new file mode 100644
index 0000000..93160da
--- /dev/null
+++ b/include/sysemu/numa_int.h
@@ -0,0 +1,11 @@
+#ifndef SYSEMU_NUMA_INT_H
+#define SYSEMU_NUMA_INT_H
+
+#include "sysemu/numa.h"
+
+extern int have_memdevs;
+extern int max_numa_nodeid;
+
+int parse_numa(void *opaque, QemuOpts *opts, Error **errp);
+
+#endif
diff --git a/numa.c b/numa.c
index 6289f46..5daa3d1 100644
--- a/numa.c
+++ b/numa.c
@@ -23,14 +23,14 @@
  */
 
 #include "qemu/osdep.h"
-#include "sysemu/numa.h"
+#include "sysemu/numa_int.h"
 #include "exec/cpu-common.h"
 #include "qemu/bitmap.h"
 #include "qom/cpu.h"
 #include "qemu/error-report.h"
 #include "include/exec/cpu-common.h" /* for RAM_ADDR_FMT */
 #include "qapi-visit.h"
-#include "qapi/opts-visitor.h"
+#include "qapi/qobject-input-visitor.h"
 #include "hw/boards.h"
 #include "sysemu/hostmem.h"
 #include "qmp-commands.h"
@@ -45,10 +45,10 @@ QemuOptsList qemu_numa_opts = {
     .desc = { { 0 } } /* validated with OptsVisitor */
 };
 
-static int have_memdevs = -1;
-static int max_numa_nodeid; /* Highest specified NUMA node ID, plus one.
-                             * For all nodes, nodeid < max_numa_nodeid
-                             */
+int have_memdevs = -1;
+int max_numa_nodeid; /* Highest specified NUMA node ID, plus one.
+                      * For all nodes, nodeid < max_numa_nodeid
+                      */
 int nb_numa_nodes;
 NodeInfo numa_info[MAX_NODES];
 
@@ -189,6 +189,9 @@ static void numa_node_parse(NumaNodeOptions *node, QemuOpts *opts, Error **errp)
     if (node->has_mem) {
         uint64_t mem_size = node->mem;
         const char *mem_str = qemu_opt_get(opts, "mem");
+        if (!mem_str) {
+            mem_str = qemu_opt_get(opts, "data.mem");
+        }
         /* Fix up legacy suffix-less format */
         if (g_ascii_isdigit(mem_str[strlen(mem_str) - 1])) {
             mem_size <<= 20;
@@ -211,16 +214,27 @@ static void numa_node_parse(NumaNodeOptions *node, QemuOpts *opts, Error **errp)
     max_numa_nodeid = MAX(max_numa_nodeid, nodenr + 1);
 }
 
-static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
+int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
 {
     NumaOptions *object = NULL;
     Error *err = NULL;
+    Visitor *v;
 
-    {
-        Visitor *v = opts_visitor_new(opts);
-        visit_type_NumaOptions(v, NULL, &object, &err);
-        visit_free(v);
+    /*
+     * Needs autocreate_list=true and permit_int_ranges=true
+     * in order to support existing syntax for 'cpus' parameter
+     * which is an int list.
+     *
+     * Needs autocreate_struct_levels=1, because existing syntax
+     * used a nested struct in the QMP schema with a flat namespace
+     * in the CLI args.
+     */
+    v = qobject_input_visitor_new_opts(opts, true, 1, true, &err);
+    if (err) {
+        goto end;
     }
+    visit_type_NumaOptions(v, NULL, &object, &err);
+    visit_free(v);
 
     if (err) {
         goto end;
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index c5850e8..661b48a 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -48,3 +48,8 @@ stub-obj-y += iohandler.o
 stub-obj-y += smbios_type_38.o
 stub-obj-y += ipmi.o
 stub-obj-y += pc_madt_cpu_entry.o
+stub-obj-y += vl.o
+stub-obj-y += exec.o
+stub-obj-y += memory.o
+stub-obj-y += hostmem.o
+stub-obj-y += qdev.o
diff --git a/stubs/exec.c b/stubs/exec.c
new file mode 100644
index 0000000..e37f002
--- /dev/null
+++ b/stubs/exec.c
@@ -0,0 +1,6 @@
+
+#include "qemu/osdep.h"
+#include "exec/cpu-common.h"
+#include "qom/cpu.h"
+
+struct CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
diff --git a/stubs/hostmem.c b/stubs/hostmem.c
new file mode 100644
index 0000000..301d853
--- /dev/null
+++ b/stubs/hostmem.c
@@ -0,0 +1,14 @@
+
+#include "qemu/osdep.h"
+#include "sysemu/hostmem.h"
+
+void host_memory_backend_set_mapped(HostMemoryBackend *backend, bool mapped)
+{
+}
+
+
+MemoryRegion *host_memory_backend_get_memory(HostMemoryBackend *backend,
+                                             Error **errp)
+{
+    return NULL;
+}
diff --git a/stubs/memory.c b/stubs/memory.c
new file mode 100644
index 0000000..c849d9d
--- /dev/null
+++ b/stubs/memory.c
@@ -0,0 +1,41 @@
+
+#include "qemu/osdep.h"
+#include "exec/memory.h"
+
+void memory_region_init_ram(MemoryRegion *mr,
+                            struct Object *owner,
+                            const char *name,
+                            uint64_t size,
+                            Error **errp)
+{
+}
+
+#ifdef __linux__
+void memory_region_init_ram_from_file(MemoryRegion *mr,
+                                      struct Object *owner,
+                                      const char *name,
+                                      uint64_t size,
+                                      bool share,
+                                      const char *path,
+                                      Error **errp)
+{
+}
+#endif
+
+void memory_region_init(MemoryRegion *mr,
+                        struct Object *owner,
+                        const char *name,
+                        uint64_t size)
+{
+}
+
+void memory_region_add_subregion(MemoryRegion *mr,
+                                 hwaddr offset,
+                                 MemoryRegion *subregion)
+{
+}
+
+bool memory_region_is_mapped(MemoryRegion *mr)
+{
+    return false;
+}
diff --git a/stubs/qdev.c b/stubs/qdev.c
new file mode 100644
index 0000000..e28fdf2
--- /dev/null
+++ b/stubs/qdev.c
@@ -0,0 +1,8 @@
+
+#include "qemu/osdep.h"
+#include "hw/qdev-core.h"
+
+Object *qdev_get_machine(void)
+{
+    return NULL;
+}
diff --git a/stubs/vl.c b/stubs/vl.c
new file mode 100644
index 0000000..698fa80
--- /dev/null
+++ b/stubs/vl.c
@@ -0,0 +1,8 @@
+
+#include "qemu/osdep.h"
+#include "exec/cpu-common.h"
+
+int max_cpus = 1;
+ram_addr_t ram_size;
+const char *mem_path;
+int mem_prealloc;
diff --git a/stubs/vmstate.c b/stubs/vmstate.c
index 94b831e..f2708ee 100644
--- a/stubs/vmstate.c
+++ b/stubs/vmstate.c
@@ -23,3 +23,7 @@ void vmstate_unregister(DeviceState *dev,
                         void *opaque)
 {
 }
+
+void vmstate_register_ram_global(struct MemoryRegion *memory)
+{
+}
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 6398678..7625ef7 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -116,6 +116,7 @@ check-unit-$(CONFIG_REPLICATION) += tests/test-replication$(EXESUF)
 check-unit-y += tests/test-bufferiszero$(EXESUF)
 gcov-files-check-bufferiszero-y = util/bufferiszero.c
 check-unit-y += tests/test-uuid$(EXESUF)
+check-unit-y += tests/test-numa$(EXESUF)
 
 check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 
@@ -583,6 +584,7 @@ tests/test-crypto-pbkdf$(EXESUF): tests/test-crypto-pbkdf.o $(test-crypto-obj-y)
 tests/test-crypto-ivgen$(EXESUF): tests/test-crypto-ivgen.o $(test-crypto-obj-y)
 tests/test-crypto-afsplit$(EXESUF): tests/test-crypto-afsplit.o $(test-crypto-obj-y)
 tests/test-crypto-block$(EXESUF): tests/test-crypto-block.o $(test-crypto-obj-y)
+tests/test-numa$(EXESUF): tests/test-numa.o  numa.o $(test-qom-obj-y)
 
 libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
 libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
diff --git a/tests/test-numa.c b/tests/test-numa.c
new file mode 100644
index 0000000..458681b
--- /dev/null
+++ b/tests/test-numa.c
@@ -0,0 +1,116 @@
+/*
+ * QEMU NUMA testing
+ *
+ * Copyright (c) 2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+
+#include "sysemu/numa_int.h"
+
+static void test_numa_parse(const char **nodestr)
+{
+    QemuOpts *opts;
+    size_t i;
+
+    DECLARE_BITMAP(node0cpus, MAX_CPUMASK_BITS);
+    DECLARE_BITMAP(node5cpus, MAX_CPUMASK_BITS);
+
+    bitmap_zero(node0cpus, MAX_CPUMASK_BITS);
+    bitmap_zero(node5cpus, MAX_CPUMASK_BITS);
+    for (i = 0; i <= 3; i++) {
+        bitmap_set(node0cpus, i, 1);
+    }
+    for (i = 8; i <= 11; i++) {
+        bitmap_set(node0cpus, i, 1);
+    }
+    for (i = 4; i <= 7; i++) {
+        bitmap_set(node5cpus, i, 1);
+    }
+    for (i = 12; i <= 15; i++) {
+        bitmap_set(node5cpus, i, 1);
+    }
+
+    max_cpus = 16;
+
+    opts = qemu_opts_parse_noisily(&qemu_numa_opts,
+                                   nodestr[0], true);
+    g_assert(opts != NULL);
+
+    opts = qemu_opts_parse_noisily(&qemu_numa_opts,
+                                   nodestr[1], true);
+    g_assert(opts != NULL);
+
+    qemu_opts_foreach(&qemu_numa_opts, parse_numa, NULL, NULL);
+
+    g_assert_cmpint(max_numa_nodeid, ==, 6);
+    g_assert(!have_memdevs);
+
+    g_assert_cmpint(nb_numa_nodes, ==, 2);
+    for (i = 0; i < MAX_NODES; i++) {
+        if (i == 0 || i == 5) {
+            g_assert(numa_info[i].present);
+            g_assert_cmpint(numa_info[i].node_mem, ==, 107 * 1024 * 1024);
+
+            if (i == 0) {
+                g_assert(bitmap_equal(node0cpus,
+                                      numa_info[i].node_cpu,
+                                      MAX_CPUMASK_BITS));
+            } else {
+                g_assert(bitmap_equal(node5cpus,
+                                      numa_info[i].node_cpu,
+                                      MAX_CPUMASK_BITS));
+            }
+        } else {
+            g_assert(!numa_info[i].present);
+        }
+    }
+
+    nb_numa_nodes = 0;
+    max_numa_nodeid = 0;
+    memset(&numa_info, 0, sizeof(numa_info));
+    g_assert(!numa_info[0].present);
+    qemu_opts_reset(&qemu_numa_opts);
+}
+
+static void test_numa_parse_legacy(void)
+{
+    const char *nodestr[] = {
+        "node,nodeid=0,cpus=0-3,cpus=8-11,mem=107",
+        "node,nodeid=5,cpus=4-7,cpus=12-15,mem=107"
+    };
+    test_numa_parse(nodestr);
+}
+
+static void test_numa_parse_modern(void)
+{
+    const char *nodestr[] = {
+        "type=node,data.nodeid=0,data.cpus.0=0,data.cpus.1=1,data.cpus.2=2,data.cpus.3=3,"
+          "data.cpus.4=8,data.cpus.5=9,data.cpus.6=10,data.cpus.7=11,data.mem=107",
+        "type=node,data.nodeid=5,data.cpus.0=4,data.cpus.1=5,data.cpus.2=6,data.cpus.3=7,"
+          "data.cpus.4=12,data.cpus.5=13,data.cpus.6=14,data.cpus.7=15,data.mem=107",
+    };
+    test_numa_parse(nodestr);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+    g_test_add_func("/numa/parse/legacy", test_numa_parse_legacy);
+    g_test_add_func("/numa/parse/modern", test_numa_parse_modern);
+    return g_test_run();
+}
-- 
2.7.4