Re: [PATCH v15 13/14] migration: Implement MigrateChannelList to hmp migration flow.

[Juan Quintela]

I intrehated al parches until this forma next pull.

On Tue, Oct 31, 2023, 19:51 Het Gala <het.gala@nutanix.com> wrote:

On 31/10/23 11:12 pm, Juan Quintela wrote:
> Fabiano Rosas <farosas@suse.de> wrote:
>> From: Het Gala <het.gala@nutanix.com>
>>
>> Integrate MigrateChannelList with all transport backends
>> (socket, exec and rdma) for both src and dest migration
>> endpoints for hmp migration.
>>
>> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
>> Signed-off-by: Het Gala <het.gala@nutanix.com>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>
>>       bool resume = qdict_get_try_bool(qdict, "resume", false);
>>       const char *uri = qdict_get_str(qdict, "uri");
>>       Error *err = NULL;
>> +    MigrationChannelList *caps = NULL;
>> +    g_autoptr(MigrationChannel) channel = NULL;
>>   
>> -    qmp_migrate(uri, false, NULL, !!blk, blk, !!inc, inc,
>> +    if (!migrate_uri_parse(uri, &channel, &err)) {
>> +        goto end;
>> +    }
>> +    QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel));
>> +
>> +    qmp_migrate(NULL, true, caps, !!blk, blk, !!inc, inc,
>>                    false, false, true, resume, &err);
>
>> -    if (hmp_handle_error(mon, err)) {
>> -        return;
>> -    }
> I think that dropping this chunk is wrong.  What assures that
> qmp_migrate will not give an error?
>
>> +    qapi_free_MigrationChannelList(caps);
>
>
>
>
>>   
>>       if (!detach) {
>>           HMPMigrationStatus *status;
>> @@ -766,6 +780,9 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
>>                                             status);
>>           timer_mod(status->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME));
>>       }
>> +
>> +end:
>> +    hmp_handle_error(mon, err);
>
> Oh, you put it here, but you enter in the detach case even if there is
> one error.
>
> I think it is easier to just repeat the hmp_mhandle_error() inplace of
> the goto.

Okay Juan, will add hmp_handle_error() instead of goto statement

if (!migrate_uri_parse(uri, &channel, &err)) {
     hmp_handle_error(mon, err);
     return;
}

I will send new patchset, squash previous commits and add the tags
wherever required.

> Later, Juan.
Regards,
Het Gala