|
| From: | Steven Sistare |
| Subject: | Re: [PATCH V5 22/23] migration-test: cpr-transfer |
| Date: | Thu, 2 Jan 2025 13:35:01 -0500 |
| User-agent: | Mozilla Thunderbird |
On 12/24/2024 3:01 PM, Peter Xu wrote:
On Tue, Dec 24, 2024 at 08:17:07AM -0800, Steve Sistare wrote: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> --- tests/qtest/migration/cpr-tests.c | 60 +++++++++++++++++++++++++++++++++++++++ tests/qtest/migration/framework.c | 19 +++++++++++++ tests/qtest/migration/framework.h | 3 ++ 3 files changed, 82 insertions(+) diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c index 44ce89a..b221980 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);So this test already uses json-format channels, IMHO we probably don't need MigrateCommon.cpr_channel anymore? We could put them all here. Then..
The previous version of this patch did that, and did not define cpr_channel, but you did not like how, in addition to using args->connect_channels, I defined the main uri in args->connect_uri and passed it to migrate_incoming_qmp. The constraint I must satisfy is that main + cpr channels must be passed to migrate_qmp, but only the main channel can be passed to migrate_incoming_qmp. I could instead define an optional args->incoming_channels, passed to migrate_incoming_qmp, and only set by cpr. Then the channel parsing extensions could be eliminated. This current patch with its channel parsing support does make test_mode_transfer_common nicer in one way: the cpr channel is only described once, in dotted keys format. A V6 that defines args->incoming_channels will need to define it once using dotted keys, to be embedded in startup args, and again in json in args->connect_uri. I don't feel strongly either way: keep V5, or define incoming_channels. - Steve
+ + 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,8 @@ void migration_test_add_cpr(MigrationTestEnv *env) if (getenv("QEMU_TEST_FLAKY_TESTS")) { migration_test_add("/migration/mode/reboot", test_mode_reboot); } + + 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 c6ea3e1..f6175de 100644 --- a/tests/qtest/migration/framework.c +++ b/tests/qtest/migration/framework.c @@ -411,6 +411,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); @@ -692,8 +693,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; } @@ -725,8 +729,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); + }... here IIUC we don't need this trick to manipulate the qlist anymore? Looks like if with that (convert the current cpr_connect string to JSON format, attach it with connect_channels), then we also don't need the pre-requisite patch to make connect_channels manipulate-able. Not sure.}if (args->result == MIG_TEST_QMP_ERROR) {@@ -739,6 +755,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, "{}"); + } }if (args->result != MIG_TEST_SUCCEED) {diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h index 1341368..4678e2a 100644 --- a/tests/qtest/migration/framework.h +++ b/tests/qtest/migration/framework.h @@ -152,6 +152,9 @@ typedef struct { */ const char *connect_channels;+ /* Optional: the cpr migration channel, in JSON or dotted keys format */+ const char *cpr_channel; + /* Optional: callback to run at start to set migration parameters */ TestMigrateStartHook start_hook; /* Optional: callback to run at finish to cleanup */ -- 1.8.3.1
| [Prev in Thread] | Current Thread | [Next in Thread] |