public inbox for qemu-devel@nongnu.org
 help / color / mirror / Atom feed
* [PULL 0/3] QTest patches for 2026-03-11
@ 2026-03-11 22:13 Fabiano Rosas
  2026-03-11 22:13 ` [PULL 1/3] tests/qtest/vhost-user-test: Use g_timeout_add() to schedule connect Fabiano Rosas
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Fabiano Rosas @ 2026-03-11 22:13 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit 1fd5ff9d76d23ab23a68419cbc76d5ee33e8b455:

  Merge tag 'for-upstream' of https://gitlab.com/kmwolf/qemu into staging (2026-03-10 16:29:24 +0000)

are available in the Git repository at:

  https://gitlab.com/farosas/qemu.git tags/qtest-20260311-pull-request

for you to fetch changes up to 6ba08f4acec830d12f6983b0988c901258654a12:

  tests/qtest/test-x86-cpuid-compat: Switch test_feature_flag to data-driven (2026-03-11 18:43:20 -0300)

----------------------------------------------------------------
Qtest pull request

- memory leak fixes

----------------------------------------------------------------

Peter Maydell (3):
  tests/qtest/vhost-user-test: Use g_timeout_add() to schedule connect
  tests/qtest/test-x86-cpuid-compat: Switch test_cpuid_prop to
    data-driven
  tests/qtest/test-x86-cpuid-compat: Switch test_feature_flag to
    data-driven

 tests/qtest/test-x86-cpuid-compat.c | 410 ++++++++++++++++------------
 tests/qtest/vhost-user-test.c       |  36 ++-
 2 files changed, 262 insertions(+), 184 deletions(-)

-- 
2.51.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PULL 1/3] tests/qtest/vhost-user-test: Use g_timeout_add() to schedule connect
  2026-03-11 22:13 [PULL 0/3] QTest patches for 2026-03-11 Fabiano Rosas
@ 2026-03-11 22:13 ` Fabiano Rosas
  2026-03-11 22:13 ` [PULL 2/3] tests/qtest/test-x86-cpuid-compat: Switch test_cpuid_prop to data-driven Fabiano Rosas
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Fabiano Rosas @ 2026-03-11 22:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell

From: Peter Maydell <peter.maydell@linaro.org>

In vhost-user-test, we currently create a new g_thread to run the
connect_thread() function.  This function sleeps for 1 second, and
then calls test_server_create_chr() to create and configure a
chardev:

    chr = qemu_chr_new(server->chr_name, chr_path, server->context);
    g_assert(chr);

    qemu_chr_fe_init(&server->chr, chr, &error_abort);
    qemu_chr_fe_set_handlers(&server->chr, chr_can_read, chr_read,
                             chr_event, NULL, server, server->context, true);

This has a race condition, because when we set the
'reconnect-ms=1000' option on the chardev the socket chardev's
implementation handles the connect asynchronously, via a background
thread and a callback invoked in the main-loop thread.  This means
that that callback and the test_server_create_chr() call to
qemu_chr_fe_set_handlers() can both enter the char-socket code
simultaneously.  The result is random assertion failures and memory
leaks reported by the clang address-sanitizer.

Fix this by using g_timeout_source_new() to set up a GSource that
will run test_server_connect() on the main-loop thread.  This ensures
it can't execute in parallel with the callback that the socket
chardev sets up.  This is similar to how we already handle the
reconnect_cb() in test_reconnect().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/vhost-user-test.c | 36 ++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index d2bb21d5b9..c8b5f8ff71 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -930,26 +930,42 @@ reconnect_cb(gpointer user_data)
     return FALSE;
 }
 
-static gpointer
-connect_thread(gpointer data)
+static gboolean connect_cb(gpointer user_data)
 {
-    TestServer *s = data;
+    TestServer *s = user_data;
 
-    /* wait for qemu to start before first try, to avoid extra warnings */
-    g_usleep(G_USEC_PER_SEC);
     test_server_connect(s);
 
-    return NULL;
+    /* We only need to be called once */
+    return G_SOURCE_REMOVE;
+}
+
+/* Initial delay before connect, in milliseconds (1 second) */
+#define INITIAL_CONNECT_DELAY_MS (1 * 1000)
+
+static void test_schedule_connect(TestServer *s)
+{
+    /*
+     * Wait for a bit for QEMU to start before we first try to connect,
+     * to avoid extra warnings. We must run the "connect" on the
+     * main-loop thread so it doesn't race with a callback that
+     * the socket-chardev sets up on the main-loop.
+     */
+    GSource *src = g_timeout_source_new(INITIAL_CONNECT_DELAY_MS);
+    g_source_set_callback(src, connect_cb, s, NULL);
+    g_source_attach(src, s->context);
+    g_source_unref(src);
 }
 
 static void *vhost_user_test_setup_reconnect(GString *cmd_line, void *arg)
 {
     TestServer *s = test_server_new("reconnect", arg);
 
-    g_thread_unref(g_thread_new("connect", connect_thread, s));
     append_mem_opts(s, cmd_line, 256, TEST_MEMFD_AUTO);
     s->vu_ops->append_opts(s, cmd_line, ",server=on");
 
+    test_schedule_connect(s);
+
     g_test_queue_destroy(vhost_user_test_cleanup, s);
 
     return s;
@@ -983,10 +999,11 @@ static void *vhost_user_test_setup_connect_fail(GString *cmd_line, void *arg)
 
     s->test_fail = true;
 
-    g_thread_unref(g_thread_new("connect", connect_thread, s));
     append_mem_opts(s, cmd_line, 256, TEST_MEMFD_AUTO);
     s->vu_ops->append_opts(s, cmd_line, ",server=on");
 
+    test_schedule_connect(s);
+
     g_test_queue_destroy(vhost_user_test_cleanup, s);
 
     return s;
@@ -998,10 +1015,11 @@ static void *vhost_user_test_setup_flags_mismatch(GString *cmd_line, void *arg)
 
     s->test_flags = TEST_FLAGS_DISCONNECT;
 
-    g_thread_unref(g_thread_new("connect", connect_thread, s));
     append_mem_opts(s, cmd_line, 256, TEST_MEMFD_AUTO);
     s->vu_ops->append_opts(s, cmd_line, ",server=on");
 
+    test_schedule_connect(s);
+
     g_test_queue_destroy(vhost_user_test_cleanup, s);
 
     return s;
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PULL 2/3] tests/qtest/test-x86-cpuid-compat: Switch test_cpuid_prop to data-driven
  2026-03-11 22:13 [PULL 0/3] QTest patches for 2026-03-11 Fabiano Rosas
  2026-03-11 22:13 ` [PULL 1/3] tests/qtest/vhost-user-test: Use g_timeout_add() to schedule connect Fabiano Rosas
