Re: [PATCH] [for-10.1] qtest: introduce qtest_init_ext

[Fabiano Rosas <farosas@suse.de>]

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> Merge qtest_init_with_env_and_capabilities() and qtest_init_with_env()
> into one qtest_init_ext().
>
> Reasons:
>
> 1. qtest_init_with_env() is just wrong: it gets do_connect parameter
>    but always pass true to qtest_init_with_env_and_capabilities().
>    Happily, all qtest_init_with_env() callers pass true as well.
>
> 2. qtest_init_with_env() is not used outside of libqtest.c, so no
>    reason to keep it as public function
>
> 3. and in libqtest.c it's used not often, so no problem to use
>    more generic function instead.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  tests/qtest/libqtest.c            | 18 +++++-------------
>  tests/qtest/libqtest.h            | 30 +++++++-----------------------
>  tests/qtest/migration/framework.c |  7 +++----
>  3 files changed, 15 insertions(+), 40 deletions(-)
>
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index fad307d125..66ff318201 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -574,10 +574,8 @@ void qtest_qmp_handshake(QTestState *s, QList *capabilities)
>      }
>  }
>  
> -QTestState *qtest_init_with_env_and_capabilities(const char *var,
> -                                                 const char *extra_args,
> -                                                 QList *capabilities,
> -                                                 bool do_connect)
> +QTestState *qtest_init_ext(const char *var, const char *extra_args,
> +                           QList *capabilities, bool do_connect)
>  {
>      QTestState *s = qtest_init_internal(qtest_qemu_binary(var), extra_args,
>                                          do_connect);
> @@ -594,15 +592,9 @@ QTestState *qtest_init_with_env_and_capabilities(const char *var,
>      return s;
>  }
>  
> -QTestState *qtest_init_with_env(const char *var, const char *extra_args,
> -                                bool do_connect)
> -{
> -    return qtest_init_with_env_and_capabilities(var, extra_args, NULL, true);
> -}
> -
>  QTestState *qtest_init(const char *extra_args)
>  {
> -    return qtest_init_with_env(NULL, extra_args, true);
> +    return qtest_init_ext(NULL, extra_args, NULL, true);
>  }
>  
>  QTestState *qtest_vinitf(const char *fmt, va_list ap)
> @@ -1662,7 +1654,7 @@ static struct MachInfo *qtest_get_machines(const char *var)
>  
>      silence_spawn_log = !g_test_verbose();
>  
> -    qts = qtest_init_with_env(qemu_var, "-machine none", true);
> +    qts = qtest_init_ext(qemu_var, "-machine none", NULL, true);
>      response = qtest_qmp(qts, "{ 'execute': 'query-machines' }");
>      g_assert(response);
>      list = qdict_get_qlist(response, "return");
> @@ -1717,7 +1709,7 @@ static struct CpuModel *qtest_get_cpu_models(void)
>  
>      silence_spawn_log = !g_test_verbose();
>  
> -    qts = qtest_init_with_env(NULL, "-machine none", true);
> +    qts = qtest_init_ext(NULL, "-machine none", NULL, true);
>      response = qtest_qmp(qts, "{ 'execute': 'query-cpu-definitions' }");
>      g_assert(response);
>      list = qdict_get_qlist(response, "return");
> diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
> index 930a91dcb7..b3f2e7fbef 100644
> --- a/tests/qtest/libqtest.h
> +++ b/tests/qtest/libqtest.h
> @@ -57,37 +57,21 @@ QTestState *qtest_vinitf(const char *fmt, va_list ap) G_GNUC_PRINTF(1, 0);
>  QTestState *qtest_init(const char *extra_args);
>  
>  /**
> - * qtest_init_with_env:
> - * @var: Environment variable from where to take the QEMU binary
> - * @extra_args: Other arguments to pass to QEMU.  CAUTION: these
> - * arguments are subject to word splitting and shell evaluation.
> - * @do_connect: connect to qemu monitor and qtest socket.
> - *
> - * Like qtest_init(), but use a different environment variable for the
> - * QEMU binary.
> - *
> - * Returns: #QTestState instance.
> - */
> -QTestState *qtest_init_with_env(const char *var, const char *extra_args,
> -                                bool do_connect);
> -
> -/**
> - * qtest_init_with_env_and_capabilities:
> + * qtest_init_ext:
>   * @var: Environment variable from where to take the QEMU binary
>   * @extra_args: Other arguments to pass to QEMU.  CAUTION: these
>   * arguments are subject to word splitting and shell evaluation.
>   * @capabilities: list of QMP capabilities (strings) to enable
>   * @do_connect: connect to qemu monitor and qtest socket.
>   *
> - * Like qtest_init_with_env(), but enable specified capabilities during
> - * hadshake.
> + * Like qtest_init(), but use a different environment variable for the
> + * QEMU binary, allow specify capabilities and skip connecting
> + * to QEMU monitor.
>   *
>   * Returns: #QTestState instance.
>   */
> -QTestState *qtest_init_with_env_and_capabilities(const char *var,
> -                                                 const char *extra_args,
> -                                                 QList *capabilities,
> -                                                 bool do_connect);
> +QTestState *qtest_init_ext(const char *var, const char *extra_args,
> +                           QList *capabilities, bool do_connect);
>  
>  /**
>   * qtest_init_without_qmp_handshake:
> @@ -102,7 +86,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
>   * qtest_connect
>   * @s: #QTestState instance to connect
>   * Connect to qemu monitor and qtest socket, after skipping them in
> - * qtest_init_with_env.  Does not handshake with the monitor.
> + * qtest_init_ext.  Does not handshake with the monitor.
>   */
>  void qtest_connect(QTestState *s);
>  
> diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
> index 10e1d04b58..1802304e1d 100644
> --- a/tests/qtest/migration/framework.c
> +++ b/tests/qtest/migration/framework.c
> @@ -336,8 +336,7 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
>                                   args->opts_source ? args->opts_source : "",
>                                   ignore_stderr);
>      if (!args->only_target) {
> -        *from = qtest_init_with_env_and_capabilities(QEMU_ENV_SRC, cmd_source,
> -                                                     capabilities, true);
> +        *from = qtest_init_ext(QEMU_ENV_SRC, cmd_source, capabilities, true);
>          qtest_qmp_set_event_callback(*from,
>                                       migrate_watch_for_events,
>                                       &src_state);
> @@ -365,8 +364,8 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
>                                   shmem_opts ? shmem_opts : "",
>                                   args->opts_target ? args->opts_target : "",
>                                   ignore_stderr);
> -    *to = qtest_init_with_env_and_capabilities(QEMU_ENV_DST, cmd_target,
> -                                               capabilities, !args->defer_target_connect);
> +    *to = qtest_init_ext(QEMU_ENV_DST, cmd_target, capabilities,
> +                         !args->defer_target_connect);
>      qtest_qmp_set_event_callback(*to,
>                                   migrate_watch_for_events,
>                                   &dst_state);

Queued, thanks.