Re: [PATCH] tests/qtest: optimize qtest_get_machines

[Fabiano Rosas <farosas@suse.de>]

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.