[Qemu-devel] [PATCH] runstate: add more valid transitions

[Qemu-devel] [PATCH] runstate: add more valid transitions

[Paolo Bonzini]

This patch adds more valid transitions to the table, and avoids
that the VM remains stuck in RSTATE_SAVEVM state when savevm is
done on a paused virtual machine.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 savevm.c |   11 ++++++++---
 vl.c     |    5 +++++
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/savevm.c b/savevm.c
index 46f2447..ba5beb0 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1599,7 +1599,7 @@ void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f)
 
 static int qemu_savevm_state(Monitor *mon, QEMUFile *f)
 {
-    int saved_vm_running;
+    int saved_vm_state;
     int ret;
 
     saved_vm_running = runstate_is_running();
@@ -1626,8 +1626,13 @@ out:
     if (qemu_file_has_error(f))
         ret = -EIO;
 
-    if (!ret && saved_vm_running)
-        vm_start();
+    if (!ret) {
+        if (saved_vm_running) {
+            vm_start();
+        } else {
+            runstate_set(RSTATE_PAUSED);
+        }
+    }
 
     return ret;
 }
diff --git a/vl.c b/vl.c
index bd4a5ce..165712e 100644
--- a/vl.c
+++ b/vl.c
@@ -346,8 +346,12 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RSTATE_IO_ERROR, RSTATE_RUNNING },
 
     { RSTATE_PAUSED, RSTATE_RUNNING },
+    { RSTATE_PAUSED, RSTATE_PRE_MIGRATE },
+    { RSTATE_PAUSED, RSTATE_SAVEVM },
 
     { RSTATE_POST_MIGRATE, RSTATE_RUNNING },
+    { RSTATE_POST_MIGRATE, RSTATE_PRE_MIGRATE },
+    { RSTATE_POST_MIGRATE, RSTATE_SAVEVM },
 
     { RSTATE_PRE_LAUNCH, RSTATE_RUNNING },
     { RSTATE_PRE_LAUNCH, RSTATE_POST_MIGRATE },
@@ -368,6 +372,7 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RSTATE_RUNNING, RSTATE_WATCHDOG },
 
     { RSTATE_SAVEVM, RSTATE_RUNNING },
+    { RSTATE_SAVEVM, RSTATE_PAUSED },
 
     { RSTATE_SHUTDOWN, RSTATE_PAUSED },
 
-- 
1.7.6

[Qemu-devel] [PATCH v2] runstate: add more valid transitions

[Paolo Bonzini]

This patch adds more valid transitions to the table, and avoids
that the VM remains stuck in RSTATE_SAVEVM state when savevm is
done on a paused virtual machine.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 savevm.c |    9 +++++++--
 vl.c     |    5 +++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/savevm.c b/savevm.c
index 46f2447..ff37968 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1626,8 +1626,13 @@ out:
     if (qemu_file_has_error(f))
         ret = -EIO;
 
-    if (!ret && saved_vm_running)
-        vm_start();
+    if (!ret) {
+        if (saved_vm_running) {
+            vm_start();
+	} else {
+	    runstate_set(RSTATE_PAUSED);
+	}
+    }
 
     return ret;
 }
diff --git a/vl.c b/vl.c
index bd4a5ce..165712e 100644
--- a/vl.c
+++ b/vl.c
@@ -346,8 +346,12 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RSTATE_IO_ERROR, RSTATE_RUNNING },
 
     { RSTATE_PAUSED, RSTATE_RUNNING },
+    { RSTATE_PAUSED, RSTATE_PRE_MIGRATE },
+    { RSTATE_PAUSED, RSTATE_SAVEVM },
 
     { RSTATE_POST_MIGRATE, RSTATE_RUNNING },
+    { RSTATE_POST_MIGRATE, RSTATE_PRE_MIGRATE },
+    { RSTATE_POST_MIGRATE, RSTATE_SAVEVM },
 
     { RSTATE_PRE_LAUNCH, RSTATE_RUNNING },
     { RSTATE_PRE_LAUNCH, RSTATE_POST_MIGRATE },
@@ -368,6 +372,7 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RSTATE_RUNNING, RSTATE_WATCHDOG },
 
     { RSTATE_SAVEVM, RSTATE_RUNNING },
+    { RSTATE_SAVEVM, RSTATE_PAUSED },
 
     { RSTATE_SHUTDOWN, RSTATE_PAUSED },
 
-- 
1.7.6

Re: [Qemu-devel] [PATCH v2] runstate: add more valid transitions

[Luiz Capitulino]

On Tue,  4 Oct 2011 15:15:40 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> This patch adds more valid transitions to the table, and avoids
> that the VM remains stuck in RSTATE_SAVEVM state when savevm is
> done on a paused virtual machine.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Looks good, but it needs to be rebased against latest master.

> ---
>  savevm.c |    9 +++++++--
>  vl.c     |    5 +++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/savevm.c b/savevm.c
> index 46f2447..ff37968 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1626,8 +1626,13 @@ out:
>      if (qemu_file_has_error(f))
>          ret = -EIO;
>  
> -    if (!ret && saved_vm_running)
> -        vm_start();
> +    if (!ret) {
> +        if (saved_vm_running) {
> +            vm_start();
> +	} else {
> +	    runstate_set(RSTATE_PAUSED);
> +	}
> +    }
>  
>      return ret;
>  }
> diff --git a/vl.c b/vl.c
> index bd4a5ce..165712e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -346,8 +346,12 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RSTATE_IO_ERROR, RSTATE_RUNNING },
>  
>      { RSTATE_PAUSED, RSTATE_RUNNING },
> +    { RSTATE_PAUSED, RSTATE_PRE_MIGRATE },
> +    { RSTATE_PAUSED, RSTATE_SAVEVM },
>  
>      { RSTATE_POST_MIGRATE, RSTATE_RUNNING },
> +    { RSTATE_POST_MIGRATE, RSTATE_PRE_MIGRATE },
> +    { RSTATE_POST_MIGRATE, RSTATE_SAVEVM },
>  
>      { RSTATE_PRE_LAUNCH, RSTATE_RUNNING },
>      { RSTATE_PRE_LAUNCH, RSTATE_POST_MIGRATE },
> @@ -368,6 +372,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RSTATE_RUNNING, RSTATE_WATCHDOG },
>  
>      { RSTATE_SAVEVM, RSTATE_RUNNING },
> +    { RSTATE_SAVEVM, RSTATE_PAUSED },
>  
>      { RSTATE_SHUTDOWN, RSTATE_PAUSED },
>  

[Qemu-devel] [PATCH v2] runstate: add more valid transitions

[Paolo Bonzini]

This patch adds more valid transitions to the table, and avoids
that the VM remains stuck in RSTATE_SAVEVM state when savevm is
done on a paused virtual machine.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 savevm.c |    9 +++++++--
 vl.c     |    5 +++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/savevm.c b/savevm.c
index bf4d0e7..ac45db8 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1626,8 +1626,13 @@ out:
     if (qemu_file_has_error(f))
         ret = -EIO;
 
-    if (!ret && saved_vm_running)
-        vm_start();
+    if (!ret) {
+        if (saved_vm_running) {
+            vm_start();
+	} else {
+	    runstate_set(RSTATE_PAUSED);
+	}
+    }
 
     return ret;
 }
