diff mbox series

[V4,16/19] tests/qtest: defer connection

Message ID 1733145611-62315-17-git-send-email-steven.sistare@oracle.com (mailing list archive)
State New, archived
Headers show
Series Live update: cpr-transfer | expand

Commit Message

Steve Sistare Dec. 2, 2024, 1:20 p.m. UTC
Add an option to defer making the connecting to the monitor and qtest
sockets when calling qtest_init_with_env.  The client makes the connection
later by calling qtest_connect_deferred and qtest_qmp_handshake.

A test cannot specify port=0 for a deferred connection, because qmp_migrate
cannot query for the assigned port, because the monitor is not connected
yet.  However, even if the test does not specify port=0, qmp_migrate ->
migrate_set_ports unconditionally queries connection parameters.
Modify migrate_set_ports to only query when port=0.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 tests/qtest/libqtest.c          | 80 +++++++++++++++++++++++++----------------
 tests/qtest/libqtest.h          | 19 +++++++++-
 tests/qtest/migration-helpers.c | 19 +++++-----
 tests/qtest/migration-test.c    |  4 +--
 4 files changed, 80 insertions(+), 42 deletions(-)

Comments

Steve Sistare Dec. 18, 2024, 9:02 p.m. UTC | #1
If someone is bored, this needs review, else let it ride until V5.

- Steve

