Patchwork [v13,2/8] start vm after resetting it

login
register
mail settings
Submitter Hu Tao
Date Feb. 28, 2013, 12:13 p.m.
Message ID <d1f8b5689b05b9db923d86941e914d08ac4c2b7c.1362051582.git.hutao@cn.fujitsu.com>
Download mbox | patch
Permalink /patch/223962/
State New
Headers show

Comments

Hu Tao - Feb. 28, 2013, 12:13 p.m.
From: Wen Congyang <wency@cn.fujitsu.com>

The guest should run after resetting it, but it does not run if its
old state is RUN_STATE_INTERNAL_ERROR or RUN_STATE_PAUSED.

We don't set runstate to RUN_STATE_PAUSED when resetting the guest,
so the runstate will be changed from RUN_STATE_INTERNAL_ERROR or
RUN_STATE_PAUSED to RUN_STATE_RUNNING(not RUN_STATE_PAUSED).

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 include/block/block.h | 2 ++
 qmp.c                 | 2 +-
 vl.c                  | 7 ++++---
 3 files changed, 7 insertions(+), 4 deletions(-)
Jan Kiszka - Feb. 28, 2013, 1:23 p.m.
On 2013-02-28 13:13, Hu Tao wrote:
> From: Wen Congyang <wency@cn.fujitsu.com>
> 
> The guest should run after resetting it, but it does not run if its
> old state is RUN_STATE_INTERNAL_ERROR or RUN_STATE_PAUSED.
> 
> We don't set runstate to RUN_STATE_PAUSED when resetting the guest,
> so the runstate will be changed from RUN_STATE_INTERNAL_ERROR or
> RUN_STATE_PAUSED to RUN_STATE_RUNNING(not RUN_STATE_PAUSED).

I just wonder what will happen if I interrupted the guest via gdb and
then issue "monitor system_reset", also via gdb - common pattern if you
set a breakpoint on some BUG() or fault handler and then want to restart
the guest. Will the guest continue then while gdb thinks it is still
stopped? Likely, we do not differentiate between gdb-initiated stops and
the rest. Could you clarify?

Thanks,
Jan
Paolo Bonzini - March 4, 2013, 9:32 a.m.
Il 28/02/2013 13:13, Hu Tao ha scritto:
> From: Wen Congyang <wency@cn.fujitsu.com>
> 
> The guest should run after resetting it, but it does not run if its
> old state is RUN_STATE_INTERNAL_ERROR or RUN_STATE_PAUSED.
> 
> We don't set runstate to RUN_STATE_PAUSED when resetting the guest,
> so the runstate will be changed from RUN_STATE_INTERNAL_ERROR or
> RUN_STATE_PAUSED to RUN_STATE_RUNNING(not RUN_STATE_PAUSED).

This is also debatable.  In particular, restarting an INTERNAL_ERROR
guest makes it harder to inspect the state at the time of the failure.

INTERNAL_ERROR should never happen, let's separate this patch too.

Paolo
Hu Tao - March 5, 2013, 3:05 a.m.
On Thu, Feb 28, 2013 at 02:23:42PM +0100, Jan Kiszka wrote:
> On 2013-02-28 13:13, Hu Tao wrote:
> > From: Wen Congyang <wency@cn.fujitsu.com>
> > 
> > The guest should run after resetting it, but it does not run if its
> > old state is RUN_STATE_INTERNAL_ERROR or RUN_STATE_PAUSED.
> > 
> > We don't set runstate to RUN_STATE_PAUSED when resetting the guest,
> > so the runstate will be changed from RUN_STATE_INTERNAL_ERROR or
> > RUN_STATE_PAUSED to RUN_STATE_RUNNING(not RUN_STATE_PAUSED).
> 
> I just wonder what will happen if I interrupted the guest via gdb and
> then issue "monitor system_reset", also via gdb - common pattern if you
> set a breakpoint on some BUG() or fault handler and then want to restart
> the guest. Will the guest continue then while gdb thinks it is still
> stopped? Likely, we do not differentiate between gdb-initiated stops and
> the rest. Could you clarify?

Guest won't continue unless issue gdb "continue". Anyway, I'll seperate
this patch, as Paolo requested.
Hu Tao - March 5, 2013, 3:06 a.m.
On Mon, Mar 04, 2013 at 10:32:17AM +0100, Paolo Bonzini wrote:
> Il 28/02/2013 13:13, Hu Tao ha scritto:
> > From: Wen Congyang <wency@cn.fujitsu.com>
> > 
> > The guest should run after resetting it, but it does not run if its
> > old state is RUN_STATE_INTERNAL_ERROR or RUN_STATE_PAUSED.
> > 
> > We don't set runstate to RUN_STATE_PAUSED when resetting the guest,
> > so the runstate will be changed from RUN_STATE_INTERNAL_ERROR or
> > RUN_STATE_PAUSED to RUN_STATE_RUNNING(not RUN_STATE_PAUSED).
> 
> This is also debatable.  In particular, restarting an INTERNAL_ERROR
> guest makes it harder to inspect the state at the time of the failure.
> 
> INTERNAL_ERROR should never happen, let's separate this patch too.

Sure.

> 
> Paolo

Patch

diff --git a/include/block/block.h b/include/block/block.h
index 0f750d7..8effc1e 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -376,6 +376,8 @@  void bdrv_disable_copy_on_read(BlockDriverState *bs);
 void bdrv_set_in_use(BlockDriverState *bs, int in_use);
 int bdrv_in_use(BlockDriverState *bs);
 
+void iostatus_bdrv_it(void *opaque, BlockDriverState *bs);
+
 #ifdef CONFIG_LINUX_AIO
 int raw_get_aio_fd(BlockDriverState *bs);
 #else
diff --git a/qmp.c b/qmp.c
index 55b056b..5f1bed1 100644
--- a/qmp.c
+++ b/qmp.c
@@ -130,7 +130,7 @@  SpiceInfo *qmp_query_spice(Error **errp)
 };
 #endif
 
-static void iostatus_bdrv_it(void *opaque, BlockDriverState *bs)
+void iostatus_bdrv_it(void *opaque, BlockDriverState *bs)
 {
     bdrv_iostatus_reset(bs);
 }
diff --git a/vl.c b/vl.c
index 7991f2e..3d08e1a 100644
--- a/vl.c
+++ b/vl.c
@@ -537,7 +537,7 @@  static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
     { RUN_STATE_INMIGRATE, RUN_STATE_PAUSED },
 
-    { RUN_STATE_INTERNAL_ERROR, RUN_STATE_PAUSED },
+    { RUN_STATE_INTERNAL_ERROR, RUN_STATE_RUNNING },
     { RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
 
     { RUN_STATE_IO_ERROR, RUN_STATE_RUNNING },
@@ -572,7 +572,7 @@  static const RunStateTransition runstate_transitions_def[] = {
 
     { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
 
-    { RUN_STATE_SHUTDOWN, RUN_STATE_PAUSED },
+    { RUN_STATE_SHUTDOWN, RUN_STATE_RUNNING },
     { RUN_STATE_SHUTDOWN, RUN_STATE_FINISH_MIGRATE },
 
     { RUN_STATE_DEBUG, RUN_STATE_SUSPENDED },
@@ -2002,7 +2002,8 @@  static bool main_loop_should_exit(void)
         resume_all_vcpus();
         if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
             runstate_check(RUN_STATE_SHUTDOWN)) {
-            runstate_set(RUN_STATE_PAUSED);
+            bdrv_iterate(iostatus_bdrv_it, NULL);
+            vm_start();
         }
     }
     if (qemu_wakeup_requested()) {