Patchwork migration: fix detached migration with fd

login
register
mail settings
Submitter Juan Quintela
Date Nov. 9, 2011, 8:29 p.m.
Message ID <1320870541-4571-1-git-send-email-quintela@redhat.com>
Download mbox | patch
Permalink /patch/124695/
State New
Headers show

Comments

Juan Quintela - Nov. 9, 2011, 8:29 p.m.
Migration with fd uses s->mon to pass the fd.  But we only assign the
s->mon for !detached migration.  Fix it.  Once there add a comment
indicating that s->mon has two uses.

Bug reported by:  Wen Congyang <wency@cn.fujitsu.com>

Signed-off-by: Juan Quintela <quintela@redhat.com>
CC:  Wen Congyang <wency@cn.fujitsu.com>
---
 migration.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)
Stefan Berger - Nov. 10, 2011, 1:48 a.m.
On 11/09/2011 03:29 PM, Juan Quintela wrote:
> Migration with fd uses s->mon to pass the fd.  But we only assign the
> s->mon for !detached migration.  Fix it.  Once there add a comment
> indicating that s->mon has two uses.

I had encounter the NULL pointer problem when suspending using 'virsh 
save...'. Now with your patch it unfortunately doesn't terminate, i.e., 
qemu is still running while the file has been written completely.

    Stefan
Anthony Liguori - Nov. 11, 2011, 7:45 p.m.
On 11/09/2011 02:29 PM, Juan Quintela wrote:
> Migration with fd uses s->mon to pass the fd.  But we only assign the
> s->mon for !detached migration.  Fix it.  Once there add a comment
> indicating that s->mon has two uses.
>
> Bug reported by:  Wen Congyang<wency@cn.fujitsu.com>
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
> CC:  Wen Congyang<wency@cn.fujitsu.com>

Applied.  Thanks.

Regards,

Anthony Liguori

> ---
>   migration.c |   12 ++++++++++--
>   1 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 4b17566..41c3c24 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -155,7 +155,6 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>
>   static void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
>   {
> -    s->mon = mon;
>       if (monitor_suspend(mon) == 0) {
>           DPRINTF("suspending monitor\n");
>       } else {
> @@ -383,7 +382,12 @@ static MigrationState *migrate_init(Monitor *mon, int detach, int blk, int inc)
>       s->bandwidth_limit = bandwidth_limit;
>       s->blk = blk;
>       s->shared = inc;
> -    s->mon = NULL;
> +
> +    /* s->mon is used for two things:
> +       - pass fd in fd migration
> +       - suspend/resume monitor for not detached migration
> +    */
> +    s->mon = mon;
>       s->bandwidth_limit = bandwidth_limit;
>       s->state = MIG_STATE_SETUP;
>
> @@ -435,6 +439,10 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>           return ret;
>       }
>
> +    if (detach) {
> +        s->mon = NULL;
> +    }
> +
>       notifier_list_notify(&migration_state_notifiers, s);
>       return 0;
>   }

Patch

diff --git a/migration.c b/migration.c
index 4b17566..41c3c24 100644
--- a/migration.c
+++ b/migration.c
@@ -155,7 +155,6 @@  MigrationInfo *qmp_query_migrate(Error **errp)

 static void migrate_fd_monitor_suspend(MigrationState *s, Monitor *mon)
 {
-    s->mon = mon;
     if (monitor_suspend(mon) == 0) {
         DPRINTF("suspending monitor\n");
     } else {
@@ -383,7 +382,12 @@  static MigrationState *migrate_init(Monitor *mon, int detach, int blk, int inc)
     s->bandwidth_limit = bandwidth_limit;
     s->blk = blk;
     s->shared = inc;
-    s->mon = NULL;
+
+    /* s->mon is used for two things:
+       - pass fd in fd migration
+       - suspend/resume monitor for not detached migration
+    */
+    s->mon = mon;
     s->bandwidth_limit = bandwidth_limit;
     s->state = MIG_STATE_SETUP;

@@ -435,6 +439,10 @@  int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
         return ret;
     }

+    if (detach) {
+        s->mon = NULL;
+    }
+
     notifier_list_notify(&migration_state_notifiers, s);
     return 0;
 }