1. Patchew
  2. QEMU
  3. View series

[PATCH] tests/qtest: optimize qtest_get_machines

Steve Sistare posted 1 patch 1 week, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1758290310-349623-1-git-send-email-steven.sistare@oracle.com
Maintainers: Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
tests/qtest/libqtest.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
[PATCH] tests/qtest: optimize qtest_get_machines
Posted by Steve Sistare 1 week, 2 days ago 
qtest_get_machines returns the machines supported by the QEMU binary
described by an environment variable and caches the result.  If the
next call to qtest_get_machines passes the same variable name, the cached
result is returned, but if the name changes, the caching is defeated.
To make caching more effective, remember the path of the QEMU binary
instead.  Different env vars, eg QTEST_QEMU_BINARY_SRC and
QTEST_QEMU_BINARY_DST, usually resolve to the same path.

Before the optimization, the test /x86_64/migration/precopy/unix/plain
exec's QEMU and calls query-machines 3 times.  After optimization, that
only happens once.  This does not significantly speed up the tests, but
it reduces QTEST_LOG output, and launches fewer QEMU instances, making
it easier to debug problems.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 tests/qtest/libqtest.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 94526b7..f3d4e08 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1630,7 +1630,8 @@ static void qtest_free_machine_list(struct MachInfo *machines)
 static struct MachInfo *qtest_get_machines(const char *var)
 {
     static struct MachInfo *machines;
-    static char *qemu_var;
+    static char *qemu_bin;
+    const char *new_qemu_bin;
     QDict *response, *minfo;
     QList *list;
     const QListEntry *p;
@@ -1639,9 +1640,10 @@ static struct MachInfo *qtest_get_machines(const char *var)
     QTestState *qts;
     int idx;
 
-    if (g_strcmp0(qemu_var, var)) {
-        g_free(qemu_var);
-        qemu_var = g_strdup(var);
+    new_qemu_bin = qtest_qemu_binary(var);
+    if (g_strcmp0(qemu_bin, new_qemu_bin)) {
+        g_free(qemu_bin);
+        qemu_bin = g_strdup(new_qemu_bin);
 
         /* new qemu, clear the cache */
         qtest_free_machine_list(machines);
@@ -1654,7 +1656,7 @@ static struct MachInfo *qtest_get_machines(const char *var)
 
     silence_spawn_log = !g_test_verbose();
 
-    qts = qtest_init_ext(qemu_var, "-machine none", NULL, true);
+    qts = qtest_init_ext(var, "-machine none", NULL, true);
     response = qtest_qmp(qts, "{ 'execute': 'query-machines' }");
     g_assert(response);
     list = qdict_get_qlist(response, "return");
-- 
1.8.3.1
Re: [PATCH] tests/qtest: optimize qtest_get_machines
Posted by Markus Armbruster 1 week, 1 day ago 
Steve Sistare <steven.sistare@oracle.com> writes:

> qtest_get_machines returns the machines supported by the QEMU binary
> described by an environment variable and caches the result.  If the
> next call to qtest_get_machines passes the same variable name, the cached
> result is returned, but if the name changes, the caching is defeated.
> To make caching more effective, remember the path of the QEMU binary
> instead.  Different env vars, eg QTEST_QEMU_BINARY_SRC and
> QTEST_QEMU_BINARY_DST, usually resolve to the same path.
>
> Before the optimization, the test /x86_64/migration/precopy/unix/plain
> exec's QEMU and calls query-machines 3 times.  After optimization, that
> only happens once.  This does not significantly speed up the tests, but
> it reduces QTEST_LOG output, and launches fewer QEMU instances, making
> it easier to debug problems.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

I guess this is a followup to an observation I made during review of my
[PATCH 1/5] qtest/qom-test: Plug memory leak with -p:

    Message-ID: <87h5ymdzrf.fsf@pond.sub.org>
    https://lore.kernel.org/qemu-devel/87h5ymdzrf.fsf@pond.sub.org/

Appreciated!
Re: [PATCH] tests/qtest: optimize qtest_get_machines
Posted by Steven Sistare 5 days, 20 hours ago 
On 9/20/2025 3:12 AM, Markus Armbruster wrote:
> Steve Sistare <steven.sistare@oracle.com> writes:
> 
>> qtest_get_machines returns the machines supported by the QEMU binary
>> described by an environment variable and caches the result.  If the
>> next call to qtest_get_machines passes the same variable name, the cached
>> result is returned, but if the name changes, the caching is defeated.
>> To make caching more effective, remember the path of the QEMU binary
>> instead.  Different env vars, eg QTEST_QEMU_BINARY_SRC and
>> QTEST_QEMU_BINARY_DST, usually resolve to the same path.
>>
>> Before the optimization, the test /x86_64/migration/precopy/unix/plain
>> exec's QEMU and calls query-machines 3 times.  After optimization, that
>> only happens once.  This does not significantly speed up the tests, but
>> it reduces QTEST_LOG output, and launches fewer QEMU instances, making
>> it easier to debug problems.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> 
> I guess this is a followup to an observation I made during review of my
> [PATCH 1/5] qtest/qom-test: Plug memory leak with -p:
> 
>      Message-ID: <87h5ymdzrf.fsf@pond.sub.org>
>      https://lore.kernel.org/qemu-devel/87h5ymdzrf.fsf@pond.sub.org/
> 
> Appreciated!

In truth, this new patch is not intended to reduce leaks.  If it does,
that is a bonus :)

- Steve
Re: [PATCH] tests/qtest: optimize qtest_get_machines
Posted by Markus Armbruster 5 days, 9 hours ago 
Steven Sistare <steven.sistare@oracle.com> writes:

> On 9/20/2025 3:12 AM, Markus Armbruster wrote:
>> Steve Sistare <steven.sistare@oracle.com> writes:
>> 
>>> qtest_get_machines returns the machines supported by the QEMU binary
>>> described by an environment variable and caches the result.  If the
>>> next call to qtest_get_machines passes the same variable name, the cached
>>> result is returned, but if the name changes, the caching is defeated.
>>> To make caching more effective, remember the path of the QEMU binary
>>> instead.  Different env vars, eg QTEST_QEMU_BINARY_SRC and
>>> QTEST_QEMU_BINARY_DST, usually resolve to the same path.
>>>
>>> Before the optimization, the test /x86_64/migration/precopy/unix/plain
>>> exec's QEMU and calls query-machines 3 times.  After optimization, that
>>> only happens once.  This does not significantly speed up the tests, but
>>> it reduces QTEST_LOG output, and launches fewer QEMU instances, making
>>> it easier to debug problems.
>>>
>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> 
>> I guess this is a followup to an observation I made during review of my
>> [PATCH 1/5] qtest/qom-test: Plug memory leak with -p:
>> 
>>      Message-ID: <87h5ymdzrf.fsf@pond.sub.org>
>>      https://lore.kernel.org/qemu-devel/87h5ymdzrf.fsf@pond.sub.org/
>> 
>> Appreciated!
>
> In truth, this new patch is not intended to reduce leaks.  If it does,
> that is a bonus :)

Harmless misunderstanding!  During review of my patch (which we didn't
take, because it was incomplete and flawed), I pointed out the cache
thrashing your patch fixes.  That's all.
Re: [PATCH] tests/qtest: optimize qtest_get_machines
Posted by Fabiano Rosas 1 week, 1 day ago 
Steve Sistare <steven.sistare@oracle.com> writes:

> qtest_get_machines returns the machines supported by the QEMU binary
> described by an environment variable and caches the result.  If the
> next call to qtest_get_machines passes the same variable name, the cached
> result is returned, but if the name changes, the caching is defeated.
> To make caching more effective, remember the path of the QEMU binary
> instead.  Different env vars, eg QTEST_QEMU_BINARY_SRC and
> QTEST_QEMU_BINARY_DST, usually resolve to the same path.
>
> Before the optimization, the test /x86_64/migration/precopy/unix/plain
> exec's QEMU and calls query-machines 3 times.  After optimization, that
> only happens once.  This does not significantly speed up the tests, but
> it reduces QTEST_LOG output, and launches fewer QEMU instances, making
> it easier to debug problems.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  tests/qtest/libqtest.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 94526b7..f3d4e08 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -1630,7 +1630,8 @@ static void qtest_free_machine_list(struct MachInfo *machines)
>  static struct MachInfo *qtest_get_machines(const char *var)
>  {
>      static struct MachInfo *machines;
> -    static char *qemu_var;
> +    static char *qemu_bin;
> +    const char *new_qemu_bin;
>      QDict *response, *minfo;
>      QList *list;
>      const QListEntry *p;
> @@ -1639,9 +1640,10 @@ static struct MachInfo *qtest_get_machines(const char *var)
>      QTestState *qts;
>      int idx;
>  
> -    if (g_strcmp0(qemu_var, var)) {
> -        g_free(qemu_var);
> -        qemu_var = g_strdup(var);
> +    new_qemu_bin = qtest_qemu_binary(var);
> +    if (g_strcmp0(qemu_bin, new_qemu_bin)) {
> +        g_free(qemu_bin);
> +        qemu_bin = g_strdup(new_qemu_bin);
>  
>          /* new qemu, clear the cache */
>          qtest_free_machine_list(machines);
> @@ -1654,7 +1656,7 @@ static struct MachInfo *qtest_get_machines(const char *var)
>  
>      silence_spawn_log = !g_test_verbose();
>  
> -    qts = qtest_init_ext(qemu_var, "-machine none", NULL, true);
> +    qts = qtest_init_ext(var, "-machine none", NULL, true);
>      response = qtest_qmp(qts, "{ 'execute': 'query-machines' }");
>      g_assert(response);
>      list = qdict_get_qlist(response, "return");

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Thanks! Queued.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.