Juan Quintela <quint...@redhat.com> writes:
> 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?
>
Yes, I'll add for v2.
Thanks
