Re: [PATCH V4 18/19] migration-test: cpr-transfer

[Peter Xu <peterx@redhat.com>]

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..

-- 
Peter Xu