[Qemu-devel] [PATCH 0/2] tests: cpu-plug-test: fix x86 device_add cpu-foo test cases

[Qemu-devel] [PATCH 0/2] tests: cpu-plug-test: fix x86 device_add cpu-foo test cases

[Igor Mammedov]

Fixes bc1fb850a3 (vl.c deprecate incorrect CPUs topology) that introduced
regression.

Igor Mammedov (2):
  tests: add qtest_qmp_device_add_qdict() helper
  tests: cpu-plug-test: fix device_add for pc/q35 machines

 tests/libqtest.h      | 12 +++++++++
 tests/cpu-plug-test.c | 62 ++++++++++++++++++-------------------------
 tests/libqtest.c      | 29 +++++++++++++-------
 3 files changed, 57 insertions(+), 46 deletions(-)

-- 
2.18.1

[Qemu-devel] [PATCH 1/2] tests: add qtest_qmp_device_add_qdict() helper

[Igor Mammedov]

Add an API that takes QDict directly, so users could skip steps
of first building json dictionary and converting it back to
QDict in existing qtest_qmp_device_add() and instead use QDict
directly without intermediate conversion.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/libqtest.h | 12 ++++++++++++
 tests/libqtest.c | 29 +++++++++++++++++++----------
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/tests/libqtest.h b/tests/libqtest.h
index 07ea35867c..0788f70954 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -948,6 +948,18 @@ QDict *qmp_fd(int fd, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 void qtest_cb_for_every_machine(void (*cb)(const char *machine),
                                 bool skip_old_versioned);
 
+/**
+ * qtest_qmp_device_add_qdict:
+ * @qts: QTestState instance to operate on
+ * @drv: Name of the device that should be added
+ * @arguments: QDict with properties for the device to intialize
+ *
+ * Generic hot-plugging test via the device_add QMP command with properties
+ * supplied in form of QDict. Use NULL for empty properties list.
+ */
+void qtest_qmp_device_add_qdict(QTestState *qts, const char *drv,
+                                const QDict *arguments);
+
 /**
  * qtest_qmp_device_add:
  * @qts: QTestState instance to operate on
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 2713b86cf7..13d52f5cf3 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -1250,28 +1250,37 @@ QDict *qtest_qmp_receive_success(QTestState *s,
 }
 
 /*
- * Generic hot-plugging test via the device_add QMP command.
+ * Generic hot-plugging test via the device_add QMP commands.
  */
+void qtest_qmp_device_add_qdict(QTestState *qts, const char *drv,
+                                const QDict *arguments)
+{
+    QDict *resp;
+    QDict *args = arguments ? qdict_clone_shallow(arguments) : qdict_new();
+
+    g_assert(!qdict_haskey(args, "driver"));
+    qdict_put_str(args, "driver", drv);
+    resp = qtest_qmp(qts, "{'execute': 'device_add', 'arguments': %p}", args);
+    g_assert(resp);
+    g_assert(!qdict_haskey(resp, "event")); /* We don't expect any events */
+    g_assert(!qdict_haskey(resp, "error"));
+    qobject_unref(resp);
+}
+
 void qtest_qmp_device_add(QTestState *qts, const char *driver, const char *id,
                           const char *fmt, ...)
 {
-    QDict *args, *response;
+    QDict *args;
     va_list ap;
 
     va_start(ap, fmt);
     args = qdict_from_vjsonf_nofail(fmt, ap);
     va_end(ap);
 
-    g_assert(!qdict_haskey(args, "driver") && !qdict_haskey(args, "id"));
-    qdict_put_str(args, "driver", driver);
+    g_assert(!qdict_haskey(args, "id"));
     qdict_put_str(args, "id", id);
 
-    response = qtest_qmp(qts, "{'execute': 'device_add', 'arguments': %p}",
-                         args);
-    g_assert(response);
-    g_assert(!qdict_haskey(response, "event")); /* We don't expect any events */
-    g_assert(!qdict_haskey(response, "error"));
-    qobject_unref(response);
+    qtest_qmp_device_add_qdict(qts, driver, args);
 }
 
 static void device_deleted_cb(void *opaque, const char *name, QDict *data)
-- 
2.18.1

[Qemu-devel] [PATCH 2/2] tests: cpu-plug-test: fix device_add for pc/q35 machines

[Igor Mammedov]

Commit bc1fb850a3 silently broke device_add test for CPU hotplug which
resulted in test successfully passing though it wasn't actually run.
Fix it by making sure that all non present CPUs reported
by "query-hotpluggable-cpus" are hotplugged instead of making up
and hardcoding values.

Use of query-hotpluggable-cpus also allows consolidatiate device_add
cpu testcases and reuse the same test function for all targets.

While at it also add a check that at least one CPU was hotplugged,
to avoid silent breakage in the future.

Fixes: bc1fb850a3 (vl.c deprecate incorrect CPUs topology)
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/cpu-plug-test.c | 62 ++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 36 deletions(-)

diff --git a/tests/cpu-plug-test.c b/tests/cpu-plug-test.c
index 3049620854..46e51dc07a 100644
--- a/tests/cpu-plug-test.c
+++ b/tests/cpu-plug-test.c
@@ -12,6 +12,7 @@
 #include "qemu-common.h"
 #include "libqtest.h"
 #include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qlist.h"
 
 struct PlugTestData {
     char *machine;
@@ -72,12 +73,15 @@ static void test_plug_without_cpu_add(gconstpointer data)
     g_free(args);
 }
 
-static void test_plug_with_device_add_x86(gconstpointer data)
+static void test_plug_with_device_add(gconstpointer data)
 {
     const PlugTestData *td = data;
     char *args;
-    unsigned int s, c, t;
     QTestState *qts;
+    QDict *resp;
+    QList *cpus;
+    QObject *e;
+    int hotplugged = 0;
 
     args = g_strdup_printf("-machine %s -cpu %s "
                            "-smp 1,sockets=%u,cores=%u,threads=%u,maxcpus=%u",
@@ -85,43 +89,29 @@ static void test_plug_with_device_add_x86(gconstpointer data)
                            td->sockets, td->cores, td->threads, td->maxcpus);
     qts = qtest_init(args);
 
-    for (s = 1; s < td->sockets; s++) {
-        for (c = 0; c < td->cores; c++) {
-            for (t = 0; t < td->threads; t++) {
-                char *id = g_strdup_printf("id-%i-%i-%i", s, c, t);
-                qtest_qmp_device_add(qts, td->device_model, id,
-                                     "{'socket-id':%u, 'core-id':%u,"
-                                     " 'thread-id':%u}",
-                                     s, c, t);
-                g_free(id);
-            }
-        }
-    }
+    resp = qtest_qmp(qts, "{ 'execute': 'query-hotpluggable-cpus'}");
+    g_assert(qdict_haskey(resp, "return"));
+    cpus = qdict_get_qlist(resp, "return");
+    g_assert(cpus);
 
-    qtest_quit(qts);
-    g_free(args);
-}
+    while ((e = qlist_pop(cpus))) {
+        const QDict *cpu, *props;
 
-static void test_plug_with_device_add_coreid(gconstpointer data)
-{
-    const PlugTestData *td = data;
-    char *args;
-    unsigned int c;
-    QTestState *qts;
+        cpu = qobject_to(QDict, e);
+        if (qdict_haskey(cpu, "qom-path")) {
+            continue;
+        }
 
-    args = g_strdup_printf("-machine %s -cpu %s "
-                           "-smp 1,sockets=%u,cores=%u,threads=%u,maxcpus=%u",
-                           td->machine, td->cpu_model,
-                           td->sockets, td->cores, td->threads, td->maxcpus);
-    qts = qtest_init(args);
+        g_assert(qdict_haskey(cpu, "props"));
+        props = qdict_get_qdict(cpu, "props");
 
-    for (c = 1; c < td->cores; c++) {
-        char *id = g_strdup_printf("id-%i", c);
-        qtest_qmp_device_add(qts, td->device_model, id,
-                             "{'core-id':%u}", c);
-        g_free(id);
+        qtest_qmp_device_add_qdict(qts, td->device_model, props);
+        hotplugged++;
     }
 
+    /* make sure that there were hotplugged CPUs */
+    g_assert(hotplugged);
+    qobject_unref(resp);
     qtest_quit(qts);
     g_free(args);
 }
@@ -182,7 +172,7 @@ static void add_pc_test_case(const char *mname)
         path = g_strdup_printf("cpu-plug/%s/device-add/%ux%ux%u&maxcpus=%u",
                                mname, data2->sockets, data2->cores,
                                data2->threads, data2->maxcpus);
-        qtest_add_data_func_full(path, data2, test_plug_with_device_add_x86,
+        qtest_add_data_func_full(path, data2, test_plug_with_device_add,
                                  test_data_free);
         g_free(path);
     }
@@ -209,7 +199,7 @@ static void add_pseries_test_case(const char *mname)
     path = g_strdup_printf("cpu-plug/%s/device-add/%ux%ux%u&maxcpus=%u",
                            mname, data->sockets, data->cores,
                            data->threads, data->maxcpus);
-    qtest_add_data_func_full(path, data, test_plug_with_device_add_coreid,
+    qtest_add_data_func_full(path, data, test_plug_with_device_add,
                              test_data_free);
     g_free(path);
 }
@@ -246,7 +236,7 @@ static void add_s390x_test_case(const char *mname)
     path = g_strdup_printf("cpu-plug/%s/device-add/%ux%ux%u&maxcpus=%u",
                            mname, data2->sockets, data2->cores,
                            data2->threads, data2->maxcpus);
-    qtest_add_data_func_full(path, data2, test_plug_with_device_add_coreid,
+    qtest_add_data_func_full(path, data2, test_plug_with_device_add,
                              test_data_free);
     g_free(path);
 }
-- 
2.18.1

Re: [Qemu-devel] [PATCH 0/2] tests: cpu-plug-test: fix x86 device_add cpu-foo test cases

[Eduardo Habkost]

On Fri, Aug 30, 2019 at 07:07:21AM -0400, Igor Mammedov wrote:
> Fixes bc1fb850a3 (vl.c deprecate incorrect CPUs topology) that introduced
> regression.

Queued on machine-next.  Thanks!

-- 
Eduardo