Fabiano Rosas <faro...@suse.de> wrote:
> We are sending a migration event of MIGRATION_STATUS_SETUP at
> qemu_start_incoming_migration but never actually setting the state.
>
> This creates a window between qmp_migrate_incoming and
> process_incoming_migration_co where the migration status is still
> MIGRATION_STATUS_NONE. Calling query-migrate during this time will
> return an empty response even though the incoming migration command
> has already been issued.
>
> Commit 7cf1fe6d68 ("migration: Add migration events on target side")
> has added support to the 'events' capability to the incoming part of
> migration, but chose to send the SETUP event without setting the
> state. I'm assuming this was a mistake.
>
> To avoid introducing a change in behavior, we need to keep sending the
> SETUP event, even if the 'events' capability is not set.
>
> Signed-off-by: Fabiano Rosas <faro...@suse.de>
> ---
> migration/migration.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 7c8292d4d4..562b78261d 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -424,13 +424,26 @@ void migrate_add_address(SocketAddress *address)
> static void qemu_start_incoming_migration(const char *uri, Error **errp)
> {
> const char *p = NULL;
> + MigrationIncomingState *mis = migration_incoming_get_current();
>
> /* URI is not suitable for migration? */
> if (!migration_channels_and_uri_compatible(uri, errp)) {
> return;
> }
>
> - qapi_event_send_migration(MIGRATION_STATUS_SETUP);
> + migrate_set_state(&mis->state, MIGRATION_STATUS_NONE,
> + MIGRATION_STATUS_SETUP);
> + /*
> + * QMP clients should have set the 'events' migration capability
> + * if they want to receive this event, in which case the
> + * migrate_set_state() call above will have already sent the
> + * event. We still need to send the event for compatibility even
> + * if migration events are disabled.
> + */
> + if (!migrate_events()) {
> + qapi_event_send_migration(MIGRATION_STATUS_SETUP);
> + }
Can we add and test for a property here, so we can drop this at some
point in the future?
> +
> if (strstart(uri, "tcp:", &p) ||
> strstart(uri, "unix:", NULL) ||
> strstart(uri, "vsock:", NULL)) {
> @@ -524,7 +537,7 @@ process_incoming_migration_co(void *opaque)
>
> mis->largest_page_size = qemu_ram_pagesize_largest();
> postcopy_state_set(POSTCOPY_INCOMING_NONE);
> - migrate_set_state(&mis->state, MIGRATION_STATUS_NONE,
> + migrate_set_state(&mis->state, MIGRATION_STATUS_SETUP,
> MIGRATION_STATUS_ACTIVE);
>
> mis->loadvm_co = qemu_coroutine_self();
