Peter Xu <peterx@redhat.com> 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 <farosas@suse.de>
>> ---
>> 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
>>