| Message ID | 20240410111541.188504-5-het.gala@nutanix.com |
|---|---|
| State | New |
| Headers | show |
| Series | tests/qtest/migration: Add postcopy qtests for introducing 'channels' argument with new QAPI syntax | expand |
Het Gala <het.gala@nutanix.com> writes: > Add qtests to perform postcopy live migration by having list of > 'channels' argument as the starting point instead of uri string. > (Note: length of the list is restricted to 1 for now) > > Signed-off-by: Het Gala <het.gala@nutanix.com> > --- > tests/qtest/migration-test.c | 38 ++++++++++++++++++++++++++++++++++-- > 1 file changed, 36 insertions(+), 2 deletions(-) > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > index fa8a860811..599018baa0 100644 > --- a/tests/qtest/migration-test.c > +++ b/tests/qtest/migration-test.c > @@ -1296,13 +1296,17 @@ static int migrate_postcopy_prepare(QTestState **from_ptr, > migrate_ensure_non_converge(from); > > migrate_prepare_for_dirty_mem(from); > - migrate_incoming_qmp(to, "tcp:127.0.0.1:0", NULL, "{}"); > + if (args->connect_channels) { > + migrate_incoming_qmp(to, NULL, args->connect_channels, "{}"); > + } else { > + migrate_incoming_qmp(to, "tcp:127.0.0.1:0", NULL, "{}"); > + } From an interface perspective it's a bit unexpected to need this conditional when the migrate_qmp below doesn't need it. > > /* Wait for the first serial output from the source */ > wait_for_serial("src_serial"); > wait_for_suspend(from, &src_state); > > - migrate_qmp(from, to, NULL, NULL, "{}"); > + migrate_qmp(from, to, args->connect_uri, args->connect_channels, "{}"); > > migrate_wait_for_dirty_mem(from, to); > > @@ -1355,6 +1359,20 @@ static void test_postcopy(void) > test_postcopy_common(&args); > } > > +static void test_postcopy_channels(void) > +{ > + MigrateCommon args = { > + .listen_uri = "defer", > + .connect_channels = "[ { 'channel-type': 'main'," > + " 'addr': { 'transport': 'socket'," > + " 'type': 'inet'," > + " 'host': '127.0.0.1'," > + " 'port': '0' } } ]", > + }; > + > + test_postcopy_common(&args); > +} > + > static void test_postcopy_suspend(void) > { > MigrateCommon args = { > @@ -1555,6 +1573,18 @@ static void test_postcopy_recovery(void) > test_postcopy_recovery_common(&args); > } > > +static void test_postcopy_recovery_channels(void) > +{ > + MigrateCommon args = { > + .connect_channels = "[ { 'channel-type': 'main'," > + " 'addr': { 'transport': 'socket'," > + " 'type': 'inet'," > + " 'host': '127.0.0.1'," > + " 'port': '0' } } ]", > + }; > + > + test_postcopy_recovery_common(&args); > +} > static void test_postcopy_recovery_compress(void) > { > MigrateCommon args = { > @@ -3585,8 +3615,12 @@ int main(int argc, char **argv) > > if (has_uffd) { > migration_test_add("/migration/postcopy/plain", test_postcopy); > + migration_test_add("/migration/postcopy/channels/plain", > + test_postcopy_channels); > migration_test_add("/migration/postcopy/recovery/plain", > test_postcopy_recovery); > + migration_test_add("/migration/postcopy/recovery/channels/plain", > + test_postcopy_recovery_channels); > migration_test_add("/migration/postcopy/preempt/plain", > test_postcopy_preempt); > migration_test_add("/migration/postcopy/preempt/recovery/plain",
On 10/04/24 6:45 pm, Fabiano Rosas wrote: > !-------------------------------------------------------------------| > CAUTION: External Email > > |-------------------------------------------------------------------! > > Het Gala<het.gala@nutanix.com> writes: > >> Add qtests to perform postcopy live migration by having list of >> 'channels' argument as the starting point instead of uri string. >> (Note: length of the list is restricted to 1 for now) >> >> Signed-off-by: Het Gala<het.gala@nutanix.com> >> --- >> tests/qtest/migration-test.c | 38 ++++++++++++++++++++++++++++++++++-- >> 1 file changed, 36 insertions(+), 2 deletions(-) >> >> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c >> index fa8a860811..599018baa0 100644 >> --- a/tests/qtest/migration-test.c >> +++ b/tests/qtest/migration-test.c >> @@ -1296,13 +1296,17 @@ static int migrate_postcopy_prepare(QTestState **from_ptr, >> migrate_ensure_non_converge(from); >> >> migrate_prepare_for_dirty_mem(from); >> - migrate_incoming_qmp(to, "tcp:127.0.0.1:0", NULL, "{}"); >> + if (args->connect_channels) { >> + migrate_incoming_qmp(to, NULL, args->connect_channels, "{}"); >> + } else { >> + migrate_incoming_qmp(to, "tcp:127.0.0.1:0", NULL, "{}"); >> + } > From an interface perspective it's a bit unexpected to need this > conditional when the migrate_qmp below doesn't need it. I think, migrate_qmp has an unique advantage here. Please correct me If my understanding is not correct. 1. test_migrate_start starts the qemu cmd line with either --incoming tcp:127.0.0.1:0 or "defer". If tcp uri is provided, then migrate_incoming_qmp is not necessary 2. If "defer" is passed, then only migrate_incoming_qmp is required with tcp uri string 3. migrate_qmp can get the uri anyway either from test_migrate_start -> defer + migrate_incoming_qmp -> tcp:127.0.0.1:0 test_migrate_start -> tcp:127.0.0.1:0 with the help of migrate_get_connect_uri() with the correct migration port. Even if we always pass NULL, we are okay, But this is just for tcp transport, and not unix We can't have the unique situation for migrate_incoming_qmp, hence avoiding conditional earlier seemed to be necessary for me. But, with the hack suggested by you in previous patch, will prevent us from using conditional if else >> >> /* Wait for the first serial output from the source */ >> wait_for_serial("src_serial"); >> wait_for_suspend(from, &src_state); >> >> - migrate_qmp(from, to, NULL, NULL, "{}"); >> + migrate_qmp(from, to, args->connect_uri, args->connect_channels, "{}"); >> >> migrate_wait_for_dirty_mem(from, to); >> >> @@ -1355,6 +1359,20 @@ static void test_postcopy(void) >> test_postcopy_common(&args); >> } >> >> +static void test_postcopy_channels(void) >> +{ >> + MigrateCommon args = { >> + .listen_uri = "defer", >> + .connect_channels = "[ { 'channel-type': 'main'," >> + " 'addr': { 'transport': 'socket'," >> + " 'type': 'inet'," >> + " 'host': '127.0.0.1'," >> + " 'port': '0' } } ]", >> + }; >> + >> + test_postcopy_common(&args); >> +} >> + >> static void test_postcopy_suspend(void) >> { >> MigrateCommon args = { >> @@ -1555,6 +1573,18 @@ static void test_postcopy_recovery(void) >> test_postcopy_recovery_common(&args); >> } >> >> +static void test_postcopy_recovery_channels(void) >> +{ >> + MigrateCommon args = { >> + .connect_channels = "[ { 'channel-type': 'main'," >> + " 'addr': { 'transport': 'socket'," >> + " 'type': 'inet'," >> + " 'host': '127.0.0.1'," >> + " 'port': '0' } } ]", >> + }; >> + >> + test_postcopy_recovery_common(&args); >> +} >> static void test_postcopy_recovery_compress(void) >> { >> MigrateCommon args = { >> @@ -3585,8 +3615,12 @@ int main(int argc, char **argv) >> >> if (has_uffd) { >> migration_test_add("/migration/postcopy/plain", test_postcopy); >> + migration_test_add("/migration/postcopy/channels/plain", >> + test_postcopy_channels); >> migration_test_add("/migration/postcopy/recovery/plain", >> test_postcopy_recovery); >> + migration_test_add("/migration/postcopy/recovery/channels/plain", >> + test_postcopy_recovery_channels); >> migration_test_add("/migration/postcopy/preempt/plain", >> test_postcopy_preempt); >> migration_test_add("/migration/postcopy/preempt/recovery/plain", Regards, Het Gala
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index fa8a860811..599018baa0 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -1296,13 +1296,17 @@ static int migrate_postcopy_prepare(QTestState **from_ptr, migrate_ensure_non_converge(from); migrate_prepare_for_dirty_mem(from); - migrate_incoming_qmp(to, "tcp:127.0.0.1:0", NULL, "{}"); + if (args->connect_channels) { + migrate_incoming_qmp(to, NULL, args->connect_channels, "{}"); + } else { + migrate_incoming_qmp(to, "tcp:127.0.0.1:0", NULL, "{}"); + } /* Wait for the first serial output from the source */ wait_for_serial("src_serial"); wait_for_suspend(from, &src_state); - migrate_qmp(from, to, NULL, NULL, "{}"); + migrate_qmp(from, to, args->connect_uri, args->connect_channels, "{}"); migrate_wait_for_dirty_mem(from, to); @@ -1355,6 +1359,20 @@ static void test_postcopy(void) test_postcopy_common(&args); } +static void test_postcopy_channels(void) +{ + MigrateCommon args = { + .listen_uri = "defer", + .connect_channels = "[ { 'channel-type': 'main'," + " 'addr': { 'transport': 'socket'," + " 'type': 'inet'," + " 'host': '127.0.0.1'," + " 'port': '0' } } ]", + }; + + test_postcopy_common(&args); +} + static void test_postcopy_suspend(void) { MigrateCommon args = { @@ -1555,6 +1573,18 @@ static void test_postcopy_recovery(void) test_postcopy_recovery_common(&args); } +static void test_postcopy_recovery_channels(void) +{ + MigrateCommon args = { + .connect_channels = "[ { 'channel-type': 'main'," + " 'addr': { 'transport': 'socket'," + " 'type': 'inet'," + " 'host': '127.0.0.1'," + " 'port': '0' } } ]", + }; + + test_postcopy_recovery_common(&args); +} static void test_postcopy_recovery_compress(void) { MigrateCommon args = { @@ -3585,8 +3615,12 @@ int main(int argc, char **argv) if (has_uffd) { migration_test_add("/migration/postcopy/plain", test_postcopy); + migration_test_add("/migration/postcopy/channels/plain", + test_postcopy_channels); migration_test_add("/migration/postcopy/recovery/plain", test_postcopy_recovery); + migration_test_add("/migration/postcopy/recovery/channels/plain", + test_postcopy_recovery_channels); migration_test_add("/migration/postcopy/preempt/plain", test_postcopy_preempt); migration_test_add("/migration/postcopy/preempt/recovery/plain",
Add qtests to perform postcopy live migration by having list of 'channels' argument as the starting point instead of uri string. (Note: length of the list is restricted to 1 for now) Signed-off-by: Het Gala <het.gala@nutanix.com> --- tests/qtest/migration-test.c | 38 ++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-)