On Thu, Jan 02, 2025 at 01:35:01PM -0500, Steven Sistare wrote:
> 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.sist...@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.
Hmm true, I see now.
>
> 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.
Me too, please choose whatever easier for you.
Thanks,
--
Peter Xu