On 12/2/2024 8:20 AM, Steve Sistare wrote:
> Add an option to defer making the connecting to the monitor and qtest
> sockets when calling qtest_init_with_env.  The client makes the connection
> later by calling qtest_connect_deferred and qtest_qmp_handshake.
> 
> A test cannot specify port=0 for a deferred connection, because qmp_migrate
> cannot query for the assigned port, because the monitor is not connected
> yet.  However, even if the test does not specify port=0, qmp_migrate ->
> migrate_set_ports unconditionally queries connection parameters.
> Modify migrate_set_ports to only query when port=0.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>   tests/qtest/libqtest.c          | 80 +++++++++++++++++++++++++----------------
>   tests/qtest/libqtest.h          | 19 +++++++++-
>   tests/qtest/migration-helpers.c | 19 +++++-----
>   tests/qtest/migration-test.c    |  4 +--
>   4 files changed, 80 insertions(+), 42 deletions(-)
> 
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 817fd7a..31c4032 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -75,6 +75,8 @@ struct QTestState
>   {
>       int fd;
>       int qmp_fd;
> +    int sock;
> +    int qmpsock;
>       pid_t qemu_pid;  /* our child QEMU process */
>       int wstatus;
>   #ifdef _WIN32
> @@ -442,18 +444,19 @@ static QTestState *G_GNUC_PRINTF(2, 3) qtest_spawn_qemu(const char *qemu_bin,
>       return s;
>   }
>   
> +static char *qtest_socket_path(const char *suffix)
> +{
> +    return g_strdup_printf("%s/qtest-%d.%s", g_get_tmp_dir(), getpid(), suffix);
> +}
> +
>   static QTestState *qtest_init_internal(const char *qemu_bin,
> -                                       const char *extra_args)
> +                                       const char *extra_args,
> +                                       bool defer_connect)
>   {
>       QTestState *s;
>       int sock, qmpsock, i;
> -    gchar *socket_path;
> -    gchar *qmp_socket_path;
> -
> -    socket_path = g_strdup_printf("%s/qtest-%d.sock",
> -                                  g_get_tmp_dir(), getpid());
> -    qmp_socket_path = g_strdup_printf("%s/qtest-%d.qmp",
> -                                      g_get_tmp_dir(), getpid());
> +    g_autofree gchar *socket_path = qtest_socket_path("sock");
> +    g_autofree gchar *qmp_socket_path = qtest_socket_path("qmp");
>   
>       /*
>        * It's possible that if an earlier test run crashed it might
> @@ -485,22 +488,17 @@ static QTestState *qtest_init_internal(const char *qemu_bin,
>       qtest_client_set_rx_handler(s, qtest_client_socket_recv_line);
>       qtest_client_set_tx_handler(s, qtest_client_socket_send);
>   
> -    s->fd = socket_accept(sock);
> -    if (s->fd >= 0) {
> -        s->qmp_fd = socket_accept(qmpsock);
> -    }
> -    unlink(socket_path);
> -    unlink(qmp_socket_path);
> -    g_free(socket_path);
> -    g_free(qmp_socket_path);
> -
> -    g_assert(s->fd >= 0 && s->qmp_fd >= 0);
> -
>       s->rx = g_string_new("");
>       for (i = 0; i < MAX_IRQ; i++) {
>           s->irq_level[i] = false;
>       }
>   
> +    s->sock = sock;
> +    s->qmpsock = qmpsock;
> +    if (!defer_connect) {
> +        qtest_connect_deferred(s);
> +    }
> +
>       /*
>        * Stopping QEMU for debugging is not supported on Windows.
>        *
> @@ -515,34 +513,54 @@ static QTestState *qtest_init_internal(const char *qemu_bin,
>       }
>   #endif
>   
> +   return s;
> +}
> +
> +void qtest_connect_deferred(QTestState *s)
> +{
> +    g_autofree gchar *socket_path = qtest_socket_path("sock");
> +    g_autofree gchar *qmp_socket_path = qtest_socket_path("qmp");
> +
> +    g_assert(s->sock >= 0 && s->qmpsock >= 0);
> +    s->fd = socket_accept(s->sock);
> +    if (s->fd >= 0) {
> +        s->qmp_fd = socket_accept(s->qmpsock);
> +    }
> +    unlink(socket_path);
> +    unlink(qmp_socket_path);
> +    g_assert(s->fd >= 0 && s->qmp_fd >= 0);
> +    s->sock = s->qmpsock = -1;
>       /* ask endianness of the target */
> -
>       s->big_endian = qtest_query_target_endianness(s);
> -
> -   return s;
>   }
>   
>   QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
>   {
> -    return qtest_init_internal(qtest_qemu_binary(NULL), extra_args);
> +    return qtest_init_internal(qtest_qemu_binary(NULL), extra_args, false);
>   }
>   
> -QTestState *qtest_init_with_env(const char *var, const char *extra_args)
> +void qtest_qmp_handshake(QTestState *s)
>   {
> -    QTestState *s = qtest_init_internal(qtest_qemu_binary(var), extra_args);
> -    QDict *greeting;
> -
>       /* Read the QMP greeting and then do the handshake */
> -    greeting = qtest_qmp_receive(s);
> +    QDict *greeting = qtest_qmp_receive(s);
>       qobject_unref(greeting);
>       qobject_unref(qtest_qmp(s, "{ 'execute': 'qmp_capabilities' }"));
> +}
>   
> +QTestState *qtest_init_with_env(const char *var, const char *extra_args,
> +                                bool defer_connect)
> +{
> +    QTestState *s = qtest_init_internal(qtest_qemu_binary(var), extra_args,
> +                                        defer_connect);
> +    if (!defer_connect) {
> +        qtest_qmp_handshake(s);
> +    }
>       return s;
>   }
>   
>   QTestState *qtest_init(const char *extra_args)
>   {
> -    return qtest_init_with_env(NULL, extra_args);
> +    return qtest_init_with_env(NULL, extra_args, false);
>   }
>   
>   QTestState *qtest_vinitf(const char *fmt, va_list ap)
> @@ -1523,7 +1541,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");
> +    qts = qtest_init_with_env(qemu_var, "-machine none", false);
>       response = qtest_qmp(qts, "{ 'execute': 'query-machines' }");
>       g_assert(response);
>       list = qdict_get_qlist(response, "return");
> @@ -1578,7 +1596,7 @@ static struct CpuModel *qtest_get_cpu_models(void)
>   
>       silence_spawn_log = !g_test_verbose();
>   
> -    qts = qtest_init_with_env(NULL, "-machine none");
> +    qts = qtest_init_with_env(NULL, "-machine none", false);
>       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 beb96b1..db76f2c 100644
> --- a/tests/qtest/libqtest.h
> +++ b/tests/qtest/libqtest.h
> @@ -60,13 +60,15 @@ QTestState *qtest_init(const char *extra_args);
>    * @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.
> + * @defer_connect: do not 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);
> +QTestState *qtest_init_with_env(const char *var, const char *extra_args,
> +                                bool defer_connect);
>   
>   /**
>    * qtest_init_without_qmp_handshake:
> @@ -78,6 +80,21 @@ QTestState *qtest_init_with_env(const char *var, const char *extra_args);
>   QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
>   
>   /**
> + * qtest_connect_deferred:
> + * @s: #QTestState instance to connect
> + * Connect to qemu monitor and qtest socket, after deferring them in
> + * qtest_init_with_env.  Does not handshake with the monitor.
> + */
> +void qtest_connect_deferred(QTestState *s);
> +
> +/**
> + * qtest_qmp_handshake:
> + * @s: #QTestState instance to operate on.
> + * Perform handshake after connecting to qemu monitor.
> + */
> +void qtest_qmp_handshake(QTestState *s);
> +
> +/**
>    * qtest_init_with_serial:
>    * @extra_args: other arguments to pass to QEMU.  CAUTION: these
>    * arguments are subject to word splitting and shell evaluation.
> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
> index 3f8ba7f..9f39401 100644
> --- a/tests/qtest/migration-helpers.c
> +++ b/tests/qtest/migration-helpers.c
> @@ -127,25 +127,28 @@ migrate_get_connect_qdict(QTestState *who)
>   
>   static void migrate_set_ports(QTestState *to, QList *channel_list)
>   {
> -    QDict *addr;
> +    g_autoptr(QDict) addr = NULL;
>       QListEntry *entry;
>       const char *addr_port = NULL;
>   
> -    addr = migrate_get_connect_qdict(to);
> -
>       QLIST_FOREACH_ENTRY(channel_list, entry) {
>           QDict *channel = qobject_to(QDict, qlist_entry_obj(entry));
>           QDict *addrdict = qdict_get_qdict(channel, "addr");
>   
> -        if (qdict_haskey(addrdict, "port") &&
> -            qdict_haskey(addr, "port") &&
> -            (strcmp(qdict_get_str(addrdict, "port"), "0") == 0)) {
> +        if (!qdict_haskey(addrdict, "port") ||
> +            strcmp(qdict_get_str(addrdict, "port"), "0")) {
> +            continue;
> +        }
> +
> +        if (!addr) {
> +            addr = migrate_get_connect_qdict(to);
> +        }
> +
> +        if (qdict_haskey(addr, "port")) {
>               addr_port = qdict_get_str(addr, "port");
>               qdict_put_str(addrdict, "port", addr_port);
>           }
>       }
> -
> -    qobject_unref(addr);
>   }
>   
>   bool migrate_watch_for_events(QTestState *who, const char *name,
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 64e1c50..b7001b0 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -844,7 +844,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>                                    args->opts_source ? args->opts_source : "",
>                                    ignore_stderr);
>       if (!args->only_target) {
> -        *from = qtest_init_with_env(QEMU_ENV_SRC, cmd_source);
> +        *from = qtest_init_with_env(QEMU_ENV_SRC, cmd_source, false);
>           qtest_qmp_set_event_callback(*from,
>                                        migrate_watch_for_events,
>                                        &src_state);
> @@ -865,7 +865,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>                                    shmem_opts ? shmem_opts : "",
>                                    args->opts_target ? args->opts_target : "",
>                                    ignore_stderr);
> -    *to = qtest_init_with_env(QEMU_ENV_DST, cmd_target);
> +    *to = qtest_init_with_env(QEMU_ENV_DST, cmd_target, false);
>       qtest_qmp_set_event_callback(*to,
>                                    migrate_watch_for_events,
>                                    &dst_state);
Peter Xu Dec. 19, 2024, 3:46 p.m. UTC | #2
On Mon, Dec 02, 2024 at 05:20:08AM -0800, Steve Sistare wrote:
> Add an option to defer making the connecting to the monitor and qtest
> sockets when calling qtest_init_with_env.  The client makes the connection
> later by calling qtest_connect_deferred and qtest_qmp_handshake.
> 
> A test cannot specify port=0 for a deferred connection, because qmp_migrate
> cannot query for the assigned port, because the monitor is not connected
> yet.  However, even if the test does not specify port=0, qmp_migrate ->
> migrate_set_ports unconditionally queries connection parameters.
> Modify migrate_set_ports to only query when port=0.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

Looks mostly good, nitpicks inline..

> ---
>  tests/qtest/libqtest.c          | 80 +++++++++++++++++++++++++----------------
>  tests/qtest/libqtest.h          | 19 +++++++++-
>  tests/qtest/migration-helpers.c | 19 +++++-----
>  tests/qtest/migration-test.c    |  4 +--
>  4 files changed, 80 insertions(+), 42 deletions(-)
> 
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 817fd7a..31c4032 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -75,6 +75,8 @@ struct QTestState
>  {
>      int fd;
>      int qmp_fd;
> +    int sock;
> +    int qmpsock;
>      pid_t qemu_pid;  /* our child QEMU process */
>      int wstatus;
>  #ifdef _WIN32
> @@ -442,18 +444,19 @@ static QTestState *G_GNUC_PRINTF(2, 3) qtest_spawn_qemu(const char *qemu_bin,
>      return s;
>  }
>  
> +static char *qtest_socket_path(const char *suffix)
> +{
> +    return g_strdup_printf("%s/qtest-%d.%s", g_get_tmp_dir(), getpid(), suffix);
> +}
> +
>  static QTestState *qtest_init_internal(const char *qemu_bin,
> -                                       const char *extra_args)
> +                                       const char *extra_args,
> +                                       bool defer_connect)

Suggest to stick with positive logic naming.

That is, s/defer_connect/do_connect/ or similar, then invert the values in
callers.

>  {
>      QTestState *s;
>      int sock, qmpsock, i;
> -    gchar *socket_path;
> -    gchar *qmp_socket_path;
> -
> -    socket_path = g_strdup_printf("%s/qtest-%d.sock",
> -                                  g_get_tmp_dir(), getpid());
> -    qmp_socket_path = g_strdup_printf("%s/qtest-%d.qmp",
> -                                      g_get_tmp_dir(), getpid());
> +    g_autofree gchar *socket_path = qtest_socket_path("sock");
> +    g_autofree gchar *qmp_socket_path = qtest_socket_path("qmp");
>  
>      /*
>       * It's possible that if an earlier test run crashed it might
> @@ -485,22 +488,17 @@ static QTestState *qtest_init_internal(const char *qemu_bin,
>      qtest_client_set_rx_handler(s, qtest_client_socket_recv_line);
>      qtest_client_set_tx_handler(s, qtest_client_socket_send);
>  
> -    s->fd = socket_accept(sock);
> -    if (s->fd >= 0) {
> -        s->qmp_fd = socket_accept(qmpsock);
> -    }
> -    unlink(socket_path);
> -    unlink(qmp_socket_path);
> -    g_free(socket_path);
> -    g_free(qmp_socket_path);
> -
> -    g_assert(s->fd >= 0 && s->qmp_fd >= 0);
> -
>      s->rx = g_string_new("");
>      for (i = 0; i < MAX_IRQ; i++) {
>          s->irq_level[i] = false;
>      }
>  
> +    s->sock = sock;
> +    s->qmpsock = qmpsock;
> +    if (!defer_connect) {
> +        qtest_connect_deferred(s);

Now qtest_connect_deferred() itself has nothing to do with the "defer"
concept.. it is the helper to connect the sockets, so maybe better call it
qtest_connect_socks(), or similar.

> +    }
> +
>      /*
>       * Stopping QEMU for debugging is not supported on Windows.
>       *
> @@ -515,34 +513,54 @@ static QTestState *qtest_init_internal(const char *qemu_bin,
>      }
>  #endif
>  
> +   return s;
> +}
> +
> +void qtest_connect_deferred(QTestState *s)
> +{
> +    g_autofree gchar *socket_path = qtest_socket_path("sock");
> +    g_autofree gchar *qmp_socket_path = qtest_socket_path("qmp");
> +
> +    g_assert(s->sock >= 0 && s->qmpsock >= 0);
> +    s->fd = socket_accept(s->sock);
> +    if (s->fd >= 0) {
> +        s->qmp_fd = socket_accept(s->qmpsock);
> +    }
> +    unlink(socket_path);
> +    unlink(qmp_socket_path);
> +    g_assert(s->fd >= 0 && s->qmp_fd >= 0);
> +    s->sock = s->qmpsock = -1;
>      /* ask endianness of the target */
> -
>      s->big_endian = qtest_query_target_endianness(s);
> -
> -   return s;
>  }
>  
>  QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
>  {
> -    return qtest_init_internal(qtest_qemu_binary(NULL), extra_args);
> +    return qtest_init_internal(qtest_qemu_binary(NULL), extra_args, false);
>  }
>  
> -QTestState *qtest_init_with_env(const char *var, const char *extra_args)
> +void qtest_qmp_handshake(QTestState *s)
>  {
> -    QTestState *s = qtest_init_internal(qtest_qemu_binary(var), extra_args);
> -    QDict *greeting;
> -
>      /* Read the QMP greeting and then do the handshake */
> -    greeting = qtest_qmp_receive(s);
> +    QDict *greeting = qtest_qmp_receive(s);
>      qobject_unref(greeting);
>      qobject_unref(qtest_qmp(s, "{ 'execute': 'qmp_capabilities' }"));
> +}
>  
> +QTestState *qtest_init_with_env(const char *var, const char *extra_args,
> +                                bool defer_connect)
> +{
> +    QTestState *s = qtest_init_internal(qtest_qemu_binary(var), extra_args,
> +                                        defer_connect);
> +    if (!defer_connect) {
> +        qtest_qmp_handshake(s);
> +    }
>      return s;
>  }
>  
>  QTestState *qtest_init(const char *extra_args)
>  {
> -    return qtest_init_with_env(NULL, extra_args);
> +    return qtest_init_with_env(NULL, extra_args, false);
>  }
>  
>  QTestState *qtest_vinitf(const char *fmt, va_list ap)
> @@ -1523,7 +1541,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");
> +    qts = qtest_init_with_env(qemu_var, "-machine none", false);
>      response = qtest_qmp(qts, "{ 'execute': 'query-machines' }");
>      g_assert(response);
>      list = qdict_get_qlist(response, "return");
> @@ -1578,7 +1596,7 @@ static struct CpuModel *qtest_get_cpu_models(void)
>  
>      silence_spawn_log = !g_test_verbose();
>  
> -    qts = qtest_init_with_env(NULL, "-machine none");
> +    qts = qtest_init_with_env(NULL, "-machine none", false);
>      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 beb96b1..db76f2c 100644
> --- a/tests/qtest/libqtest.h
> +++ b/tests/qtest/libqtest.h
> @@ -60,13 +60,15 @@ QTestState *qtest_init(const char *extra_args);
>   * @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.
> + * @defer_connect: do not 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);
> +QTestState *qtest_init_with_env(const char *var, const char *extra_args,
> +                                bool defer_connect);
>  
>  /**
>   * qtest_init_without_qmp_handshake:
> @@ -78,6 +80,21 @@ QTestState *qtest_init_with_env(const char *var, const char *extra_args);
>  QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
>  
>  /**
> + * qtest_connect_deferred:
> + * @s: #QTestState instance to connect
> + * Connect to qemu monitor and qtest socket, after deferring them in
> + * qtest_init_with_env.  Does not handshake with the monitor.
> + */
> +void qtest_connect_deferred(QTestState *s);
> +
> +/**
> + * qtest_qmp_handshake:
> + * @s: #QTestState instance to operate on.
> + * Perform handshake after connecting to qemu monitor.
> + */
> +void qtest_qmp_handshake(QTestState *s);
> +
> +/**
>   * qtest_init_with_serial:
>   * @extra_args: other arguments to pass to QEMU.  CAUTION: these
>   * arguments are subject to word splitting and shell evaluation.
> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
> index 3f8ba7f..9f39401 100644
> --- a/tests/qtest/migration-helpers.c
> +++ b/tests/qtest/migration-helpers.c
> @@ -127,25 +127,28 @@ migrate_get_connect_qdict(QTestState *who)
>  
>  static void migrate_set_ports(QTestState *to, QList *channel_list)
>  {
> -    QDict *addr;
> +    g_autoptr(QDict) addr = NULL;
>      QListEntry *entry;
>      const char *addr_port = NULL;
>  
> -    addr = migrate_get_connect_qdict(to);
> -
>      QLIST_FOREACH_ENTRY(channel_list, entry) {
>          QDict *channel = qobject_to(QDict, qlist_entry_obj(entry));
>          QDict *addrdict = qdict_get_qdict(channel, "addr");
>  
> -        if (qdict_haskey(addrdict, "port") &&
> -            qdict_haskey(addr, "port") &&
> -            (strcmp(qdict_get_str(addrdict, "port"), "0") == 0)) {
> +        if (!qdict_haskey(addrdict, "port") ||
> +            strcmp(qdict_get_str(addrdict, "port"), "0")) {
> +            continue;
> +        }
> +
> +        if (!addr) {
> +            addr = migrate_get_connect_qdict(to);

May be good to add a comment above on why the query was done only lazily.

Meanwhile this chunk of change can be separate; it's relevant to the defer
idea but still pretty standalone change. Can be one small patch prior to
this one, IMHO.

Optional idea, can be for later: if QTestState can have the state showing
whether the QMP is ready, we could already assert making sure the query
happens only if the QMP is available.

> +        }
> +
> +        if (qdict_haskey(addr, "port")) {
>              addr_port = qdict_get_str(addr, "port");
>              qdict_put_str(addrdict, "port", addr_port);
>          }
>      }
> -
> -    qobject_unref(addr);
>  }
>  
>  bool migrate_watch_for_events(QTestState *who, const char *name,
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 64e1c50..b7001b0 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -844,7 +844,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>                                   args->opts_source ? args->opts_source : "",
>                                   ignore_stderr);
>      if (!args->only_target) {
> -        *from = qtest_init_with_env(QEMU_ENV_SRC, cmd_source);
> +        *from = qtest_init_with_env(QEMU_ENV_SRC, cmd_source, false);
>          qtest_qmp_set_event_callback(*from,
>                                       migrate_watch_for_events,
>                                       &src_state);
> @@ -865,7 +865,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>                                   shmem_opts ? shmem_opts : "",
>                                   args->opts_target ? args->opts_target : "",
>                                   ignore_stderr);
> -    *to = qtest_init_with_env(QEMU_ENV_DST, cmd_target);
> +    *to = qtest_init_with_env(QEMU_ENV_DST, cmd_target, false);
>      qtest_qmp_set_event_callback(*to,
>                                   migrate_watch_for_events,
>                                   &dst_state);
> -- 
> 1.8.3.1
>
Steve Sistare Dec. 19, 2024, 10:33 p.m. UTC | #3
On 12/19/2024 10:46 AM, Peter Xu wrote:
> On Mon, Dec 02, 2024 at 05:20:08AM -0800, Steve Sistare wrote:
>> Add an option to defer making the connecting to the monitor and qtest
>> sockets when calling qtest_init_with_env.  The client makes the connection
>> later by calling qtest_connect_deferred and qtest_qmp_handshake.
>>
>> A test cannot specify port=0 for a deferred connection, because qmp_migrate
>> cannot query for the assigned port, because the monitor is not connected
>> yet.  However, even if the test does not specify port=0, qmp_migrate ->
>> migrate_set_ports unconditionally queries connection parameters.
>> Modify migrate_set_ports to only query when port=0.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> 
> Looks mostly good, nitpicks inline..
> 
>> ---
>>   tests/qtest/libqtest.c          | 80 +++++++++++++++++++++++++----------------
>>   tests/qtest/libqtest.h          | 19 +++++++++-
>>   tests/qtest/migration-helpers.c | 19 +++++-----
>>   tests/qtest/migration-test.c    |  4 +--
>>   4 files changed, 80 insertions(+), 42 deletions(-)
>>
>> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
>> index 817fd7a..31c4032 100644
>> --- a/tests/qtest/libqtest.c
>> +++ b/tests/qtest/libqtest.c
>> @@ -75,6 +75,8 @@ struct QTestState
>>   {
>>       int fd;
>>       int qmp_fd;
>> +    int sock;
>> +    int qmpsock;
>>       pid_t qemu_pid;  /* our child QEMU process */
>>       int wstatus;
>>   #ifdef _WIN32
>> @@ -442,18 +444,19 @@ static QTestState *G_GNUC_PRINTF(2, 3) qtest_spawn_qemu(const char *qemu_bin,
>>       return s;
>>   }
>>   
>> +static char *qtest_socket_path(const char *suffix)
>> +{
>> +    return g_strdup_printf("%s/qtest-%d.%s", g_get_tmp_dir(), getpid(), suffix);
>> +}
>> +
>>   static QTestState *qtest_init_internal(const char *qemu_bin,
>> -                                       const char *extra_args)
>> +                                       const char *extra_args,
>> +                                       bool defer_connect)
> 
> Suggest to stick with positive logic naming.
> 
> That is, s/defer_connect/do_connect/ or similar, then invert the values in
> callers.

Will do, and will rename qtest_connect_deferred -> qtest_connect.

Thanks for reviewing these last few patches.

>>   {
>>       QTestState *s;
>>       int sock, qmpsock, i;
>> -    gchar *socket_path;
>> -    gchar *qmp_socket_path;
>> -
>> -    socket_path = g_strdup_printf("%s/qtest-%d.sock",
>> -                                  g_get_tmp_dir(), getpid());
>> -    qmp_socket_path = g_strdup_printf("%s/qtest-%d.qmp",
>> -                                      g_get_tmp_dir(), getpid());
>> +    g_autofree gchar *socket_path = qtest_socket_path("sock");
>> +    g_autofree gchar *qmp_socket_path = qtest_socket_path("qmp");
>>   
>>       /*
>>        * It's possible that if an earlier test run crashed it might
>> @@ -485,22 +488,17 @@ static QTestState *qtest_init_internal(const char *qemu_bin,
>>       qtest_client_set_rx_handler(s, qtest_client_socket_recv_line);
>>       qtest_client_set_tx_handler(s, qtest_client_socket_send);
>>   
>> -    s->fd = socket_accept(sock);
>> -    if (s->fd >= 0) {
>> -        s->qmp_fd = socket_accept(qmpsock);
>> -    }
>> -    unlink(socket_path);
>> -    unlink(qmp_socket_path);
>> -    g_free(socket_path);
>> -    g_free(qmp_socket_path);
>> -
>> -    g_assert(s->fd >= 0 && s->qmp_fd >= 0);
>> -
>>       s->rx = g_string_new("");
>>       for (i = 0; i < MAX_IRQ; i++) {
>>           s->irq_level[i] = false;
>>       }
>>   
>> +    s->sock = sock;
>> +    s->qmpsock = qmpsock;
>> +    if (!defer_connect) {
>> +        qtest_connect_deferred(s);
> 
> Now qtest_connect_deferred() itself has nothing to do with the "defer"
> concept.. it is the helper to connect the sockets, so maybe better call it
> qtest_connect_socks(), or similar.
> 
>> +    }
>> +
>>       /*
>>        * Stopping QEMU for debugging is not supported on Windows.
>>        *
>> @@ -515,34 +513,54 @@ static QTestState *qtest_init_internal(const char *qemu_bin,
>>       }
>>   #endif
>>   
>> +   return s;
>> +}
>> +
>> +void qtest_connect_deferred(QTestState *s)
>> +{
>> +    g_autofree gchar *socket_path = qtest_socket_path("sock");
>> +    g_autofree gchar *qmp_socket_path = qtest_socket_path("qmp");
>> +
>> +    g_assert(s->sock >= 0 && s->qmpsock >= 0);
>> +    s->fd = socket_accept(s->sock);
>> +    if (s->fd >= 0) {
>> +        s->qmp_fd = socket_accept(s->qmpsock);
>> +    }
>> +    unlink(socket_path);
>> +    unlink(qmp_socket_path);
>> +    g_assert(s->fd >= 0 && s->qmp_fd >= 0);
>> +    s->sock = s->qmpsock = -1;
>>       /* ask endianness of the target */
>> -
>>       s->big_endian = qtest_query_target_endianness(s);
>> -
>> -   return s;
>>   }
>>   
>>   QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
>>   {
>> -    return qtest_init_internal(qtest_qemu_binary(NULL), extra_args);
>> +    return qtest_init_internal(qtest_qemu_binary(NULL), extra_args, false);
>>   }
>>   
>> -QTestState *qtest_init_with_env(const char *var, const char *extra_args)
>> +void qtest_qmp_handshake(QTestState *s)
>>   {
>> -    QTestState *s = qtest_init_internal(qtest_qemu_binary(var), extra_args);
>> -    QDict *greeting;
>> -
>>       /* Read the QMP greeting and then do the handshake */
>> -    greeting = qtest_qmp_receive(s);
>> +    QDict *greeting = qtest_qmp_receive(s);
>>       qobject_unref(greeting);
>>       qobject_unref(qtest_qmp(s, "{ 'execute': 'qmp_capabilities' }"));
>> +}
>>   
>> +QTestState *qtest_init_with_env(const char *var, const char *extra_args,
>> +                                bool defer_connect)
>> +{
>> +    QTestState *s = qtest_init_internal(qtest_qemu_binary(var), extra_args,
>> +                                        defer_connect);
>> +    if (!defer_connect) {
>> +        qtest_qmp_handshake(s);
>> +    }
>>       return s;
>>   }
>>   
>>   QTestState *qtest_init(const char *extra_args)
>>   {
>> -    return qtest_init_with_env(NULL, extra_args);
>> +    return qtest_init_with_env(NULL, extra_args, false);
>>   }
>>   
>>   QTestState *qtest_vinitf(const char *fmt, va_list ap)
>> @@ -1523,7 +1541,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");
>> +    qts = qtest_init_with_env(qemu_var, "-machine none", false);
>>       response = qtest_qmp(qts, "{ 'execute': 'query-machines' }");
>>       g_assert(response);
>>       list = qdict_get_qlist(response, "return");
>> @@ -1578,7 +1596,7 @@ static struct CpuModel *qtest_get_cpu_models(void)
>>   
>>       silence_spawn_log = !g_test_verbose();
>>   
>> -    qts = qtest_init_with_env(NULL, "-machine none");
>> +    qts = qtest_init_with_env(NULL, "-machine none", false);
>>       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 beb96b1..db76f2c 100644
>> --- a/tests/qtest/libqtest.h
>> +++ b/tests/qtest/libqtest.h
>> @@ -60,13 +60,15 @@ QTestState *qtest_init(const char *extra_args);
>>    * @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.
>> + * @defer_connect: do not 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);
>> +QTestState *qtest_init_with_env(const char *var, const char *extra_args,
>> +                                bool defer_connect);
>>   
>>   /**
>>    * qtest_init_without_qmp_handshake:
>> @@ -78,6 +80,21 @@ QTestState *qtest_init_with_env(const char *var, const char *extra_args);
>>   QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
>>   
>>   /**
>> + * qtest_connect_deferred:
>> + * @s: #QTestState instance to connect
>> + * Connect to qemu monitor and qtest socket, after deferring them in
>> + * qtest_init_with_env.  Does not handshake with the monitor.
>> + */
>> +void qtest_connect_deferred(QTestState *s);
>> +
>> +/**
>> + * qtest_qmp_handshake:
>> + * @s: #QTestState instance to operate on.
>> + * Perform handshake after connecting to qemu monitor.
>> + */
>> +void qtest_qmp_handshake(QTestState *s);
>> +
>> +/**
>>    * qtest_init_with_serial:
>>    * @extra_args: other arguments to pass to QEMU.  CAUTION: these
>>    * arguments are subject to word splitting and shell evaluation.
>> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
>> index 3f8ba7f..9f39401 100644
>> --- a/tests/qtest/migration-helpers.c
>> +++ b/tests/qtest/migration-helpers.c
>> @@ -127,25 +127,28 @@ migrate_get_connect_qdict(QTestState *who)
>>   
>>   static void migrate_set_ports(QTestState *to, QList *channel_list)
>>   {
>> -    QDict *addr;
>> +    g_autoptr(QDict) addr = NULL;
>>       QListEntry *entry;
>>       const char *addr_port = NULL;
>>   
>> -    addr = migrate_get_connect_qdict(to);
>> -
>>       QLIST_FOREACH_ENTRY(channel_list, entry) {
>>           QDict *channel = qobject_to(QDict, qlist_entry_obj(entry));
>>           QDict *addrdict = qdict_get_qdict(channel, "addr");
>>   
>> -        if (qdict_haskey(addrdict, "port") &&
>> -            qdict_haskey(addr, "port") &&
>> -            (strcmp(qdict_get_str(addrdict, "port"), "0") == 0)) {
>> +        if (!qdict_haskey(addrdict, "port") ||
>> +            strcmp(qdict_get_str(addrdict, "port"), "0")) {
>> +            continue;
>> +        }
>> +
>> +        if (!addr) {
>> +            addr = migrate_get_connect_qdict(to);
> 
> May be good to add a comment above on why the query was done only lazily.
> 
> Meanwhile this chunk of change can be separate; it's relevant to the defer
> idea but still pretty standalone change. Can be one small patch prior to
> this one, IMHO.

Will do both.

> Optional idea, can be for later: if QTestState can have the state showing
> whether the QMP is ready, we could already assert making sure the query
> happens only if the QMP is available.

Good idea, I'll add it now in a small patch.  It already caught bugs as I
flipped defer_connect to do_connect :)

- Steve

>> +        }
>> +
>> +        if (qdict_haskey(addr, "port")) {
>>               addr_port = qdict_get_str(addr, "port");
>>               qdict_put_str(addrdict, "port", addr_port);
>>           }
>>       }
>> -
>> -    qobject_unref(addr);
>>   }
>>   
>>   bool migrate_watch_for_events(QTestState *who, const char *name,
>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>> index 64e1c50..b7001b0 100644
>> --- a/tests/qtest/migration-test.c
>> +++ b/tests/qtest/migration-test.c
>> @@ -844,7 +844,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>>                                    args->opts_source ? args->opts_source : "",
>>                                    ignore_stderr);
>>       if (!args->only_target) {
>> -        *from = qtest_init_with_env(QEMU_ENV_SRC, cmd_source);
>> +        *from = qtest_init_with_env(QEMU_ENV_SRC, cmd_source, false);
>>           qtest_qmp_set_event_callback(*from,
>>                                        migrate_watch_for_events,
>>                                        &src_state);
>> @@ -865,7 +865,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>>                                    shmem_opts ? shmem_opts : "",
>>                                    args->opts_target ? args->opts_target : "",
>>                                    ignore_stderr);
>> -    *to = qtest_init_with_env(QEMU_ENV_DST, cmd_target);
>> +    *to = qtest_init_with_env(QEMU_ENV_DST, cmd_target, false);
>>       qtest_qmp_set_event_callback(*to,
>>                                    migrate_watch_for_events,
>>                                    &dst_state);
>> -- 
>> 1.8.3.1
>>
>
diff mbox series

Patch

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 817fd7a..31c4032 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -75,6 +75,8 @@  struct QTestState
 {
     int fd;
     int qmp_fd;
+    int sock;
+    int qmpsock;
     pid_t qemu_pid;  /* our child QEMU process */
     int wstatus;
 #ifdef _WIN32
