Peter Xu <[email protected]> writes: > On Fri, Dec 26, 2025 at 06:19:06PM -0300, Fabiano Rosas wrote: >> The multifd_recv_setup() call is currently in a place where it will be >> called for every channel that appears. That doesn't make much >> sense. >> >> It seems it was moved when the channel discovery mechanism was added >> back at commit 6720c2b327 (migration: check magic value for deciding >> the mapping of channels, 2022-12-20). The original place was >> migration_incoming_setup() which would run for just the main channel, >> but it was discovered that the main channel might arrive after a >> multifd channel. >> >> Move the call back to a place where it will be called only once. >> >> With respect to cleanup, this new location at >> qemu_start_incoming_migration() has the same issue as the previous >> callsite at migration_ioc_process_incoming(): no cleanup ever happens. >> >> The error message goes from being emitted via error_report_err(), to >> being returned to the qmp_migrate_incoming() incoming command, which >> is arguably better, since this is setup code. > > This is not the only and real reason that you moved it, right? >
It was odd where it was and I just moved it. It could probably remain there even after the rest of the series, I didn't check. I think it would then need to move to channel.c which would make that file access multifd code, so maybe it's a layering argument. > Neither should it be the reason that you want it to be called only exactly > once; after all the function will be no-op in the 2nd+ calls. > It's not a no-op. But yes, it returns early on subsequent calls. > I'll keep reading.. I'm guessing I'll find it later, but IMHO it'll always > be good to mention the real motivation in the commit log. > >> >> Signed-off-by: Fabiano Rosas <[email protected]> >> --- >> migration/migration.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/migration/migration.c b/migration/migration.c >> index 71efe945f6..974313944c 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -786,6 +786,10 @@ static void qemu_start_incoming_migration(const char >> *uri, bool has_channels, >> return; >> } >> >> + if (multifd_recv_setup(errp) != 0) { >> + return; >> + } >> + >> if (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET) { >> SocketAddress *saddr = &addr->u.socket; >> if (saddr->type == SOCKET_ADDRESS_TYPE_INET || >> @@ -1065,10 +1069,6 @@ void migration_ioc_process_incoming(QIOChannel *ioc, >> Error **errp) >> channel = CH_POSTCOPY; >> } >> >> - if (multifd_recv_setup(errp) != 0) { >> - return; >> - } >> - >> if (channel == CH_MAIN) { >> f = qemu_file_new_input(ioc); >> migration_incoming_setup(f); >> -- >> 2.51.0 >>
