Re: [PATCH V7 23/24] migration-test: cpr-transfer

[Fabiano Rosas]

Steve Sistare <steven.sistare@oracle.com> writes:

> Add a migration test for cpr-transfer mode.  Defer the connection to the
> target monitor, else the test hangs because in cpr-transfer mode QEMU does
> not listen for monitor connections until we send the migrate command to
> source QEMU.
>
> To test -incoming defer, send a migrate incoming command to the target,
> after sending the migrate command to the source, as required by
> cpr-transfer mode.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> ---
>  tests/qtest/migration/cpr-tests.c | 62 
> +++++++++++++++++++++++++++++++++++++++
>  tests/qtest/migration/framework.c | 19 ++++++++++++
>  tests/qtest/migration/framework.h |  3 ++
>  3 files changed, 84 insertions(+)
>
> diff --git a/tests/qtest/migration/cpr-tests.c 
> b/tests/qtest/migration/cpr-tests.c
> index 44ce89a..215b0df 100644
> --- a/tests/qtest/migration/cpr-tests.c
> +++ b/tests/qtest/migration/cpr-tests.c
> @@ -44,6 +44,62 @@ static void test_mode_reboot(void)
>      test_file_common(&args, true);
>  }
>  
> +static void *test_mode_transfer_start(QTestState *from, QTestState *to)
> +{
> +    migrate_set_parameter_str(from, "mode", "cpr-transfer");
> +    return NULL;
> +}
> +
> +/*
> + * cpr-transfer mode cannot use the target monitor prior to starting the
> + * migration, and cannot connect synchronously to the monitor, so defer
> + * the target connection.
> + */
> +static void test_mode_transfer_common(bool incoming_defer)
> +{
> +    g_autofree char *cpr_path = g_strdup_printf("%s/cpr.sock", tmpfs);
> +    g_autofree char *mig_path = g_strdup_printf("%s/migsocket", tmpfs);
> +    g_autofree char *uri = g_strdup_printf("unix:%s", mig_path);
> +
> +    const char *opts = "-machine aux-ram-share=on -nodefaults";
> +    g_autofree const char *cpr_channel = g_strdup_printf(
> +        "cpr,addr.transport=socket,addr.type=unix,addr.path=%s",
> +        cpr_path);
> +    g_autofree char *opts_target = g_strdup_printf("-incoming %s %s",
> +                                                   cpr_channel, opts);
> +
> +    g_autofree char *connect_channels = g_strdup_printf(
> +        "[ { 'channel-type': 'main',"
> +        "    'addr': { 'transport': 'socket',"
> +        "              'type': 'unix',"
> +        "              'path': '%s' } } ]",
> +        mig_path);
> +
> +    MigrateCommon args = {
> +        .start.opts_source = opts,
> +        .start.opts_target = opts_target,
> +        .start.defer_target_connect = true,
> +        .start.memory_backend = "-object 
> memory-backend-memfd,id=pc.ram,size=%s"
> +                                " -machine memory-backend=pc.ram",
> +        .listen_uri = incoming_defer ? "defer" : uri,
> +        .connect_channels = connect_channels,
> +        .cpr_channel = cpr_channel,
> +        .start_hook = test_mode_transfer_start,
> +    };
> +
> +    test_precopy_common(&args);
> +}
> +
> +static void test_mode_transfer(void)
> +{
> +    test_mode_transfer_common(NULL);
> +}
> +
> +static void test_mode_transfer_defer(void)
> +{
> +    test_mode_transfer_common(true);
> +}
> +
>  void migration_test_add_cpr(MigrationTestEnv *env)
>  {
>      tmpfs = env->tmpfs;
> @@ -55,4 +111,10 @@ void migration_test_add_cpr(MigrationTestEnv *env)
>      if (getenv("QEMU_TEST_FLAKY_TESTS")) {
>          migration_test_add("/migration/mode/reboot", test_mode_reboot);
>      }
> +
> +    if (env->has_kvm) {
> +        migration_test_add("/migration/mode/transfer", test_mode_transfer);
> +        migration_test_add("/migration/mode/transfer/defer",
> +                           test_mode_transfer_defer);
> +    }
>  }
> diff --git a/tests/qtest/migration/framework.c 
> b/tests/qtest/migration/framework.c
> index 8d34cb2..699beda 100644
> --- a/tests/qtest/migration/framework.c
> +++ b/tests/qtest/migration/framework.c
> @@ -407,6 +407,7 @@ void migrate_end(QTestState *from, QTestState *to, bool 
> test_dest)
>      qtest_quit(to);
>  
>      cleanup("migsocket");
> +    cleanup("cpr.sock");
>      cleanup("src_serial");
>      cleanup("dest_serial");
>      cleanup(FILE_TEST_FILENAME);
> @@ -688,8 +689,11 @@ void test_precopy_common(MigrateCommon *args)
>  {
>      QTestState *from, *to;
>      void *data_hook = NULL;
> +    QObject *in_channels = NULL;
>      QObject *out_channels = NULL;
>  
> +    g_assert(!args->cpr_channel || args->connect_channels);
> +
>      if (migrate_start(&from, &to, args->listen_uri, &args->start)) {
>          return;
>      }
> @@ -721,8 +725,20 @@ void test_precopy_common(MigrateCommon *args)
>          }
>      }
>  
> +    /*
> +     * The cpr channel must be included in outgoing channels, but not in
> +     * migrate-incoming channels.
> +     */
>      if (args->connect_channels) {
> +        in_channels = qobject_from_json(args->connect_channels, 
> &error_abort);
>          out_channels = qobject_from_json(args->connect_channels, 
> &error_abort);
> +
> +        if (args->cpr_channel) {
> +            QList *channels_list = qobject_to(QList, out_channels);
> +            QObject *obj = migrate_str_to_channel(args->cpr_channel);
> +
> +            qlist_append(channels_list, obj);
> +        }
>      }
>  
>      if (args->result == MIG_TEST_QMP_ERROR) {
> @@ -735,6 +751,9 @@ void test_precopy_common(MigrateCommon *args)
>      if (args->start.defer_target_connect) {
>          qtest_connect(to);
>          qtest_qmp_handshake(to);
> +        if (!strcmp(args->listen_uri, "defer")) {
> +            migrate_incoming_qmp(to, args->connect_uri, in_channels, "{}");
> +        }

Paths that don't call migrate_incoming_qmp() never free
in_channels. We'll need something like this, let me know if I can squash
it in or you want to do it differently:

-- >8 --
>From 62d60c39b3e5d38cac20241e63b9d023bd296d2f Mon Sep 17 00:00:00 2001
From: Fabiano Rosas <farosas@suse.de>
Date: Thu, 16 Jan 2025 15:40:22 -0300
Subject: [PATCH] fixup! migration-test: cpr-transfer

---
 tests/qtest/migration/framework.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/qtest/migration/framework.c 
b/tests/qtest/migration/framework.c
index 699bedae69..1d5918d922 100644
--- a/tests/qtest/migration/framework.c
+++ b/tests/qtest/migration/framework.c
@@ -753,9 +753,14 @@ void test_precopy_common(MigrateCommon *args)
         qtest_qmp_handshake(to);
         if (!strcmp(args->listen_uri, "defer")) {
             migrate_incoming_qmp(to, args->connect_uri, in_channels, "{}");
+            in_channels = NULL;
         }
     }
 
+    if (in_channels) {
+        qobject_unref(in_channels);
+    }
+
     if (args->result != MIG_TEST_SUCCEED) {
         bool allow_active = args->result == MIG_TEST_FAIL;
         wait_for_migration_fail(from, allow_active);
-- 
2.35.3