@@ -442,18 +444,19 @@  static QTestState *G_GNUC_PRINTF(2, 3) qtest_spawn_qemu(const char *qemu_bin,
     return s;
 }
 
+static char *qtest_socket_path(const char *suffix)
+{
+    return g_strdup_printf("%s/qtest-%d.%s", g_get_tmp_dir(), getpid(), suffix);
+}
+
 static QTestState *qtest_init_internal(const char *qemu_bin,
-                                       const char *extra_args)
+                                       const char *extra_args,
+                                       bool defer_connect)
 {
     QTestState *s;
     int sock, qmpsock, i;
-    gchar *socket_path;
-    gchar *qmp_socket_path;
-
-    socket_path = g_strdup_printf("%s/qtest-%d.sock",
-                                  g_get_tmp_dir(), getpid());
-    qmp_socket_path = g_strdup_printf("%s/qtest-%d.qmp",
-                                      g_get_tmp_dir(), getpid());
+    g_autofree gchar *socket_path = qtest_socket_path("sock");
+    g_autofree gchar *qmp_socket_path = qtest_socket_path("qmp");
 
     /*
      * It's possible that if an earlier test run crashed it might
@@ -485,22 +488,17 @@  static QTestState *qtest_init_internal(const char *qemu_bin,
     qtest_client_set_rx_handler(s, qtest_client_socket_recv_line);
     qtest_client_set_tx_handler(s, qtest_client_socket_send);
 
-    s->fd = socket_accept(sock);
-    if (s->fd >= 0) {
-        s->qmp_fd = socket_accept(qmpsock);
-    }
-    unlink(socket_path);
-    unlink(qmp_socket_path);
-    g_free(socket_path);
-    g_free(qmp_socket_path);
-
-    g_assert(s->fd >= 0 && s->qmp_fd >= 0);
-
     s->rx = g_string_new("");
     for (i = 0; i < MAX_IRQ; i++) {
         s->irq_level[i] = false;
     }
 
+    s->sock = sock;
+    s->qmpsock = qmpsock;
+    if (!defer_connect) {
+        qtest_connect_deferred(s);
+    }
+
     /*
      * Stopping QEMU for debugging is not supported on Windows.
      *
@@ -515,34 +513,54 @@  static QTestState *qtest_init_internal(const char *qemu_bin,
     }
 #endif
 
+   return s;
+}
+
+void qtest_connect_deferred(QTestState *s)
+{
+    g_autofree gchar *socket_path = qtest_socket_path("sock");
+    g_autofree gchar *qmp_socket_path = qtest_socket_path("qmp");
+
+    g_assert(s->sock >= 0 && s->qmpsock >= 0);
+    s->fd = socket_accept(s->sock);
+    if (s->fd >= 0) {
+        s->qmp_fd = socket_accept(s->qmpsock);
+    }
+    unlink(socket_path);
+    unlink(qmp_socket_path);
+    g_assert(s->fd >= 0 && s->qmp_fd >= 0);
+    s->sock = s->qmpsock = -1;
     /* ask endianness of the target */
-
     s->big_endian = qtest_query_target_endianness(s);
-
-   return s;
 }
 
 QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
 {
-    return qtest_init_internal(qtest_qemu_binary(NULL), extra_args);
+    return qtest_init_internal(qtest_qemu_binary(NULL), extra_args, false);
 }
 