@ 2026-03-11 22:13 ` Fabiano Rosas
  2026-03-11 22:13 ` [PULL 3/3] tests/qtest/test-x86-cpuid-compat: Switch test_feature_flag " Fabiano Rosas
  2026-03-12 15:20 ` [PULL 0/3] QTest patches for 2026-03-11 Peter Maydell
  3 siblings, 0 replies; 5+ messages in thread
From: Fabiano Rosas @ 2026-03-11 22:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell

From: Peter Maydell <peter.maydell@linaro.org>

In test-x86-cpuid-compat, we allocate memory in add_cpuid_test()
which is then freed in the test function test_cpuid_prop().  This
means that this memory is leaked if the test is never run, which
happens in several cases:
 * user asks to list tests with -l option
 * user asks to run only some tests with -p or similar option
 * we are running a "subprocess" test and glib re-invokes the
   test binary telling it to run a specific single test

As noted in the commit message of commit 93ed7d330321dc, we cannot
deal with this by using g_test_add_data_func_full() and passing a
free-function, because glib doesn't call that if the test is not
executed.

Instead, move all the data allocations into test_cpuid_prop(),
and pass it a pointer to a struct with the necessary parameters
which is a compile-time constant.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/test-x86-cpuid-compat.c | 278 ++++++++++++++++------------
 1 file changed, 164 insertions(+), 114 deletions(-)

diff --git a/tests/qtest/test-x86-cpuid-compat.c b/tests/qtest/test-x86-cpuid-compat.c
index be67631a85..fc90a46d37 100644
--- a/tests/qtest/test-x86-cpuid-compat.c
+++ b/tests/qtest/test-x86-cpuid-compat.c
@@ -44,19 +44,43 @@ static bool qom_get_bool(const char *path, const char *prop)
 }
 
 typedef struct CpuidTestArgs {
-    const char *cmdline;
+    /* test name */
+    const char *name;
+    /* CPU type */
+    const char *cpu;
+    /* CPU features (may be NULL) */
+    const char *cpufeat;
+    /* machine type (may be NULL to use default machine) */
+    const char *machine;
+    /* CPU property to read */
     const char *property;
+    /* expected value of the property */
     int64_t expected_value;
 } CpuidTestArgs;
 
 static void test_cpuid_prop(const void *data)
 {
     const CpuidTestArgs *args = data;
+    char *cmdline;
+    char *save;
     char *path;
     QNum *value;
     int64_t val;
 
-    qtest_start(args->cmdline);
+    cmdline = g_strdup_printf("-cpu %s", args->cpu);
+
+    if (args->cpufeat) {
+        save = cmdline;
+        cmdline = g_strdup_printf("%s,%s", cmdline, args->cpufeat);
+        g_free(save);
+    }
+    if (args->machine) {
+        save = cmdline;
+        cmdline = g_strdup_printf("-machine %s %s", args->machine, cmdline);
+        g_free(save);
+    }
+
+    qtest_start(cmdline);
     path = get_cpu0_qom_path();
     value = qobject_to(QNum, qom_get(path, args->property));
     g_assert(qnum_get_try_int(value, &val));
@@ -65,40 +89,9 @@ static void test_cpuid_prop(const void *data)
 
     qobject_unref(value);
     g_free(path);
-    g_free((void *)args->cmdline);
-    g_free((void *)data);
+    g_free(cmdline);
 }
 
-static void add_cpuid_test(const char *name, const char *cpu,
-                           const char *cpufeat, const char *machine,
-                           const char *property, int64_t expected_value)
-{
-    CpuidTestArgs *args = g_new0(CpuidTestArgs, 1);
-    char *cmdline;
-    char *save;
-
-    if (!qtest_has_cpu_model(cpu)) {
-        return;
-    }
-    cmdline = g_strdup_printf("-cpu %s", cpu);
-
-    if (cpufeat) {
-        save = cmdline;
-        cmdline = g_strdup_printf("%s,%s", cmdline, cpufeat);
-        g_free(save);
-    }
-    if (machine) {
-        save = cmdline;
-        cmdline = g_strdup_printf("-machine %s %s", machine, cmdline);
-        g_free(save);
-    }
-    args->cmdline = cmdline;
-    args->property = property;
-    args->expected_value = expected_value;
-    qtest_add_data_func(name, args, test_cpuid_prop);
-}
-
-
 /* Parameters to a add_feature_test() test case */
 typedef struct FeatureTestArgs {
     /* cmdline to start QEMU */
@@ -249,103 +242,160 @@ static void test_plus_minus(void)
     g_test_trap_assert_stdout("");
 }
 
-int main(int argc, char **argv)
-{
-    g_test_init(&argc, &argv, NULL);
-
-    g_test_add_func("/x86/cpuid/parsing-plus-minus/subprocess",
-                    test_plus_minus_subprocess);
-    g_test_add_func("/x86/cpuid/parsing-plus-minus", test_plus_minus);
-
+static const CpuidTestArgs cpuid_tests[] = {
     /* Original level values for CPU models: */
-    add_cpuid_test("x86/cpuid/phenom/level",
-                   "phenom", NULL, NULL, "level", 5);
-    add_cpuid_test("x86/cpuid/Conroe/level",
-                   "Conroe", NULL, NULL, "level", 10);
-    add_cpuid_test("x86/cpuid/SandyBridge/level",
-                   "SandyBridge", NULL, NULL, "level", 0xd);
-    add_cpuid_test("x86/cpuid/486/xlevel",
-                   "486", NULL, NULL, "xlevel", 0);
-    add_cpuid_test("x86/cpuid/core2duo/xlevel",
-                   "core2duo", NULL, NULL, "xlevel", 0x80000008);
-    add_cpuid_test("x86/cpuid/phenom/xlevel",
-                   "phenom", NULL, NULL, "xlevel", 0x8000001A);
-    add_cpuid_test("x86/cpuid/athlon/xlevel",
-                   "athlon", NULL, NULL, "xlevel", 0x80000008);
-
+    {
+        "x86/cpuid/phenom/level",
+        "phenom", NULL, NULL, "level", 5,
+    },
+    {
+        "x86/cpuid/Conroe/level",
+        "Conroe", NULL, NULL, "level", 10,
+    },
+    {
+        "x86/cpuid/SandyBridge/level",
+        "SandyBridge", NULL, NULL, "level", 0xd,
+    },
+    {
+        "x86/cpuid/486/xlevel",
+        "486", NULL, NULL, "xlevel", 0,
+    },
+    {
+        "x86/cpuid/core2duo/xlevel",
+        "core2duo", NULL, NULL, "xlevel", 0x80000008,
+    },
+    {
+        "x86/cpuid/phenom/xlevel",
+        "phenom", NULL, NULL, "xlevel", 0x8000001A,
+    },
+    {
+        "x86/cpuid/athlon/xlevel",
+        "athlon", NULL, NULL, "xlevel", 0x80000008,
+    },
     /* If level is not large enough, it should increase automatically: */
     /* CPUID[6].EAX: */
-    add_cpuid_test("x86/cpuid/auto-level/486/arat",
-                   "486", "arat=on", NULL, "level", 6);
+    {
+        "x86/cpuid/auto-level/486/arat",
+        "486", "arat=on", NULL, "level", 6,
+    },
     /* CPUID[EAX=7,ECX=0].EBX: */
-    add_cpuid_test("x86/cpuid/auto-level/phenom/fsgsbase",
-                   "phenom", "fsgsbase=on", NULL, "level", 7);
+    {
+        "x86/cpuid/auto-level/phenom/fsgsbase",
+        "phenom", "fsgsbase=on", NULL, "level", 7,
+    },
     /* CPUID[EAX=7,ECX=0].ECX: */
-    add_cpuid_test("x86/cpuid/auto-level/phenom/avx512vbmi",
-                   "phenom", "avx512vbmi=on", NULL, "level", 7);
+    {
+        "x86/cpuid/auto-level/phenom/avx512vbmi",
+        "phenom", "avx512vbmi=on", NULL, "level", 7,
+    },
     /* CPUID[EAX=0xd,ECX=1].EAX: */
-    add_cpuid_test("x86/cpuid/auto-level/phenom/xsaveopt",
-                   "phenom", "xsaveopt=on", NULL, "level", 0xd);
+    {
+        "x86/cpuid/auto-level/phenom/xsaveopt",
+        "phenom", "xsaveopt=on", NULL, "level", 0xd,
+    },
     /* CPUID[8000_0001].EDX: */
-    add_cpuid_test("x86/cpuid/auto-xlevel/486/3dnow",
-                   "486", "3dnow=on", NULL, "xlevel", 0x80000001);
+    {
+        "x86/cpuid/auto-xlevel/486/3dnow",
+        "486", "3dnow=on", NULL, "xlevel", 0x80000001,
+    },
     /* CPUID[8000_0001].ECX: */
-    add_cpuid_test("x86/cpuid/auto-xlevel/486/sse4a",
-                   "486", "sse4a=on", NULL, "xlevel", 0x80000001);
+    {
+        "x86/cpuid/auto-xlevel/486/sse4a",
+        "486", "sse4a=on", NULL, "xlevel", 0x80000001,
+    },
     /* CPUID[8000_0007].EDX: */
-    add_cpuid_test("x86/cpuid/auto-xlevel/486/invtsc",
-                   "486", "invtsc=on", NULL, "xlevel", 0x80000007);
+    {
+        "x86/cpuid/auto-xlevel/486/invtsc",
+        "486", "invtsc=on", NULL, "xlevel", 0x80000007,
+    },
     /* CPUID[8000_000A].EDX: */
-    add_cpuid_test("x86/cpuid/auto-xlevel/486/npt",
-                   "486", "svm=on,npt=on", NULL, "xlevel", 0x8000000A);
+    {
+        "x86/cpuid/auto-xlevel/486/npt",
+        "486", "svm=on,npt=on", NULL, "xlevel", 0x8000000A,
+    },
     /* CPUID[C000_0001].EDX: */
-    add_cpuid_test("x86/cpuid/auto-xlevel2/phenom/xstore",
-                   "phenom", "xstore=on", NULL, "xlevel2", 0xC0000001);
+    {
+        "x86/cpuid/auto-xlevel2/phenom/xstore",
+        "phenom", "xstore=on", NULL, "xlevel2", 0xC0000001,
+    },
     /* SVM needs CPUID[0x8000000A] */
-    add_cpuid_test("x86/cpuid/auto-xlevel/athlon/svm",
-                   "athlon", "svm=on", NULL, "xlevel", 0x8000000A);
-
-
+    {
+        "x86/cpuid/auto-xlevel/athlon/svm",
+        "athlon", "svm=on", NULL, "xlevel", 0x8000000A,
+    },
     /* If level is already large enough, it shouldn't change: */
-    add_cpuid_test("x86/cpuid/auto-level/SandyBridge/multiple",
-                   "SandyBridge", "arat=on,fsgsbase=on,avx512vbmi=on",
-                   NULL, "level", 0xd);
+    {
+        "x86/cpuid/auto-level/SandyBridge/multiple",
+        "SandyBridge", "arat=on,fsgsbase=on,avx512vbmi=on",
+        NULL, "level", 0xd,
+    },
     /* If level is explicitly set, it shouldn't change: */
-    add_cpuid_test("x86/cpuid/auto-level/486/fixed/0xF",
-                   "486",
-                   "level=0xF,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
-                   NULL, "level", 0xF);
-    add_cpuid_test("x86/cpuid/auto-level/486/fixed/2",
-                   "486",
-                   "level=2,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
-                   NULL, "level", 2);
-    add_cpuid_test("x86/cpuid/auto-level/486/fixed/0",
-                   "486",
-                   "level=0,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
-                   NULL, "level", 0);
-
+    {
+        "x86/cpuid/auto-level/486/fixed/0xF",
+        "486",
+        "level=0xF,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
+        NULL, "level", 0xF,
+    },
+    {
+        "x86/cpuid/auto-level/486/fixed/2",
+        "486",
+        "level=2,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
+        NULL, "level", 2,
+    },
+    {
+        "x86/cpuid/auto-level/486/fixed/0",
+        "486",
+        "level=0,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
+        NULL, "level", 0,
+    },
     /* if xlevel is already large enough, it shouldn't change: */
-    add_cpuid_test("x86/cpuid/auto-xlevel/phenom/3dnow",
-                   "phenom", "3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
-                   NULL, "xlevel", 0x8000001A);
+    {
+        "x86/cpuid/auto-xlevel/phenom/3dnow",
+        "phenom", "3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
+        NULL, "xlevel", 0x8000001A,
+    },
     /* If xlevel is explicitly set, it shouldn't change: */
-    add_cpuid_test("x86/cpuid/auto-xlevel/486/fixed/80000002",
-                   "486",
-                   "xlevel=0x80000002,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
-                   NULL, "xlevel", 0x80000002);
-    add_cpuid_test("x86/cpuid/auto-xlevel/486/fixed/8000001A",
-                   "486",
-                   "xlevel=0x8000001A,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
-                   NULL, "xlevel", 0x8000001A);
-    add_cpuid_test("x86/cpuid/auto-xlevel/phenom/fixed/0",
-                   "486",
-                   "xlevel=0,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
-                   NULL, "xlevel", 0);
-
+    {
+        "x86/cpuid/auto-xlevel/486/fixed/80000002",
+        "486",
+        "xlevel=0x80000002,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
+        NULL, "xlevel", 0x80000002,
+    },
+    {
+        "x86/cpuid/auto-xlevel/486/fixed/8000001A",
+        "486",
+        "xlevel=0x8000001A,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
+        NULL, "xlevel", 0x8000001A,
+    },
+    {
+        "x86/cpuid/auto-xlevel/phenom/fixed/0",
+        "486",
+        "xlevel=0,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
+        NULL, "xlevel", 0,
+    },
     /* if xlevel2 is already large enough, it shouldn't change: */
-    add_cpuid_test("x86/cpuid/auto-xlevel2/486/fixed",
-                   "486", "xlevel2=0xC0000002,xstore=on",
-                   NULL, "xlevel2", 0xC0000002);
+    {
+        "x86/cpuid/auto-xlevel2/486/fixed",
+        "486", "xlevel2=0xC0000002,xstore=on",
+        NULL, "xlevel2", 0xC0000002,
+    },
+};
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    g_test_add_func("/x86/cpuid/parsing-plus-minus/subprocess",
+                    test_plus_minus_subprocess);
+    g_test_add_func("/x86/cpuid/parsing-plus-minus", test_plus_minus);
+
+    for (int i = 0; i < ARRAY_SIZE(cpuid_tests); i++) {
+        if (!qtest_has_cpu_model(cpuid_tests[i].cpu)) {
+            continue;
+        }
+        qtest_add_data_func(cpuid_tests[i].name,
+                            &cpuid_tests[i], test_cpuid_prop);
+    }
 
     /* Test feature parsing */
     add_feature_test("x86/cpuid/features/plus",
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PULL 3/3] tests/qtest/test-x86-cpuid-compat: Switch test_feature_flag to data-driven
  2026-03-11 22:13 [PULL 0/3] QTest patches for 2026-03-11 Fabiano Rosas
  2026-03-11 22:13 ` [PULL 1/3] tests/qtest/vhost-user-test: Use g_timeout_add() to schedule connect Fabiano Rosas
  2026-03-11 22:13 ` [PULL 2/3] tests/qtest/test-x86-cpuid-compat: Switch test_cpuid_prop to data-driven Fabiano Rosas
@ 2026-03-11 22:13 ` Fabiano Rosas
  2026-03-12 15:20 ` [PULL 0/3] QTest patches for 2026-03-11 Peter Maydell
  3 siblings, 0 replies; 5+ messages in thread
From: Fabiano Rosas @ 2026-03-11 22:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell

From: Peter Maydell <peter.maydell@linaro.org>

As with add_cpuid_test(), the add_feature_test() function also
allocates memory that is leaked if the test case is not run.  Fix
this in the same way, by moving all the allocations into
test_feature_flag() and passing it a pointer to a struct with the
necessary parameters which is a compile-time constant.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/test-x86-cpuid-compat.c | 134 +++++++++++++++-------------
 1 file changed, 72 insertions(+), 62 deletions(-)

diff --git a/tests/qtest/test-x86-cpuid-compat.c b/tests/qtest/test-x86-cpuid-compat.c
index fc90a46d37..17c0965827 100644
--- a/tests/qtest/test-x86-cpuid-compat.c
+++ b/tests/qtest/test-x86-cpuid-compat.c
@@ -94,8 +94,12 @@ static void test_cpuid_prop(const void *data)
 
 /* Parameters to a add_feature_test() test case */
 typedef struct FeatureTestArgs {
-    /* cmdline to start QEMU */
-    const char *cmdline;
+    /* Test name */
+    const char *name;
+    /* CPU type */
+    const char *cpu;
+    /* CPU features, may be NULL */
+    const char *cpufeat;
     /*
      * cpuid-input-eax and cpuid-input-ecx values to look for,
      * in "feature-words" and "filtered-features" properties.
@@ -140,10 +144,17 @@ static void test_feature_flag(const void *data)
 {
     const FeatureTestArgs *args = data;
     char *path;
+    char *cmdline;
     QList *present, *filtered;
     uint32_t value;
 
-    qtest_start(args->cmdline);
+    if (args->cpufeat) {
+        cmdline = g_strdup_printf("-cpu %s,%s", args->cpu, args->cpufeat);
+    } else {
+        cmdline = g_strdup_printf("-cpu %s", args->cpu);
+    }
+
+    qtest_start(cmdline);
     path = get_cpu0_qom_path();
     present = qobject_to(QList, qom_get(path, "feature-words"));
     filtered = qobject_to(QList, qom_get(path, "filtered-features"));
@@ -156,40 +167,7 @@ static void test_feature_flag(const void *data)
     qobject_unref(present);
     qobject_unref(filtered);
     g_free(path);
-    g_free((void *)args->cmdline);
-    g_free((void *)data);
-}
-
-/*
- * Add test case to ensure that a given feature flag is set in
- * either "feature-words" or "filtered-features", when running QEMU
- * using cmdline
- */
-static void add_feature_test(const char *name, const char *cpu,
-                             const char *cpufeat, uint32_t eax,
-                             uint32_t ecx, const char *reg,
-                             int bitnr, bool expected_value)
-{
-    FeatureTestArgs *args = g_new0(FeatureTestArgs, 1);
-    char *cmdline;
-
-    if (!qtest_has_cpu_model(cpu)) {
-        return;
-    }
-
-    if (cpufeat) {
-        cmdline = g_strdup_printf("-cpu %s,%s", cpu, cpufeat);
-    } else {
-        cmdline = g_strdup_printf("-cpu %s", cpu);
-    }
-
-    args->cmdline = cmdline;
-    args->in_eax = eax;
-    args->in_ecx = ecx;
-    args->reg = reg;
-    args->bitnr = bitnr;
-    args->expected_value = expected_value;
-    qtest_add_data_func(name, args, test_feature_flag);
+    g_free(cmdline);
 }
 
 static void test_plus_minus_subprocess(void)
@@ -381,6 +359,56 @@ static const CpuidTestArgs cpuid_tests[] = {
     },
 };
 
+/*
+ * Test cases to ensure that a given feature flag is set in
+ * either "feature-words" or "filtered-features", when running QEMU
+ * using cmdline
+ */
+static const FeatureTestArgs feature_tests[] = {
+    /* Test feature parsing */
+    {
+        "x86/cpuid/features/plus",
+        "486", "+arat",
+        6, 0, "EAX", 2, true,
+    },
+    {
+        "x86/cpuid/features/minus",
+        "pentium", "-mmx",
+        1, 0, "EDX", 23, false,
+    },
+    {
+        "x86/cpuid/features/on",
+        "486", "arat=on",
+        6, 0, "EAX", 2, true,
+    },
+    {
+        "x86/cpuid/features/off",
+        "pentium", "mmx=off",
+        1, 0, "EDX", 23, false,
+    },
+
+    {
+        "x86/cpuid/features/max-plus-invtsc",
+        "max" , "+invtsc",
+        0x80000007, 0, "EDX", 8, true,
+    },
+    {
+        "x86/cpuid/features/max-invtsc-on",
+        "max", "invtsc=on",
+        0x80000007, 0, "EDX", 8, true,
+    },
+    {
+        "x86/cpuid/features/max-minus-mmx",
+        "max", "-mmx",
+        1, 0, "EDX", 23, false,
+    },
+    {
+        "x86/cpuid/features/max-invtsc-on,mmx=off",
+        "max", "mmx=off",
+        1, 0, "EDX", 23, false,
+    },
+};
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
@@ -397,32 +425,14 @@ int main(int argc, char **argv)
                             &cpuid_tests[i], test_cpuid_prop);
     }
 
-    /* Test feature parsing */
-    add_feature_test("x86/cpuid/features/plus",
-                     "486", "+arat",
-                     6, 0, "EAX", 2, true);
-    add_feature_test("x86/cpuid/features/minus",
-                     "pentium", "-mmx",
-                     1, 0, "EDX", 23, false);
-    add_feature_test("x86/cpuid/features/on",
-                     "486", "arat=on",
-                     6, 0, "EAX", 2, true);
-    add_feature_test("x86/cpuid/features/off",
-                     "pentium", "mmx=off",
-                     1, 0, "EDX", 23, false);
 
-    add_feature_test("x86/cpuid/features/max-plus-invtsc",
-                     "max" , "+invtsc",
-                     0x80000007, 0, "EDX", 8, true);
-    add_feature_test("x86/cpuid/features/max-invtsc-on",
-                     "max", "invtsc=on",
-                     0x80000007, 0, "EDX", 8, true);
-    add_feature_test("x86/cpuid/features/max-minus-mmx",
-                     "max", "-mmx",
-                     1, 0, "EDX", 23, false);
-    add_feature_test("x86/cpuid/features/max-invtsc-on,mmx=off",
-                     "max", "mmx=off",
-                     1, 0, "EDX", 23, false);
+    for (int i = 0; i < ARRAY_SIZE(feature_tests); i++) {
+        if (!qtest_has_cpu_model(feature_tests[i].cpu)) {
+            continue;
+        }
+        qtest_add_data_func(feature_tests[i].name,
+                            &feature_tests[i], test_feature_flag);
+    }
 
     return g_test_run();
 }
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PULL 0/3] QTest patches for 2026-03-11
  2026-03-11 22:13 [PULL 0/3] QTest patches for 2026-03-11 Fabiano Rosas
                   ` (2 preceding siblings ...)
  2026-03-11 22:13 ` [PULL 3/3] tests/qtest/test-x86-cpuid-compat: Switch test_feature_flag " Fabiano Rosas
@ 2026-03-12 15:20 ` Peter Maydell
  3 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2026-03-12 15:20 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: qemu-devel