diff --git a/vl.c b/vl.c
index dbf7778..6614aac 100644
--- a/vl.c
+++ b/vl.c
@@ -343,8 +343,12 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_IO_ERROR, RUN_STATE_RUNNING },
 
     { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
+    { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
+    { RUN_STATE_PAUSED, RUN_STATE_SAVE_VM },
 
     { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING },
+    { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE },
+    { RUN_STATE_POSTMIGRATE, RUN_STATE_SAVE_VM },
 
     { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING },
     { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
@@ -366,6 +370,7 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
 
     { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
+    { RUN_STATE_SAVE_VM, RUN_STATE_PAUSED },
 
     { RUN_STATE_SHUTDOWN, RUN_STATE_PAUSED },
 
-- 
1.7.6

Re: [Qemu-devel] [PATCH v2] runstate: add more valid transitions

[Luiz Capitulino]

On Tue, 11 Oct 2011 09:22:42 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> This patch adds more valid transitions to the table, and avoids
> that the VM remains stuck in RSTATE_SAVEVM state when savevm is
> done on a paused virtual machine.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Applied to the qmp branch, thanks.

> ---
>  savevm.c |    9 +++++++--
>  vl.c     |    5 +++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/savevm.c b/savevm.c
> index bf4d0e7..ac45db8 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1626,8 +1626,13 @@ out:
>      if (qemu_file_has_error(f))
>          ret = -EIO;
>  
> -    if (!ret && saved_vm_running)
> -        vm_start();
> +    if (!ret) {
> +        if (saved_vm_running) {
> +            vm_start();
> +	} else {
> +	    runstate_set(RSTATE_PAUSED);
> +	}
> +    }
>  
>      return ret;
>  }
> diff --git a/vl.c b/vl.c
> index dbf7778..6614aac 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -343,8 +343,12 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_IO_ERROR, RUN_STATE_RUNNING },
>  
>      { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
> +    { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_PAUSED, RUN_STATE_SAVE_VM },
>  
>      { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING },
> +    { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_POSTMIGRATE, RUN_STATE_SAVE_VM },
>  
>      { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING },
>      { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
> @@ -366,6 +370,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
>  
>      { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
> +    { RUN_STATE_SAVE_VM, RUN_STATE_PAUSED },
>  
>      { RUN_STATE_SHUTDOWN, RUN_STATE_PAUSED },
>  

Re: [Qemu-devel] [PATCH v2] runstate: add more valid transitions

[Luiz Capitulino]

On Tue, 11 Oct 2011 14:52:48 -0300
Luiz Capitulino <lcapitulino@redhat.com> wrote:

> On Tue, 11 Oct 2011 09:22:42 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> > This patch adds more valid transitions to the table, and avoids
> > that the VM remains stuck in RSTATE_SAVEVM state when savevm is
> > done on a paused virtual machine.
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Applied to the qmp branch, thanks.

I'm going to take my word back on this one, I've found the real cause of the
problem. Will post the patch right now.

Re: [Qemu-devel] [PATCH v2] runstate: add more valid transitions

[Paolo Bonzini]

On 10/13/2011 10:26 PM, Luiz Capitulino wrote:
> I'm going to take my word back on this one, I've found the real cause of the
> problem. Will post the patch right now.
>

Are you keeping the vl.c hunks though?

Paolo

Re: [Qemu-devel] [PATCH v2] runstate: add more valid transitions

[Luiz Capitulino]

On Fri, 14 Oct 2011 08:56:57 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 10/13/2011 10:26 PM, Luiz Capitulino wrote:
> > I'm going to take my word back on this one, I've found the real cause of the
> > problem. Will post the patch right now.
> >
> 
> Are you keeping the vl.c hunks though?

I'm not, because I'm assuming that allowing a transition from 'paused' to
'postmigrate' plus this fix:

 http://lists.gnu.org/archive/html/qemu-devel/2011-10/msg01430.html

Will solve the problems we have today. If you don't think so, would you
mind to send a patch against the qmp queue?

 git://repo.or.cz/qemu/qmp-unstable.git queue/qmp

Thanks!

Re: [Qemu-devel] [PATCH v2] runstate: add more valid transitions

[Paolo Bonzini]

On 10/14/2011 03:23 PM, Luiz Capitulino wrote:
> I'm not, because I'm assuming that allowing a transition from 'paused' to
> 'postmigrate' plus this fix:
>
>   http://lists.gnu.org/archive/html/qemu-devel/2011-10/msg01430.html

I think POST_MIGRATE -> FINISH_MIGRATE should be still allowed in case 
you migrate twice.  Management would probably disallow that, but from 
the monitor you can do funny things: stop the machine, create two images 
based on the running machine's image, and migrate to both images.

Paolo

Re: [Qemu-devel] [PATCH v2] runstate: add more valid transitions

[Luiz Capitulino]

On Fri, 14 Oct 2011 15:37:29 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 10/14/2011 03:23 PM, Luiz Capitulino wrote:
> > I'm not, because I'm assuming that allowing a transition from 'paused' to
> > 'postmigrate' plus this fix:
> >
> >   http://lists.gnu.org/archive/html/qemu-devel/2011-10/msg01430.html
> 
> I think POST_MIGRATE -> FINISH_MIGRATE should be still allowed in case 
> you migrate twice.  Management would probably disallow that, but from 
> the monitor you can do funny things: stop the machine, create two images 
> based on the running machine's image, and migrate to both images.

Yes, you're right. But there's another problem there: the VM is stopped
when the migration process finishes. So, if you migrate again, there
won't be a POST_MIGRATE -> FINISH_MIGRATE transition, as vm_stop() will
just return.

I don't like the idea of changing current vm_stop() to always change the
state, so I'm adding a new function to do that:

diff --git a/cpus.c b/cpus.c
index 8978779..5f5b763 100644
--- a/cpus.c
+++ b/cpus.c
@@ -887,6 +887,17 @@ void vm_stop(RunState state)
     do_vm_stop(state);
 }
 
+/* does a state transition even if the VM is already stopped,
+   current state is forgotten forever */
+void vm_stop_force_state(RunState state)
+{
+    if (runstate_is_running()) {
+        vm_stop(state);
+    } else {
+        runstate_set(state);
+    }
+}
+
 static int tcg_cpu_exec(CPUState *env)
 {
     int ret;
diff --git a/migration.c b/migration.c
index 77a51ad..62b74a6 100644
--- a/migration.c
+++ b/migration.c
@@ -375,7 +375,7 @@ void migrate_fd_put_ready(void *opaque)
         int old_vm_running = runstate_is_running();
 
         DPRINTF("done iterating\n");
-        vm_stop(RUN_STATE_FINISH_MIGRATE);
+        vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
 
         if ((qemu_savevm_state_complete(s->mon, s->file)) < 0) {
             if (old_vm_running) {
diff --git a/sysemu.h b/sysemu.h
index a889d90..7d288f8 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -35,6 +35,7 @@ void vm_state_notify(int running, RunState state);
 
 void vm_start(void);
 void vm_stop(RunState state);
+void vm_stop_force_state(RunState state);
 
 void qemu_system_reset_request(void);
 void qemu_system_shutdown_request(void);
diff --git a/vl.c b/vl.c
index 2e991fc..613204b 100644
--- a/vl.c
+++ b/vl.c
@@ -346,6 +346,7 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_PAUSED, RUN_STATE_POSTMIGRATE },
 
     { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING },
+    { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE },
 
     { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING },
     { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
-- 
1.7.7.rc3

Re: [Qemu-devel] [PATCH v2] runstate: add more valid transitions

[Paolo Bonzini]

On 10/14/2011 04:24 PM, Luiz Capitulino wrote:
> Yes, you're right. But there's another problem there: the VM is stopped
> when the migration process finishes. So, if you migrate again, there
> won't be a POST_MIGRATE ->  FINISH_MIGRATE transition, as vm_stop() will
> just return.

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

for now.  I really think for 1.1 it's better to separate the 
"paused-until-user-interaction-because-..." and 
"temporarily-paused-because-..." state, as suggested elsewhere in the 
thread.

Paolo