-QTestState *qtest_init_with_env(const char *var, const char *extra_args)
+void qtest_qmp_handshake(QTestState *s)
 {
-    QTestState *s = qtest_init_internal(qtest_qemu_binary(var), extra_args);
-    QDict *greeting;
-
     /* Read the QMP greeting and then do the handshake */
-    greeting = qtest_qmp_receive(s);
+    QDict *greeting = qtest_qmp_receive(s);
     qobject_unref(greeting);
     qobject_unref(qtest_qmp(s, "{ 'execute': 'qmp_capabilities' }"));
+}
 
+QTestState *qtest_init_with_env(const char *var, const char *extra_args,
+                                bool defer_connect)
+{
+    QTestState *s = qtest_init_internal(qtest_qemu_binary(var), extra_args,
+                                        defer_connect);
+    if (!defer_connect) {
+        qtest_qmp_handshake(s);
+    }
     return s;
 }
 
 QTestState *qtest_init(const char *extra_args)
 {
-    return qtest_init_with_env(NULL, extra_args);
+    return qtest_init_with_env(NULL, extra_args, false);
 }
 
 QTestState *qtest_vinitf(const char *fmt, va_list ap)
@@ -1523,7 +1541,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");
+    qts = qtest_init_with_env(qemu_var, "-machine none", false);
     response = qtest_qmp(qts, "{ 'execute': 'query-machines' }");
     g_assert(response);
     list = qdict_get_qlist(response, "return");