On Wed, 11 Mar 2026 at 22:14, Fabiano Rosas <farosas@suse.de> wrote:
>
> The following changes since commit 1fd5ff9d76d23ab23a68419cbc76d5ee33e8b455:
>
>   Merge tag 'for-upstream' of https://gitlab.com/kmwolf/qemu into staging (2026-03-10 16:29:24 +0000)
>
> are available in the Git repository at:
>
>   https://gitlab.com/farosas/qemu.git tags/qtest-20260311-pull-request
>
> for you to fetch changes up to 6ba08f4acec830d12f6983b0988c901258654a12:
>
>   tests/qtest/test-x86-cpuid-compat: Switch test_feature_flag to data-driven (2026-03-11 18:43:20 -0300)
>
> ----------------------------------------------------------------
> Qtest pull request
>
> - memory leak fixes
>
> ----------------------------------------------------------------



Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/11.0
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-03-12 15:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-11 22:13 [PULL 0/3] QTest patches for 2026-03-11 Fabiano Rosas
2026-03-11 22:13 ` [PULL 1/3] tests/qtest/vhost-user-test: Use g_timeout_add() to schedule connect Fabiano Rosas
2026-03-11 22:13 ` [PULL 2/3] tests/qtest/test-x86-cpuid-compat: Switch test_cpuid_prop to data-driven Fabiano Rosas
2026-03-11 22:13 ` [PULL 3/3] tests/qtest/test-x86-cpuid-compat: Switch test_feature_flag " Fabiano Rosas
2026-03-12 15:20 ` [PULL 0/3] QTest patches for 2026-03-11 Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox