diff mbox

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

Message ID d1f8b5689b05b9db923d86941e914d08ac4c2b7c.1362051582.git.hutao@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hu Tao Feb. 28, 2013, 12:13 p.m. UTC
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(-)

Comments

Jan Kiszka Feb. 28, 2013, 1:23 p.m. UTC | #1
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. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hu Tao March 5, 2013, 3:05 a.m. UTC | #3
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.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hu Tao March 5, 2013, 3:06 a.m. UTC | #4
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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

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