@@ -1578,7 +1596,7 @@  static struct CpuModel *qtest_get_cpu_models(void)
 
     silence_spawn_log = !g_test_verbose();
 
-    qts = qtest_init_with_env(NULL, "-machine none");
+    qts = qtest_init_with_env(NULL, "-machine none", false);
     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 beb96b1..db76f2c 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -60,13 +60,15 @@  QTestState *qtest_init(const char *extra_args);
  * @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.
+ * @defer_connect: do not 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);
+QTestState *qtest_init_with_env(const char *var, const char *extra_args,
+                                bool defer_connect);
 
 /**
  * qtest_init_without_qmp_handshake:
@@ -78,6 +80,21 @@  QTestState *qtest_init_with_env(const char *var, const char *extra_args);
 QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
 
 /**
+ * qtest_connect_deferred:
+ * @s: #QTestState instance to connect
+ * Connect to qemu monitor and qtest socket, after deferring them in
+ * qtest_init_with_env.  Does not handshake with the monitor.
+ */
+void qtest_connect_deferred(QTestState *s);
+
+/**
+ * qtest_qmp_handshake:
+ * @s: #QTestState instance to operate on.
+ * Perform handshake after connecting to qemu monitor.
+ */
+void qtest_qmp_handshake(QTestState *s);
+
+/**
  * qtest_init_with_serial:
  * @extra_args: other arguments to pass to QEMU.  CAUTION: these
  * arguments are subject to word splitting and shell evaluation.
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 3f8ba7f..9f39401 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -127,25 +127,28 @@  migrate_get_connect_qdict(QTestState *who)
 
 static void migrate_set_ports(QTestState *to, QList *channel_list)
 {
-    QDict *addr;
+    g_autoptr(QDict) addr = NULL;
     QListEntry *entry;
     const char *addr_port = NULL;
 
-    addr = migrate_get_connect_qdict(to);
-
     QLIST_FOREACH_ENTRY(channel_list, entry) {
         QDict *channel = qobject_to(QDict, qlist_entry_obj(entry));
         QDict *addrdict = qdict_get_qdict(channel, "addr");
 
-        if (qdict_haskey(addrdict, "port") &&
-            qdict_haskey(addr, "port") &&
-            (strcmp(qdict_get_str(addrdict, "port"), "0") == 0)) {
+        if (!qdict_haskey(addrdict, "port") ||
+            strcmp(qdict_get_str(addrdict, "port"), "0")) {
+            continue;
+        }
+
+        if (!addr) {
+            addr = migrate_get_connect_qdict(to);
+        }
+
+        if (qdict_haskey(addr, "port")) {
             addr_port = qdict_get_str(addr, "port");
             qdict_put_str(addrdict, "port", addr_port);
         }
     }
-
-    qobject_unref(addr);
 }
 
 bool migrate_watch_for_events(QTestState *who, const char *name,
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 64e1c50..b7001b0 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -844,7 +844,7 @@  static int test_migrate_start(QTestState **from, QTestState **to,
                                  args->opts_source ? args->opts_source : "",
                                  ignore_stderr);
     if (!args->only_target) {
-        *from = qtest_init_with_env(QEMU_ENV_SRC, cmd_source);
+        *from = qtest_init_with_env(QEMU_ENV_SRC, cmd_source, false);
         qtest_qmp_set_event_callback(*from,
                                      migrate_watch_for_events,
                                      &src_state);
@@ -865,7 +865,7 @@  static int test_migrate_start(QTestState **from, QTestState **to,
                                  shmem_opts ? shmem_opts : "",
                                  args->opts_target ? args->opts_target : "",
                                  ignore_stderr);
-    *to = qtest_init_with_env(QEMU_ENV_DST, cmd_target);
+    *to = qtest_init_with_env(QEMU_ENV_DST, cmd_target, false);
     qtest_qmp_set_event_callback(*to,
                                  migrate_watch_for_events,
                                  &dst_state);