[Qemu-devel] [PATCH] Migration: Generate the completed event only when we complete

[Qemu-devel] [PATCH] Migration: Generate the completed event only when we complete

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The current migration-completed event is generated a bit too early,
which means that an eager libvirt that's ready to go as soon
as it sees the event ends up racing with the actual end of migration.

This corresponds to RH bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1271145

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index b7de9b7..3d40f24 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -294,12 +294,12 @@ static void process_incoming_migration_co(void *opaque)
         migrate_decompress_threads_join();
         exit(EXIT_FAILURE);
     }
-    migrate_generate_event(MIGRATION_STATUS_COMPLETED);
     qemu_announce_self();
 
     /* Make sure all file formats flush their mutable metadata */
     bdrv_invalidate_cache_all(&local_err);
     if (local_err) {
+        migrate_generate_event(MIGRATION_STATUS_FAILED);
         error_report_err(local_err);
         migrate_decompress_threads_join();
         exit(EXIT_FAILURE);
@@ -320,6 +320,12 @@ static void process_incoming_migration_co(void *opaque)
         runstate_set(global_state_get_runstate());
     }
     migrate_decompress_threads_join();
+    /*
+     * This must happen after any state changes since as soon as an external
+     * observer sees this event they might start to prod at the VM assuming
+     * it's ready to use.
+     */
+    migrate_generate_event(MIGRATION_STATUS_COMPLETED);
 }
 
 void process_incoming_migration(QEMUFile *f)
-- 
2.5.0

Re: [Qemu-devel] [PATCH] Migration: Generate the completed event only when we complete

[Amit Shah]

On (Tue) 13 Oct 2015 [12:21:27], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> The current migration-completed event is generated a bit too early,
> which means that an eager libvirt that's ready to go as soon
> as it sees the event ends up racing with the actual end of migration.
> 
> This corresponds to RH bug:
> https://bugzilla.redhat.com/show_bug.cgi?id=1271145
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Amit Shah <amit.shah@redhat.com>

>  migration/migration.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index b7de9b7..3d40f24 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -294,12 +294,12 @@ static void process_incoming_migration_co(void *opaque)
>          migrate_decompress_threads_join();
>          exit(EXIT_FAILURE);
>      }
> -    migrate_generate_event(MIGRATION_STATUS_COMPLETED);
>      qemu_announce_self();

Related in this context (but independent of this patch): this
announce_self should happen after we're through with all the error
cases: we don't want to announce our new IP and end up having run on
src due to a failed migration.

>      /* Make sure all file formats flush their mutable metadata */
>      bdrv_invalidate_cache_all(&local_err);
>      if (local_err) {
> +        migrate_generate_event(MIGRATION_STATUS_FAILED);
>          error_report_err(local_err);
>          migrate_decompress_threads_join();
>          exit(EXIT_FAILURE);

		Amit

Re: [Qemu-devel] [PATCH] Migration: Generate the completed event only when we complete

[Juan Quintela]

Amit Shah <amit.shah@redhat.com> wrote:
> On (Tue) 13 Oct 2015 [12:21:27], Dr. David Alan Gilbert (git) wrote:
>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> 
>> The current migration-completed event is generated a bit too early,
>> which means that an eager libvirt that's ready to go as soon
>> as it sees the event ends up racing with the actual end of migration.
>> 
>> This corresponds to RH bug:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1271145
>> 
>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> Reviewed-by: Amit Shah <amit.shah@redhat.com>
>
>>  migration/migration.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/migration/migration.c b/migration/migration.c
>> index b7de9b7..3d40f24 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -294,12 +294,12 @@ static void process_incoming_migration_co(void *opaque)
>>          migrate_decompress_threads_join();
>>          exit(EXIT_FAILURE);
>>      }
>> -    migrate_generate_event(MIGRATION_STATUS_COMPLETED);
>>      qemu_announce_self();
>
> Related in this context (but independent of this patch): this
> announce_self should happen after we're through with all the error
> cases: we don't want to announce our new IP and end up having run on
> src due to a failed migration.


Yeap, sounds very reasonable.

>
>>      /* Make sure all file formats flush their mutable metadata */
>>      bdrv_invalidate_cache_all(&local_err);
>>      if (local_err) {
>> +        migrate_generate_event(MIGRATION_STATUS_FAILED);
>>          error_report_err(local_err);
>>          migrate_decompress_threads_join();
>>          exit(EXIT_FAILURE);
>
> 		Amit

Re: [Qemu-devel] [PATCH] Migration: Generate the completed event only when we complete

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The current migration-completed event is generated a bit too early,
> which means that an eager libvirt that's ready to go as soon
> as it sees the event ends up racing with the actual end of migration.
>
> This corresponds to RH bug:
> https://bugzilla.redhat.com/show_bug.cgi?id=1271145
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>


Applied.


> ---
>  migration/migration.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index b7de9b7..3d40f24 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -294,12 +294,12 @@ static void process_incoming_migration_co(void *opaque)
>          migrate_decompress_threads_join();
>          exit(EXIT_FAILURE);
>      }
> -    migrate_generate_event(MIGRATION_STATUS_COMPLETED);
>      qemu_announce_self();
>  
>      /* Make sure all file formats flush their mutable metadata */
>      bdrv_invalidate_cache_all(&local_err);
>      if (local_err) {
> +        migrate_generate_event(MIGRATION_STATUS_FAILED);
>          error_report_err(local_err);
>          migrate_decompress_threads_join();
>          exit(EXIT_FAILURE);
> @@ -320,6 +320,12 @@ static void process_incoming_migration_co(void *opaque)
>          runstate_set(global_state_get_runstate());
>      }
>      migrate_decompress_threads_join();
> +    /*
> +     * This must happen after any state changes since as soon as an external
> +     * observer sees this event they might start to prod at the VM assuming
> +     * it's ready to use.
> +     */
> +    migrate_generate_event(MIGRATION_STATUS_COMPLETED);
>  }
>  
>  void process_incoming_migration(QEMUFile *f)