[Qemu-devel] [PATCH] honor -S on incoming migration

[Qemu-devel] [PATCH] honor -S on incoming migration

[Paolo Bonzini]

-S is not honored by qemu on incoming migration.  If a domain is migrated
while paused, thus, it will start running on the remote machine; this
is wrong.

Given the trivial patch to fix this, it looks more like a thinko
than anything else, probably dating back to the qemu-kvm merge.
The interesting part is that the -S mechanism was in fact *used* when
migrating (setting autostart = 0) and the incoming migration code was
starting the VM at the end of the migration.

Since I was removing the vm_start from there, I also corrected a related
imprecision.  The code was doing a vm_stop "just in case", but we can
be sure that the VM is not running---the vm_start call in vl.c has not
been reached yet, and we rely on this now that the vm_start is removed
in migrate-*.c.  So it's better to assert (!vm_running) instead.
---
 migration-exec.c |    3 +--
 migration-tcp.c  |    4 +---
 vl.c             |    4 +---
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/migration-exec.c b/migration-exec.c
index 0dd5aff..5a111a0 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -109,7 +109,7 @@ static void exec_accept_incoming_migration(void *opaque)
     QEMUFile *f = opaque;
     int ret;
 
-    vm_stop(0); /* just in case */
+    assert(!vm_running); /* just in case */
     ret = qemu_loadvm_state(f);
     if (ret < 0) {
         fprintf(stderr, "load of migration failed\n");
@@ -119,7 +119,6 @@ static void exec_accept_incoming_migration(void *opaque)
     dprintf("successfully loaded vm state\n");
     /* we've successfully migrated, close the fd */
     qemu_set_fd_handler2(qemu_popen_fd(f), NULL, NULL, NULL, NULL);
-    vm_start();
 
 err:
     qemu_fclose(f);
diff --git a/migration-tcp.c b/migration-tcp.c
index 1f4358e..53897c3 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -155,7 +155,7 @@ static void tcp_accept_incoming_migration(void *opaque)
         goto out;
     }
 
-    vm_stop(0); /* just in case */
+    assert(!vm_running); /* just in case */
     ret = qemu_loadvm_state(f);
     if (ret < 0) {
         fprintf(stderr, "load of migration failed\n");
@@ -168,8 +168,6 @@ static void tcp_accept_incoming_migration(void *opaque)
     qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
     close(s);
 
-    vm_start();
-
 out_fopen:
     qemu_fclose(f);
 out:
diff --git a/vl.c b/vl.c
index 3242c23..845d185 100644
--- a/vl.c
+++ b/vl.c
@@ -6195,10 +6195,8 @@ int main(int argc, char **argv, char **envp)
     if (loadvm)
         do_loadvm(cur_mon, loadvm);
 
-    if (incoming) {
-        autostart = 0; /* fixme how to deal with -daemonize */
+    if (incoming)
         qemu_start_incoming_migration(incoming);
-    }
 
     if (autostart)
         vm_start();
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH] honor -S on incoming migration

[Anthony Liguori]

Paolo Bonzini wrote:
> -S is not honored by qemu on incoming migration.  If a domain is migrated
> while paused, thus, it will start running on the remote machine; this
> is wrong.
>
> Given the trivial patch to fix this, it looks more like a thinko
> than anything else, probably dating back to the qemu-kvm merge.
> The interesting part is that the -S mechanism was in fact *used* when
> migrating (setting autostart = 0) and the incoming migration code was
> starting the VM at the end of the migration.
>
> Since I was removing the vm_start from there, I also corrected a related
> imprecision.  The code was doing a vm_stop "just in case", but we can
> be sure that the VM is not running---the vm_start call in vl.c has not
> been reached yet, and we rely on this now that the vm_start is removed
> in migrate-*.c.  So it's better to assert (!vm_running) instead.
>   

Needs a Signed-off-by.

> ---
>  migration-exec.c |    3 +--
>  migration-tcp.c  |    4 +---
>  vl.c             |    4 +---
>  3 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/migration-exec.c b/migration-exec.c
> index 0dd5aff..5a111a0 100644
> --- a/migration-exec.c
> +++ b/migration-exec.c
> @@ -109,7 +109,7 @@ static void exec_accept_incoming_migration(void *opaque)
>      QEMUFile *f = opaque;
>      int ret;
>  
> -    vm_stop(0); /* just in case */
> +    assert(!vm_running); /* just in case */
>   

I'd suggest removing the asserts but other than that, the patch looks good.

Regards,

Anthony Liguori

[Qemu-devel] [PATCH v2] honor -S on incoming migration

[Paolo Bonzini]

-S is not honored by qemu on incoming migration.  If a domain is migrated
while paused, thus, it will start running on the remote machine; this
is wrong.

Given the trivial patch to fix this, it looks more like a thinko
than anything else, probably dating back to the qemu-kvm merge.
The interesting part is that the -S mechanism was in fact *used* when
migrating (setting autostart = 0) and the incoming migration code was
starting the VM at the end of the migration.

Since I was removing the vm_start from there, I also corrected a related
imprecision.  The code was doing a vm_stop "just in case", but we can
be sure that the VM is not running---the vm_start call in vl.c has not
been reached yet.  So the vm_stop is removed together with the vm_start.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	> I'd suggest removing the asserts

	No objection from me.  I adjusted the log message for this.

 migration-exec.c |    2 --
 migration-tcp.c  |    3 ---
 vl.c             |    4 +---
 3 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/migration-exec.c b/migration-exec.c
index 0dd5aff..e472979 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -109,7 +109,6 @@ static void exec_accept_incoming_migration(void *opaque)
     QEMUFile *f = opaque;
     int ret;
 
-    vm_stop(0); /* just in case */
     ret = qemu_loadvm_state(f);
     if (ret < 0) {
         fprintf(stderr, "load of migration failed\n");
@@ -119,7 +118,6 @@ static void exec_accept_incoming_migration(void *opaque)
     dprintf("successfully loaded vm state\n");
     /* we've successfully migrated, close the fd */
     qemu_set_fd_handler2(qemu_popen_fd(f), NULL, NULL, NULL, NULL);
-    vm_start();
 
 err:
     qemu_fclose(f);
diff --git a/migration-tcp.c b/migration-tcp.c
index 1f4358e..7a87a1e 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -155,7 +155,6 @@ static void tcp_accept_incoming_migration(void *opaque)
         goto out;
     }
 
-    vm_stop(0); /* just in case */
     ret = qemu_loadvm_state(f);
     if (ret < 0) {
         fprintf(stderr, "load of migration failed\n");
@@ -168,8 +167,6 @@ static void tcp_accept_incoming_migration(void *opaque)
     qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
     close(s);
 
-    vm_start();
-
 out_fopen:
     qemu_fclose(f);
 out:
diff --git a/vl.c b/vl.c
index 3242c23..845d185 100644
--- a/vl.c
+++ b/vl.c
@@ -6195,10 +6195,8 @@ int main(int argc, char **argv, char **envp)
     if (loadvm)
         do_loadvm(cur_mon, loadvm);
 
-    if (incoming) {
-        autostart = 0; /* fixme how to deal with -daemonize */
+    if (incoming)
         qemu_start_incoming_migration(incoming);
-    }
 
     if (autostart)
         vm_start();
-- 
1.6.2.5