migration: fix detached migration with fd

Submitted by Juan Quintela on Nov. 9, 2011, 8:29 p.m.

Details

Message ID 1320870541-4571-1-git-send-email-quintela@redhat.com
State New
Headers show

Commit Message

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(-)

Comments

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 hide | download patch | download mbox

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;
 }