[Qemu-devel] [PATCH 2/5] runstate: Print state transition when invalid

[Qemu-devel] [PATCH 2/5] runstate: Print state transition when invalid

[Luiz Capitulino]

Makes it easier to debug.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 vl.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/vl.c b/vl.c
index dbf7778..6645720 100644
--- a/vl.c
+++ b/vl.c
@@ -397,7 +397,9 @@ void runstate_set(RunState new_state)
 {
     if (new_state >= RUN_STATE_MAX ||
         !runstate_valid_transitions[current_run_state][new_state]) {
-        fprintf(stderr, "invalid runstate transition\n");
+        fprintf(stderr, "ERROR: invalid runstate transition: '%s' -> '%s'\n",
+                RunState_lookup[current_run_state],
+                RunState_lookup[new_state]);
         abort();
     }
 
-- 
1.7.7.rc3

Re: [Qemu-devel] [PATCH 2/5] runstate: Print state transition when invalid

[Wen Congyang]

At 10/15/2011 01:26 AM, Luiz Capitulino Write:
> Makes it easier to debug.
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  vl.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index dbf7778..6645720 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -397,7 +397,9 @@ void runstate_set(RunState new_state)
>  {
>      if (new_state >= RUN_STATE_MAX ||
>          !runstate_valid_transitions[current_run_state][new_state]) {
> -        fprintf(stderr, "invalid runstate transition\n");
> +        fprintf(stderr, "ERROR: invalid runstate transition: '%s' -> '%s'\n",
> +                RunState_lookup[current_run_state],
> +                RunState_lookup[new_state]);

If new_state >= RUN_STATE_MAX, we can not use RunState_lookup.
I think it's better to use:
    new_state >= RUN_STATE_MAX ? "invalid state" : RunState_lookup[new_state]

Thanks
Wen Congyang

>          abort();
>      }
>  

Re: [Qemu-devel] [PATCH 2/5] runstate: Print state transition when invalid

[Luiz Capitulino]

On Wed, 19 Oct 2011 08:43:33 +0800
Wen Congyang <wency@cn.fujitsu.com> wrote:

> At 10/15/2011 01:26 AM, Luiz Capitulino Write:
> > Makes it easier to debug.
> > 
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  vl.c |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> > 
> > diff --git a/vl.c b/vl.c
> > index dbf7778..6645720 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -397,7 +397,9 @@ void runstate_set(RunState new_state)
> >  {
> >      if (new_state >= RUN_STATE_MAX ||
> >          !runstate_valid_transitions[current_run_state][new_state]) {
> > -        fprintf(stderr, "invalid runstate transition\n");
> > +        fprintf(stderr, "ERROR: invalid runstate transition: '%s' -> '%s'\n",
> > +                RunState_lookup[current_run_state],
> > +                RunState_lookup[new_state]);
> 
> If new_state >= RUN_STATE_MAX, we can not use RunState_lookup.

Good catch!

> I think it's better to use:
>     new_state >= RUN_STATE_MAX ? "invalid state" : RunState_lookup[new_state]

I prefer to do the following instead:

diff --git a/vl.c b/vl.c
index 2dce3ae..2a634a7 100644
--- a/vl.c
+++ b/vl.c
@@ -393,9 +393,12 @@ void runstate_init(void)
 /* This function will abort() on invalid state transitions */
 void runstate_set(RunState new_state)
 {
-    if (new_state >= RUN_STATE_MAX ||
-        !runstate_valid_transitions[current_run_state][new_state]) {
-        fprintf(stderr, "invalid runstate transition\n");
+    assert(new_state < RUN_STATE_MAX);
+
+    if (!runstate_valid_transitions[current_run_state][new_state]) {
+        fprintf(stderr, "ERROR: invalid runstate transition: '%s' -> '%s'\n",
+                RunState_lookup[current_run_state],
+                RunState_lookup[new_state]);
         abort();
     }
 
-- 
1.7.7.rc3


> 
> Thanks
> Wen Congyang
> 
> >          abort();
> >      }
> >  
> 

[Qemu-devel] [RESEND PULL 0/5]: QMP queue

[Luiz Capitulino]

Anthony,

I'm resending this pull request because Wen Congyang has found a bug in
one of the patches. And also to ping you to pull it :-)

The changes (since cfce6d8934243871c4dc6d0c5248b0b27a1b8d80) are available
in the following repository:

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

Luiz Capitulino (5):
      QMP: Fix blockdev-snapshot-sync doc example
      runstate: Print state transition when invalid
      runstate: Allow to transition from paused to postmigrate
      savevm: qemu_savevm_state(): Drop stop VM logic
      runstate: Allow user to migrate twice

 cpus.c          |   11 +++++++++++
 migration.c     |    2 +-
 qmp-commands.hx |    8 ++++----
 savevm.c        |    7 -------
 sysemu.h        |    1 +
 vl.c            |   17 +++++++++++++----
 6 files changed, 30 insertions(+), 16 deletions(-)

[Qemu-devel] [PATCH 1/5] QMP: Fix blockdev-snapshot-sync doc example

[Luiz Capitulino]

Fix wrong command name.

Reported-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qmp-commands.hx |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9c11e87..4328e8b 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -710,10 +710,10 @@ Arguments:
 
 Example:
 
--> { "execute": "blockdev-snapshot", "arguments": { "device": "ide-hd0",
-                                                    "snapshot-file":
-                                                    "/some/place/my-image",
-                                                    "format": "qcow2" } }
+-> { "execute": "blockdev-snapshot-sync", "arguments": { "device": "ide-hd0",
+                                                         "snapshot-file":
+                                                        "/some/place/my-image",
+                                                        "format": "qcow2" } }
 <- { "return": {} }
 
 EQMP
-- 
1.7.7.rc3

[Qemu-devel] [PATCH 2/5] runstate: Print state transition when invalid

[Luiz Capitulino]

Makes it easier to debug.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 vl.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/vl.c b/vl.c
index 2dce3ae..2a634a7 100644
--- a/vl.c
+++ b/vl.c
@@ -393,9 +393,12 @@ void runstate_init(void)
 /* This function will abort() on invalid state transitions */
 void runstate_set(RunState new_state)
 {
-    if (new_state >= RUN_STATE_MAX ||
-        !runstate_valid_transitions[current_run_state][new_state]) {
-        fprintf(stderr, "invalid runstate transition\n");
+    assert(new_state < RUN_STATE_MAX);
+
+    if (!runstate_valid_transitions[current_run_state][new_state]) {
+        fprintf(stderr, "ERROR: invalid runstate transition: '%s' -> '%s'\n",
+                RunState_lookup[current_run_state],
+                RunState_lookup[new_state]);
         abort();
     }
 
-- 
1.7.7.rc3

[Qemu-devel] [PATCH 3/5] runstate: Allow to transition from paused to postmigrate

[Luiz Capitulino]

The user may already have paused the VM before starting the
migration process. If s/he does that, then the state will be
'paused' when we finish the migration process. In that case
we want to transition from 'paused' to 'postmigrate' as the
latter is now the real reason why the VM is stopped.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 vl.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index 2a634a7..3e5fdf5 100644
--- a/vl.c
+++ b/vl.c
@@ -341,6 +341,7 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_IO_ERROR, RUN_STATE_RUNNING },
 
     { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
+    { RUN_STATE_PAUSED, RUN_STATE_POSTMIGRATE },
 
     { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING },
 
-- 
1.7.7.rc3

[Qemu-devel] [PATCH 4/5] savevm: qemu_savevm_state(): Drop stop VM logic

[Luiz Capitulino]

qemu_savevm_state() has some logic to stop the VM and to (or not to)
resume it. But this seems to be a big noop, as qemu_savevm_state()
is only called by do_savevm() when the VM is already stopped.

So, let's drop qemu_savevm_state()'s stop VM logic.

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 savevm.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/savevm.c b/savevm.c
index bf4d0e7..abb4a60 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1599,12 +1599,8 @@ void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f)
 
 static int qemu_savevm_state(Monitor *mon, QEMUFile *f)
 {
-    int saved_vm_running;
     int ret;
 
-    saved_vm_running = runstate_is_running();
-    vm_stop(RUN_STATE_SAVE_VM);
-
     if (qemu_savevm_state_blocked(mon)) {
         ret = -EINVAL;
         goto out;
@@ -1626,9 +1622,6 @@ out:
     if (qemu_file_has_error(f))
         ret = -EIO;
 
-    if (!ret && saved_vm_running)
-        vm_start();
-
     return ret;
 }
 
-- 
1.7.7.rc3

[Qemu-devel] [PATCH 5/5] runstate: Allow user to migrate twice

[Luiz Capitulino]

It should be a matter of allowing the transition POSTMIGRATE ->
FINISH_MIGRATE, but it turns out that the VM won't do the
transition the second time because it's already stopped.

So this commit also adds vm_stop_force_state() which performs
the transition even if the VM is already stopped.

While there also allow other states to migrate.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 cpus.c      |   11 +++++++++++
 migration.c |    2 +-
 sysemu.h    |    1 +
 vl.c        |    9 +++++++--
 4 files changed, 20 insertions(+), 3 deletions(-)

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 3e5fdf5..fff2b4c 100644
--- a/vl.c
+++ b/vl.c
@@ -337,17 +337,20 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_INMIGRATE, RUN_STATE_PRELAUNCH },
 
     { RUN_STATE_INTERNAL_ERROR, RUN_STATE_PAUSED },
+    { RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
 
     { RUN_STATE_IO_ERROR, RUN_STATE_RUNNING },
+    { RUN_STATE_IO_ERROR, RUN_STATE_FINISH_MIGRATE },
 
     { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
-    { RUN_STATE_PAUSED, RUN_STATE_POSTMIGRATE },
+    { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
 
     { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING },
+    { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE },
 
     { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING },
+    { RUN_STATE_PRELAUNCH, RUN_STATE_FINISH_MIGRATE },
     { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
-    { RUN_STATE_PRELAUNCH, RUN_STATE_POSTMIGRATE },
 
     { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
     { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
@@ -367,8 +370,10 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
 
     { RUN_STATE_SHUTDOWN, RUN_STATE_PAUSED },
+    { RUN_STATE_SHUTDOWN, RUN_STATE_FINISH_MIGRATE },
 
     { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
+    { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
 
     { RUN_STATE_MAX, RUN_STATE_MAX },
 };
-- 
1.7.7.rc3