| Message ID | 1733145611-62315-19-git-send-email-steven.sistare@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Live update: cpr-transfer | expand |
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 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-test.c | 72 +++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 71 insertions(+), 1 deletion(-) > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > index 8bc665d..4eb641c 100644 > --- a/tests/qtest/migration-test.c > +++ b/tests/qtest/migration-test.c > @@ -1729,6 +1729,7 @@ static void test_precopy_common(MigrateCommon *args) > { > QTestState *from, *to; > void *data_hook = NULL; > + const char *connect_uri; > > if (test_migrate_start(&from, &to, args->listen_uri, &args->start)) { > return; > @@ -1766,11 +1767,16 @@ static void test_precopy_common(MigrateCommon *args) > goto finish; > } > > - migrate_qmp(from, to, args->connect_uri, args->connect_channels, "{}"); > + /* If has channels, then connect_uri is only used for listen defer */ > + connect_uri = args->connect_channels ? NULL : args->connect_uri; > + migrate_qmp(from, to, connect_uri, args->connect_channels, "{}"); > > if (args->start.defer_target_connect) { > qtest_connect_deferred(to); > qtest_qmp_handshake(to); > + if (!strcmp(args->listen_uri, "defer")) { > + migrate_incoming_qmp(to, args->connect_uri, "{}"); > + } > } > > if (args->result != MIG_TEST_SUCCEED) { > @@ -2415,6 +2421,66 @@ static void test_multifd_file_mapped_ram_fdset_dio(void) > } > #endif /* !_WIN32 */ > > +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 char *opts_target = g_strdup_printf( > + "-incoming \\{\\\'channel-type\\\':\\\'cpr\\\'," > + "\\\'addr\\\':\\{\\\'transport\\\':\\\'socket\\\'," > + "\\\'type\\\':\\\'unix\\\',\\\'path\\\':\\\'%s\\\'\\}\\} %s", > + cpr_path, opts); > + > + g_autofree char *channels = g_strdup_printf( > + "[ { 'channel-type': 'main'," > + " 'addr': { 'transport': 'socket'," > + " 'type': 'unix'," > + " 'path': '%s' } }," > + " { 'channel-type': 'cpr'," > + " 'addr': { 'transport': 'socket'," > + " 'type': 'unix'," > + " 'path': '%s' } } ]", > + mig_path, cpr_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_uri = incoming_defer ? uri : NULL, > + .connect_channels = channels, > + .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); > +} > + > static void test_precopy_tcp_plain(void) > { > MigrateCommon args = { > @@ -3905,6 +3971,10 @@ int main(int argc, char **argv) > 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); > + > migration_test_add("/migration/precopy/file/mapped-ram", > test_precopy_file_mapped_ram); > migration_test_add("/migration/precopy/file/mapped-ram/live",
On Mon, Dec 02, 2024 at 05:20:10AM -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-test.c | 72 +++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 71 insertions(+), 1 deletion(-) > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > index 8bc665d..4eb641c 100644 > --- a/tests/qtest/migration-test.c > +++ b/tests/qtest/migration-test.c > @@ -1729,6 +1729,7 @@ static void test_precopy_common(MigrateCommon *args) > { > QTestState *from, *to; > void *data_hook = NULL; > + const char *connect_uri; > > if (test_migrate_start(&from, &to, args->listen_uri, &args->start)) { > return; > @@ -1766,11 +1767,16 @@ static void test_precopy_common(MigrateCommon *args) > goto finish; > } > > - migrate_qmp(from, to, args->connect_uri, args->connect_channels, "{}"); > + /* If has channels, then connect_uri is only used for listen defer */ > + connect_uri = args->connect_channels ? NULL : args->connect_uri; > + migrate_qmp(from, to, connect_uri, args->connect_channels, "{}"); This smells like abuse. If the test case sets connect_uri only because of below... > > if (args->start.defer_target_connect) { > qtest_connect_deferred(to); > qtest_qmp_handshake(to); > + if (!strcmp(args->listen_uri, "defer")) { > + migrate_incoming_qmp(to, args->connect_uri, "{}"); ... here, then IMHO it's abusing connect_uri to start service incoming ports. We do have solution for "delay" incoming, right? Shouldn't we use migrate_get_connect_uri() instead, then never set connect_uri in cpr-transfer tests? > + } > } > > if (args->result != MIG_TEST_SUCCEED) { > @@ -2415,6 +2421,66 @@ static void test_multifd_file_mapped_ram_fdset_dio(void) > } > #endif /* !_WIN32 */ > > +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 char *opts_target = g_strdup_printf( > + "-incoming \\{\\\'channel-type\\\':\\\'cpr\\\'," > + "\\\'addr\\\':\\{\\\'transport\\\':\\\'socket\\\'," > + "\\\'type\\\':\\\'unix\\\',\\\'path\\\':\\\'%s\\\'\\}\\} %s", > + cpr_path, opts); Nobody will be able to change this easily.. Maybe use g_strescape()? > + > + g_autofree char *channels = g_strdup_printf( > + "[ { 'channel-type': 'main'," > + " 'addr': { 'transport': 'socket'," > + " 'type': 'unix'," > + " 'path': '%s' } }," > + " { 'channel-type': 'cpr'," > + " 'addr': { 'transport': 'socket'," > + " 'type': 'unix'," > + " 'path': '%s' } } ]", > + mig_path, cpr_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_uri = incoming_defer ? uri : NULL, > + .connect_channels = channels, > + .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); > +} > + > static void test_precopy_tcp_plain(void) > { > MigrateCommon args = { > @@ -3905,6 +3971,10 @@ int main(int argc, char **argv) > 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); > + > migration_test_add("/migration/precopy/file/mapped-ram", > test_precopy_file_mapped_ram); > migration_test_add("/migration/precopy/file/mapped-ram/live", > -- > 1.8.3.1 >
On 12/19/2024 11:56 AM, Peter Xu wrote: > On Mon, Dec 02, 2024 at 05:20:10AM -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-test.c | 72 +++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 71 insertions(+), 1 deletion(-) >> >> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c >> index 8bc665d..4eb641c 100644 >> --- a/tests/qtest/migration-test.c >> +++ b/tests/qtest/migration-test.c >> @@ -1729,6 +1729,7 @@ static void test_precopy_common(MigrateCommon *args) >> { >> QTestState *from, *to; >> void *data_hook = NULL; >> + const char *connect_uri; >> >> if (test_migrate_start(&from, &to, args->listen_uri, &args->start)) { >> return; >> @@ -1766,11 +1767,16 @@ static void test_precopy_common(MigrateCommon *args) >> goto finish; >> } >> >> - migrate_qmp(from, to, args->connect_uri, args->connect_channels, "{}"); >> + /* If has channels, then connect_uri is only used for listen defer */ >> + connect_uri = args->connect_channels ? NULL : args->connect_uri; >> + migrate_qmp(from, to, connect_uri, args->connect_channels, "{}"); > > This smells like abuse. > > If the test case sets connect_uri only because of below... > >> >> if (args->start.defer_target_connect) { >> qtest_connect_deferred(to); >> qtest_qmp_handshake(to); >> + if (!strcmp(args->listen_uri, "defer")) { >> + migrate_incoming_qmp(to, args->connect_uri, "{}"); > > ... here, then IMHO it's abusing connect_uri to start service incoming > ports. > > We do have solution for "delay" incoming, right? Shouldn't we use > migrate_get_connect_uri() instead, then never set connect_uri in > cpr-transfer tests? We cannot use migrate_get_connect_uri() to get the URI to pass to migrate_incoming_qmp, because the migrate_incoming_qmp sets the URI returned by query-migrate. chicken-and-egg problem. I'll add channels support to migrate_incoming_qmp, like migrate_qmp. The cpr-transfer test will set listen_uri and connect_channels but will not set connect_uri. >> + } >> } >> >> if (args->result != MIG_TEST_SUCCEED) { >> @@ -2415,6 +2421,66 @@ static void test_multifd_file_mapped_ram_fdset_dio(void) >> } >> #endif /* !_WIN32 */ >> >> +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 char *opts_target = g_strdup_printf( >> + "-incoming \\{\\\'channel-type\\\':\\\'cpr\\\'," >> + "\\\'addr\\\':\\{\\\'transport\\\':\\\'socket\\\'," >> + "\\\'type\\\':\\\'unix\\\',\\\'path\\\':\\\'%s\\\'\\}\\} %s", >> + cpr_path, opts); > > Nobody will be able to change this easily.. Maybe use g_strescape()? Agreed. Fortunately incoming now accepts dotted keys after a suggestion from Markus, so this can be: -incoming cpr,addr.transport=socket,addr.type=unix,addr.path=%s - Steve >> + >> + g_autofree char *channels = g_strdup_printf( >> + "[ { 'channel-type': 'main'," >> + " 'addr': { 'transport': 'socket'," >> + " 'type': 'unix'," >> + " 'path': '%s' } }," >> + " { 'channel-type': 'cpr'," >> + " 'addr': { 'transport': 'socket'," >> + " 'type': 'unix'," >> + " 'path': '%s' } } ]", >> + mig_path, cpr_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_uri = incoming_defer ? uri : NULL, >> + .connect_channels = channels, >> + .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); >> +} >> + >> static void test_precopy_tcp_plain(void) >> { >> MigrateCommon args = { >> @@ -3905,6 +3971,10 @@ int main(int argc, char **argv) >> 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); >> + >> migration_test_add("/migration/precopy/file/mapped-ram", >> test_precopy_file_mapped_ram); >> migration_test_add("/migration/precopy/file/mapped-ram/live", >> -- >> 1.8.3.1 >> >
On Thu, Dec 19, 2024 at 05:34:59PM -0500, Steven Sistare wrote: > > > @@ -1766,11 +1767,16 @@ static void test_precopy_common(MigrateCommon *args) > > > goto finish; > > > } > > > - migrate_qmp(from, to, args->connect_uri, args->connect_channels, "{}"); > > > + /* If has channels, then connect_uri is only used for listen defer */ > > > + connect_uri = args->connect_channels ? NULL : args->connect_uri; > > > + migrate_qmp(from, to, connect_uri, args->connect_channels, "{}"); > > > > This smells like abuse. > > > > If the test case sets connect_uri only because of below... > > > > > if (args->start.defer_target_connect) { > > > qtest_connect_deferred(to); > > > qtest_qmp_handshake(to); > > > + if (!strcmp(args->listen_uri, "defer")) { > > > + migrate_incoming_qmp(to, args->connect_uri, "{}"); > > > > ... here, then IMHO it's abusing connect_uri to start service incoming > > ports. > > > > We do have solution for "delay" incoming, right? Shouldn't we use > > migrate_get_connect_uri() instead, then never set connect_uri in > > cpr-transfer tests? > > We cannot use migrate_get_connect_uri() to get the URI to pass to > migrate_incoming_qmp, because the migrate_incoming_qmp sets the URI > returned by query-migrate. chicken-and-egg problem. Oh yes, stupid me. > > I'll add channels support to migrate_incoming_qmp, like migrate_qmp. > The cpr-transfer test will set listen_uri and connect_channels but > will not set connect_uri. That's still a lightweight abuse, but better than connect_uri indeed. Hopefully cpr is the only one that uses defer_target_connect, so yeah we can go with it at least for now..
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index 8bc665d..4eb641c 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -1729,6 +1729,7 @@ static void test_precopy_common(MigrateCommon *args) { QTestState *from, *to; void *data_hook = NULL; + const char *connect_uri; if (test_migrate_start(&from, &to, args->listen_uri, &args->start)) { return; @@ -1766,11 +1767,16 @@ static void test_precopy_common(MigrateCommon *args) goto finish; } - migrate_qmp(from, to, args->connect_uri, args->connect_channels, "{}"); + /* If has channels, then connect_uri is only used for listen defer */ + connect_uri = args->connect_channels ? NULL : args->connect_uri; + migrate_qmp(from, to, connect_uri, args->connect_channels, "{}"); if (args->start.defer_target_connect) { qtest_connect_deferred(to); qtest_qmp_handshake(to); + if (!strcmp(args->listen_uri, "defer")) { + migrate_incoming_qmp(to, args->connect_uri, "{}"); + } } if (args->result != MIG_TEST_SUCCEED) { @@ -2415,6 +2421,66 @@ static void test_multifd_file_mapped_ram_fdset_dio(void) } #endif /* !_WIN32 */ +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 char *opts_target = g_strdup_printf( + "-incoming \\{\\\'channel-type\\\':\\\'cpr\\\'," + "\\\'addr\\\':\\{\\\'transport\\\':\\\'socket\\\'," + "\\\'type\\\':\\\'unix\\\',\\\'path\\\':\\\'%s\\\'\\}\\} %s", + cpr_path, opts); + + g_autofree char *channels = g_strdup_printf( + "[ { 'channel-type': 'main'," + " 'addr': { 'transport': 'socket'," + " 'type': 'unix'," + " 'path': '%s' } }," + " { 'channel-type': 'cpr'," + " 'addr': { 'transport': 'socket'," + " 'type': 'unix'," + " 'path': '%s' } } ]", + mig_path, cpr_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_uri = incoming_defer ? uri : NULL, + .connect_channels = channels, + .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); +} + static void test_precopy_tcp_plain(void) { MigrateCommon args = { @@ -3905,6 +3971,10 @@ int main(int argc, char **argv) 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); + migration_test_add("/migration/precopy/file/mapped-ram", test_precopy_file_mapped_ram); migration_test_add("/migration/precopy/file/mapped-ram/live",
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-test.c | 72 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-)