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

[Juan Quintela]

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.

Later, Juan.