All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kvm: notify host when guest panicked
@ 2012-05-21  6:46 ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-05-21  6:46 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

We can know the guest is panicked when the guest runs on xen.
But we do not have such feature on kvm.

Another purpose of this feature is: management app(for example:
libvirt) can do auto dump when the guest is panicked. If management
app does not do auto dump, the guest's user can do dump by hand if
he sees the guest is panicked.

We have three solutions to implement this feature:
1. use vmcall
2. use I/O port
3. use virtio-serial.

We have decided to avoid touching hypervisor. The reason why I choose
choose the I/O port is:
1. it is easier to implememt
2. it does not depend any virtual device
3. it can work when startint the kernel

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 arch/ia64/include/asm/kvm_para.h    |    5 +++++
 arch/powerpc/include/asm/kvm_para.h |    5 +++++
 arch/s390/include/asm/kvm_para.h    |    5 +++++
 arch/x86/include/asm/kvm_para.h     |    7 +++++++
 arch/x86/kernel/kvm.c               |   14 ++++++++++++++
 include/linux/kvm_para.h            |   15 +++++++++++++++
 6 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/arch/ia64/include/asm/kvm_para.h b/arch/ia64/include/asm/kvm_para.h
index 1588aee..a890096 100644
--- a/arch/ia64/include/asm/kvm_para.h
+++ b/arch/ia64/include/asm/kvm_para.h
@@ -26,6 +26,11 @@ static inline unsigned int kvm_arch_para_features(void)
 	return 0;
 }
 
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	return 0;
+}
+
 #endif
 
 #endif
diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h
index 7b754e7..b5f7c35 100644
--- a/arch/powerpc/include/asm/kvm_para.h
+++ b/arch/powerpc/include/asm/kvm_para.h
@@ -206,6 +206,11 @@ static inline unsigned int kvm_arch_para_features(void)
 	return r;
 }
 
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	return 0;
+}
+
 #endif /* __KERNEL__ */
 
 #endif /* __POWERPC_KVM_PARA_H__ */
diff --git a/arch/s390/include/asm/kvm_para.h b/arch/s390/include/asm/kvm_para.h
index 6964db2..21a8d18 100644
--- a/arch/s390/include/asm/kvm_para.h
+++ b/arch/s390/include/asm/kvm_para.h
@@ -149,6 +149,11 @@ static inline unsigned int kvm_arch_para_features(void)
 	return 0;
 }
 
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	return 0;
+}
+
 #endif
 
 #endif /* __S390_KVM_PARA_H */
diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index 734c376..f66b16e 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -208,6 +208,13 @@ static inline void kvm_disable_steal_time(void)
 }
 #endif
 
+#ifdef KVM_PV_PORT
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	return inl(KVM_PV_PORT);
+}
+#endif
+
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_X86_KVM_PARA_H */
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index b8ba6e4..adfde45 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -335,6 +335,17 @@ static struct notifier_block kvm_pv_reboot_nb = {
 	.notifier_call = kvm_pv_reboot_notify,
 };
 
+static int
+kvm_pv_panic_notify(struct notifier_block *nb, unsigned long code, void *unused)
+{
+	outl(KVM_PV_PANICKED, KVM_PV_PORT);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block kvm_pv_panic_nb = {
+	.notifier_call = kvm_pv_panic_notify,
+};
+
 static u64 kvm_steal_clock(int cpu)
 {
 	u64 steal;
@@ -421,6 +432,9 @@ void __init kvm_guest_init(void)
 
 	paravirt_ops_setup();
 	register_reboot_notifier(&kvm_pv_reboot_nb);
+	if (kvm_pv_has_feature(KVM_PV_FEATURE_PANICKED))
+		atomic_notifier_chain_register(&panic_notifier_list,
+			&kvm_pv_panic_nb);
 	for (i = 0; i < KVM_TASK_SLEEP_HASHSIZE; i++)
 		spin_lock_init(&async_pf_sleepers[i].lock);
 	if (kvm_para_has_feature(KVM_FEATURE_ASYNC_PF))
diff --git a/include/linux/kvm_para.h b/include/linux/kvm_para.h
index ff476dd..849b0d5 100644
--- a/include/linux/kvm_para.h
+++ b/include/linux/kvm_para.h
@@ -20,6 +20,14 @@
 #define KVM_HC_FEATURES			3
 #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
 
+#define KVM_PV_PORT	(0x505UL)
+
+/* The bit of the value read from KVM_PV_PORT */
+#define KVM_PV_FEATURE_PANICKED	0
+
+/* The value writen to KVM_PV_PORT */
+#define KVM_PV_PANICKED		1
+
 /*
  * hypercalls use architecture specific
  */
@@ -33,5 +41,12 @@ static inline int kvm_para_has_feature(unsigned int feature)
 		return 1;
 	return 0;
 }
+
+static inline int kvm_pv_has_feature(unsigned int feature)
+{
+	if (kvm_arch_pv_features() & (1UL << feature))
+		return 1;
+	return 0;
+}
 #endif /* __KERNEL__ */
 #endif /* __LINUX_KVM_PARA_H */
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [Qemu-devel] [PATCH] kvm: notify host when guest panicked
@ 2012-05-21  6:46 ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-05-21  6:46 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

We can know the guest is panicked when the guest runs on xen.
But we do not have such feature on kvm.

Another purpose of this feature is: management app(for example:
libvirt) can do auto dump when the guest is panicked. If management
app does not do auto dump, the guest's user can do dump by hand if
he sees the guest is panicked.

We have three solutions to implement this feature:
1. use vmcall
2. use I/O port
3. use virtio-serial.

We have decided to avoid touching hypervisor. The reason why I choose
choose the I/O port is:
1. it is easier to implememt
2. it does not depend any virtual device
3. it can work when startint the kernel

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 arch/ia64/include/asm/kvm_para.h    |    5 +++++
 arch/powerpc/include/asm/kvm_para.h |    5 +++++
 arch/s390/include/asm/kvm_para.h    |    5 +++++
 arch/x86/include/asm/kvm_para.h     |    7 +++++++
 arch/x86/kernel/kvm.c               |   14 ++++++++++++++
 include/linux/kvm_para.h            |   15 +++++++++++++++
 6 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/arch/ia64/include/asm/kvm_para.h b/arch/ia64/include/asm/kvm_para.h
index 1588aee..a890096 100644
--- a/arch/ia64/include/asm/kvm_para.h
+++ b/arch/ia64/include/asm/kvm_para.h
@@ -26,6 +26,11 @@ static inline unsigned int kvm_arch_para_features(void)
 	return 0;
 }
 
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	return 0;
+}
+
 #endif
 
 #endif
diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h
index 7b754e7..b5f7c35 100644
--- a/arch/powerpc/include/asm/kvm_para.h
+++ b/arch/powerpc/include/asm/kvm_para.h
@@ -206,6 +206,11 @@ static inline unsigned int kvm_arch_para_features(void)
 	return r;
 }
 
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	return 0;
+}
+
 #endif /* __KERNEL__ */
 
 #endif /* __POWERPC_KVM_PARA_H__ */
diff --git a/arch/s390/include/asm/kvm_para.h b/arch/s390/include/asm/kvm_para.h
index 6964db2..21a8d18 100644
--- a/arch/s390/include/asm/kvm_para.h
+++ b/arch/s390/include/asm/kvm_para.h
@@ -149,6 +149,11 @@ static inline unsigned int kvm_arch_para_features(void)
 	return 0;
 }
 
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	return 0;
+}
+
 #endif
 
 #endif /* __S390_KVM_PARA_H */
diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index 734c376..f66b16e 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -208,6 +208,13 @@ static inline void kvm_disable_steal_time(void)
 }
 #endif
 
+#ifdef KVM_PV_PORT
+static inline unsigned int kvm_arch_pv_features(void)
+{
+	return inl(KVM_PV_PORT);
+}
+#endif
+
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_X86_KVM_PARA_H */
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index b8ba6e4..adfde45 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -335,6 +335,17 @@ static struct notifier_block kvm_pv_reboot_nb = {
 	.notifier_call = kvm_pv_reboot_notify,
 };
 
+static int
+kvm_pv_panic_notify(struct notifier_block *nb, unsigned long code, void *unused)
+{
+	outl(KVM_PV_PANICKED, KVM_PV_PORT);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block kvm_pv_panic_nb = {
+	.notifier_call = kvm_pv_panic_notify,
+};
+
 static u64 kvm_steal_clock(int cpu)
 {
 	u64 steal;
@@ -421,6 +432,9 @@ void __init kvm_guest_init(void)
 
 	paravirt_ops_setup();
 	register_reboot_notifier(&kvm_pv_reboot_nb);
+	if (kvm_pv_has_feature(KVM_PV_FEATURE_PANICKED))
+		atomic_notifier_chain_register(&panic_notifier_list,
+			&kvm_pv_panic_nb);
 	for (i = 0; i < KVM_TASK_SLEEP_HASHSIZE; i++)
 		spin_lock_init(&async_pf_sleepers[i].lock);
 	if (kvm_para_has_feature(KVM_FEATURE_ASYNC_PF))
diff --git a/include/linux/kvm_para.h b/include/linux/kvm_para.h
index ff476dd..849b0d5 100644
--- a/include/linux/kvm_para.h
+++ b/include/linux/kvm_para.h
@@ -20,6 +20,14 @@
 #define KVM_HC_FEATURES			3
 #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
 
+#define KVM_PV_PORT	(0x505UL)
+
+/* The bit of the value read from KVM_PV_PORT */
+#define KVM_PV_FEATURE_PANICKED	0
+
+/* The value writen to KVM_PV_PORT */
+#define KVM_PV_PANICKED		1
+
 /*
  * hypercalls use architecture specific
  */
@@ -33,5 +41,12 @@ static inline int kvm_para_has_feature(unsigned int feature)
 		return 1;
 	return 0;
 }
+
+static inline int kvm_pv_has_feature(unsigned int feature)
+{
+	if (kvm_arch_pv_features() & (1UL << feature))
+		return 1;
+	return 0;
+}
 #endif /* __KERNEL__ */
 #endif /* __LINUX_KVM_PARA_H */
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [PATCH 1/3] start vm after reseting it
  2012-05-21  6:46 ` [Qemu-devel] " Wen Congyang
@ 2012-05-21  6:49   ` Wen Congyang
  -1 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-05-21  6:49 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

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

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 vl.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index 23ab3a3..7f5fed8 100644
--- a/vl.c
+++ b/vl.c
@@ -1539,6 +1539,7 @@ static bool main_loop_should_exit(void)
         if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
             runstate_check(RUN_STATE_SHUTDOWN)) {
             runstate_set(RUN_STATE_PAUSED);
+            vm_start();
         }
     }
     if (qemu_powerdown_requested()) {
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [Qemu-devel] [PATCH 1/3] start vm after reseting it
@ 2012-05-21  6:49   ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-05-21  6:49 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

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

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 vl.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index 23ab3a3..7f5fed8 100644
--- a/vl.c
+++ b/vl.c
@@ -1539,6 +1539,7 @@ static bool main_loop_should_exit(void)
         if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
             runstate_check(RUN_STATE_SHUTDOWN)) {
             runstate_set(RUN_STATE_PAUSED);
+            vm_start();
         }
     }
     if (qemu_powerdown_requested()) {
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [PATCH 2/3] update linux headers
  2012-05-21  6:46 ` [Qemu-devel] " Wen Congyang
@ 2012-05-21  6:50   ` Wen Congyang
  -1 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-05-21  6:50 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 linux-headers/linux/kvm_para.h |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/linux-headers/linux/kvm_para.h b/linux-headers/linux/kvm_para.h
index 7bdcf93..5618758 100644
--- a/linux-headers/linux/kvm_para.h
+++ b/linux-headers/linux/kvm_para.h
@@ -20,6 +20,14 @@
 #define KVM_HC_FEATURES			3
 #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
 
+#define KVM_PV_PORT	(0x505UL)
+
+/* The bit of the value read from KVM_PV_PORT */
+#define KVM_PV_FEATURE_PANICKED	0
+
+/* The value writen to KVM_PV_PORT */
+#define KVM_PV_PANICKED		1
+
 /*
  * hypercalls use architecture specific
  */
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [Qemu-devel] [PATCH 2/3] update linux headers
@ 2012-05-21  6:50   ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-05-21  6:50 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 linux-headers/linux/kvm_para.h |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/linux-headers/linux/kvm_para.h b/linux-headers/linux/kvm_para.h
index 7bdcf93..5618758 100644
--- a/linux-headers/linux/kvm_para.h
+++ b/linux-headers/linux/kvm_para.h
@@ -20,6 +20,14 @@
 #define KVM_HC_FEATURES			3
 #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
 
+#define KVM_PV_PORT	(0x505UL)
+
+/* The bit of the value read from KVM_PV_PORT */
+#define KVM_PV_FEATURE_PANICKED	0
+
+/* The value writen to KVM_PV_PORT */
+#define KVM_PV_PANICKED		1
+
 /*
  * hypercalls use architecture specific
  */
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [PATCH 3/3] deal with guest panicked event
  2012-05-21  6:46 ` [Qemu-devel] " Wen Congyang
@ 2012-05-21  6:50   ` Wen Congyang
  -1 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-05-21  6:50 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

When the guest is panicked, it will write 0x1 to the port 0x505. So if
qemu reads 0x1 from this port, we can do the folloing three things
according to the parameter -onpanic:
1. emit QEVENT_GUEST_PANICKED only
2. emit QEVENT_GUEST_PANICKED and pause VM
3. emit QEVENT_GUEST_PANICKED and quit VM

Note: if we emit QEVENT_GUEST_PANICKED only, and the management
application does not receive this event(the management may not
run when the event is emitted), the management won't know the
guest is panicked.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 kvm-all.c        |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 kvm-stub.c       |    9 ++++++
 kvm.h            |    3 ++
 monitor.c        |    3 ++
 monitor.h        |    1 +
 qapi-schema.json |    6 +++-
 qemu-options.hx  |   14 +++++++++
 qmp.c            |    3 +-
 vl.c             |   17 ++++++++++-
 9 files changed, 137 insertions(+), 3 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 9b73ccf..b5b0531 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -19,6 +19,7 @@
 #include <stdarg.h>
 
 #include <linux/kvm.h>
+#include <linux/kvm_para.h>
 
 #include "qemu-common.h"
 #include "qemu-barrier.h"
@@ -29,6 +30,8 @@
 #include "bswap.h"
 #include "memory.h"
 #include "exec-memory.h"
+#include "iorange.h"
+#include "qemu-objects.h"
 
 /* This check must be after config-host.h is included */
 #ifdef CONFIG_EVENTFD
@@ -1707,3 +1710,84 @@ int kvm_on_sigbus(int code, void *addr)
 {
     return kvm_arch_on_sigbus(code, addr);
 }
+
+/* Possible values for action parameter. */
+#define PANICKED_REPORT 1   /* emit QEVENT_GUEST_PANICKED only */
+#define PANICKED_PAUSE  2   /* emit QEVENT_GUEST_PANICKED and pause VM */
+#define PANICKED_QUIT   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
+
+static int panicked_action = PANICKED_REPORT;
+
+static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
+                             uint64_t *data)
+{
+    *data = (1 << KVM_PV_FEATURE_PANICKED);
+}
+
+static void panicked_mon_event(const char *action)
+{
+    QObject *data;
+
+    data = qobject_from_jsonf("{ 'action': %s }", action);
+    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
+    qobject_decref(data);
+}
+
+static void panicked_perform_action(void)
+{
+    switch(panicked_action) {
+    case PANICKED_REPORT:
+        panicked_mon_event("report");
+        break;
+
+    case PANICKED_PAUSE:
+        panicked_mon_event("pause");
+        vm_stop(RUN_STATE_GUEST_PANICKED);
+        break;
+
+    case PANICKED_QUIT:
+        panicked_mon_event("quit");
+        exit(0);
+        break;
+    }
+}
+
+static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
+                              uint64_t data)
+{
+    if (data == KVM_PV_PANICKED)
+        panicked_perform_action();
+}
+
+static void kvm_pv_port_destructor(IORange *iorange)
+{
+    g_free(iorange);
+}
+
+static IORangeOps pv_io_range_ops = {
+    .read = kvm_pv_port_read,
+    .write = kvm_pv_port_write,
+    .destructor = kvm_pv_port_destructor,
+};
+
+void kvm_pv_port_init(void)
+{
+    IORange *pv_io_range = g_malloc(sizeof(IORange));
+
+    iorange_init(pv_io_range, &pv_io_range_ops, 0x505, 1);
+    ioport_register(pv_io_range);
+}
+
+int select_panicked_action(const char *p)
+{
+    if (strcasecmp(p, "report") == 0)
+        panicked_action = PANICKED_REPORT;
+    else if (strcasecmp(p, "pause") == 0)
+        panicked_action = PANICKED_PAUSE;
+    else if (strcasecmp(p, "quit") == 0)
+        panicked_action = PANICKED_QUIT;
+    else
+        return -1;
+
+    return 0;
+}
diff --git a/kvm-stub.c b/kvm-stub.c
index 47c573d..4cf977e 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -128,3 +128,12 @@ int kvm_on_sigbus(int code, void *addr)
 {
     return 1;
 }
+
+void kvm_pv_port_init(void)
+{
+}
+
+int select_panicked_action(const char *p)
+{
+    return -1;
+}
diff --git a/kvm.h b/kvm.h
index 4ccae8c..95075cf 100644
--- a/kvm.h
+++ b/kvm.h
@@ -60,6 +60,9 @@ int kvm_has_gsi_routing(void);
 
 int kvm_allows_irq0_override(void);
 
+void kvm_pv_port_init(void);
+int select_panicked_action(const char *p);
+
 #ifdef NEED_CPU_H
 int kvm_init_vcpu(CPUArchState *env);
 
diff --git a/monitor.c b/monitor.c
index 12a6fe2..83cb059 100644
--- a/monitor.c
+++ b/monitor.c
@@ -493,6 +493,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
         case QEVENT_WAKEUP:
             event_name = "WAKEUP";
             break;
+        case QEVENT_GUEST_PANICKED:
+            event_name = "GUEST_PANICKED";
+            break;
         default:
             abort();
             break;
diff --git a/monitor.h b/monitor.h
index 0d49800..94e8a3c 100644
--- a/monitor.h
+++ b/monitor.h
@@ -41,6 +41,7 @@ typedef enum MonitorEvent {
     QEVENT_DEVICE_TRAY_MOVED,
     QEVENT_SUSPEND,
     QEVENT_WAKEUP,
+    QEVENT_GUEST_PANICKED,
     QEVENT_MAX,
 } MonitorEvent;
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 2ca7195..ee5c9e9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -119,11 +119,15 @@
 # @suspended: guest is suspended (ACPI S3)
 #
 # @watchdog: the watchdog action is configured to pause and has been triggered
+#
+# @guest-panicked: the panicked action is configured to pause and has been
+# triggered.
 ##
 { 'enum': 'RunState',
   'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
             'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
-            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
+            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
+            'guest-panicked' ] }
 
 ##
 # @StatusInfo:
diff --git a/qemu-options.hx b/qemu-options.hx
index 8b66264..d3d21a1 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2743,6 +2743,20 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log,
     "-qtest-log LOG  specify tracing options\n",
     QEMU_ARCH_ALL)
 
+DEF("onpanic", HAS_ARG, QEMU_OPTION_onpanic, \
+    "-onpanic report|pause|quit\n" \
+    "                action when the guest is panicked [default=report]",
+    QEMU_ARCH_ALL)
+STEXI
+@item -onpanic @var{action}
+
+The @var{action} controls what QEmu will do when the guest is panicked.
+The default is @code{report} (emit QEVENT_GUEST_PANICKED only).
+Other possible actions are:
+@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
+@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).
+ETEXI
+
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
 @end table
diff --git a/qmp.c b/qmp.c
index fee9fb2..a2d5ce9 100644
--- a/qmp.c
+++ b/qmp.c
@@ -148,7 +148,8 @@ void qmp_cont(Error **errp)
         error_set(errp, QERR_MIGRATION_EXPECTED);
         return;
     } else if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
-               runstate_check(RUN_STATE_SHUTDOWN)) {
+               runstate_check(RUN_STATE_SHUTDOWN) ||
+               runstate_check(RUN_STATE_GUEST_PANICKED)) {
         error_set(errp, QERR_RESET_REQUIRED);
         return;
     } else if (runstate_check(RUN_STATE_SUSPENDED)) {
diff --git a/vl.c b/vl.c
index 7f5fed8..8848506 100644
--- a/vl.c
+++ b/vl.c
@@ -367,6 +367,7 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_RUNNING, RUN_STATE_SAVE_VM },
     { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN },
     { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
+    { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
 
     { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
 
@@ -381,6 +382,9 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
     { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
 
+    { RUN_STATE_GUEST_PANICKED, RUN_STATE_PAUSED },
+    { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
+
     { RUN_STATE_MAX, RUN_STATE_MAX },
 };
 
@@ -1537,7 +1541,8 @@ static bool main_loop_should_exit(void)
         qemu_system_reset(VMRESET_REPORT);
         resume_all_vcpus();
         if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
-            runstate_check(RUN_STATE_SHUTDOWN)) {
+            runstate_check(RUN_STATE_SHUTDOWN) ||
+            runstate_check(RUN_STATE_GUEST_PANICKED)) {
             runstate_set(RUN_STATE_PAUSED);
             vm_start();
         }
@@ -3202,6 +3207,12 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_qtest_log:
                 qtest_log = optarg;
                 break;
+            case QEMU_OPTION_onpanic:
+                if (select_panicked_action(optarg) == -1) {
+                    fprintf(stderr, "Unknown -onpanic parameter\n");
+                    exit(1);
+                }
+                break;
             default:
                 os_parse_cmd_args(popt->index, optarg);
             }
@@ -3634,6 +3645,10 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
+    if (kvm_enabled()) {
+        kvm_pv_port_init();
+    }
+
     if (incoming) {
         Error *errp = NULL;
         int ret = qemu_start_incoming_migration(incoming, &errp);
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [Qemu-devel] [PATCH 3/3] deal with guest panicked event
@ 2012-05-21  6:50   ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-05-21  6:50 UTC (permalink / raw)
  To: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

When the guest is panicked, it will write 0x1 to the port 0x505. So if
qemu reads 0x1 from this port, we can do the folloing three things
according to the parameter -onpanic:
1. emit QEVENT_GUEST_PANICKED only
2. emit QEVENT_GUEST_PANICKED and pause VM
3. emit QEVENT_GUEST_PANICKED and quit VM

Note: if we emit QEVENT_GUEST_PANICKED only, and the management
application does not receive this event(the management may not
run when the event is emitted), the management won't know the
guest is panicked.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 kvm-all.c        |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 kvm-stub.c       |    9 ++++++
 kvm.h            |    3 ++
 monitor.c        |    3 ++
 monitor.h        |    1 +
 qapi-schema.json |    6 +++-
 qemu-options.hx  |   14 +++++++++
 qmp.c            |    3 +-
 vl.c             |   17 ++++++++++-
 9 files changed, 137 insertions(+), 3 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 9b73ccf..b5b0531 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -19,6 +19,7 @@
 #include <stdarg.h>
 
 #include <linux/kvm.h>
+#include <linux/kvm_para.h>
 
 #include "qemu-common.h"
 #include "qemu-barrier.h"
@@ -29,6 +30,8 @@
 #include "bswap.h"
 #include "memory.h"
 #include "exec-memory.h"
+#include "iorange.h"
+#include "qemu-objects.h"
 
 /* This check must be after config-host.h is included */
 #ifdef CONFIG_EVENTFD
@@ -1707,3 +1710,84 @@ int kvm_on_sigbus(int code, void *addr)
 {
     return kvm_arch_on_sigbus(code, addr);
 }
+
+/* Possible values for action parameter. */
+#define PANICKED_REPORT 1   /* emit QEVENT_GUEST_PANICKED only */
+#define PANICKED_PAUSE  2   /* emit QEVENT_GUEST_PANICKED and pause VM */
+#define PANICKED_QUIT   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
+
+static int panicked_action = PANICKED_REPORT;
+
+static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
+                             uint64_t *data)
+{
+    *data = (1 << KVM_PV_FEATURE_PANICKED);
+}
+
+static void panicked_mon_event(const char *action)
+{
+    QObject *data;
+
+    data = qobject_from_jsonf("{ 'action': %s }", action);
+    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
+    qobject_decref(data);
+}
+
+static void panicked_perform_action(void)
+{
+    switch(panicked_action) {
+    case PANICKED_REPORT:
+        panicked_mon_event("report");
+        break;
+
+    case PANICKED_PAUSE:
+        panicked_mon_event("pause");
+        vm_stop(RUN_STATE_GUEST_PANICKED);
+        break;
+
+    case PANICKED_QUIT:
+        panicked_mon_event("quit");
+        exit(0);
+        break;
+    }
+}
+
+static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
+                              uint64_t data)
+{
+    if (data == KVM_PV_PANICKED)
+        panicked_perform_action();
+}
+
+static void kvm_pv_port_destructor(IORange *iorange)
+{
+    g_free(iorange);
+}
+
+static IORangeOps pv_io_range_ops = {
+    .read = kvm_pv_port_read,
+    .write = kvm_pv_port_write,
+    .destructor = kvm_pv_port_destructor,
+};
+
+void kvm_pv_port_init(void)
+{
+    IORange *pv_io_range = g_malloc(sizeof(IORange));
+
+    iorange_init(pv_io_range, &pv_io_range_ops, 0x505, 1);
+    ioport_register(pv_io_range);
+}
+
+int select_panicked_action(const char *p)
+{
+    if (strcasecmp(p, "report") == 0)
+        panicked_action = PANICKED_REPORT;
+    else if (strcasecmp(p, "pause") == 0)
+        panicked_action = PANICKED_PAUSE;
+    else if (strcasecmp(p, "quit") == 0)
+        panicked_action = PANICKED_QUIT;
+    else
+        return -1;
+
+    return 0;
+}
diff --git a/kvm-stub.c b/kvm-stub.c
index 47c573d..4cf977e 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -128,3 +128,12 @@ int kvm_on_sigbus(int code, void *addr)
 {
     return 1;
 }
+
+void kvm_pv_port_init(void)
+{
+}
+
+int select_panicked_action(const char *p)
+{
+    return -1;
+}
diff --git a/kvm.h b/kvm.h
index 4ccae8c..95075cf 100644
--- a/kvm.h
+++ b/kvm.h
@@ -60,6 +60,9 @@ int kvm_has_gsi_routing(void);
 
 int kvm_allows_irq0_override(void);
 
+void kvm_pv_port_init(void);
+int select_panicked_action(const char *p);
+
 #ifdef NEED_CPU_H
 int kvm_init_vcpu(CPUArchState *env);
 
diff --git a/monitor.c b/monitor.c
index 12a6fe2..83cb059 100644
--- a/monitor.c
+++ b/monitor.c
@@ -493,6 +493,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
         case QEVENT_WAKEUP:
             event_name = "WAKEUP";
             break;
+        case QEVENT_GUEST_PANICKED:
+            event_name = "GUEST_PANICKED";
+            break;
         default:
             abort();
             break;
diff --git a/monitor.h b/monitor.h
index 0d49800..94e8a3c 100644
--- a/monitor.h
+++ b/monitor.h
@@ -41,6 +41,7 @@ typedef enum MonitorEvent {
     QEVENT_DEVICE_TRAY_MOVED,
     QEVENT_SUSPEND,
     QEVENT_WAKEUP,
+    QEVENT_GUEST_PANICKED,
     QEVENT_MAX,
 } MonitorEvent;
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 2ca7195..ee5c9e9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -119,11 +119,15 @@
 # @suspended: guest is suspended (ACPI S3)
 #
 # @watchdog: the watchdog action is configured to pause and has been triggered
+#
+# @guest-panicked: the panicked action is configured to pause and has been
+# triggered.
 ##
 { 'enum': 'RunState',
   'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
             'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
-            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
+            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
+            'guest-panicked' ] }
 
 ##
 # @StatusInfo:
diff --git a/qemu-options.hx b/qemu-options.hx
index 8b66264..d3d21a1 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2743,6 +2743,20 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log,
     "-qtest-log LOG  specify tracing options\n",
     QEMU_ARCH_ALL)
 
+DEF("onpanic", HAS_ARG, QEMU_OPTION_onpanic, \
+    "-onpanic report|pause|quit\n" \
+    "                action when the guest is panicked [default=report]",
+    QEMU_ARCH_ALL)
+STEXI
+@item -onpanic @var{action}
+
+The @var{action} controls what QEmu will do when the guest is panicked.
+The default is @code{report} (emit QEVENT_GUEST_PANICKED only).
+Other possible actions are:
+@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
+@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).
+ETEXI
+
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
 @end table
diff --git a/qmp.c b/qmp.c
index fee9fb2..a2d5ce9 100644
--- a/qmp.c
+++ b/qmp.c
@@ -148,7 +148,8 @@ void qmp_cont(Error **errp)
         error_set(errp, QERR_MIGRATION_EXPECTED);
         return;
     } else if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
-               runstate_check(RUN_STATE_SHUTDOWN)) {
+               runstate_check(RUN_STATE_SHUTDOWN) ||
+               runstate_check(RUN_STATE_GUEST_PANICKED)) {
         error_set(errp, QERR_RESET_REQUIRED);
         return;
     } else if (runstate_check(RUN_STATE_SUSPENDED)) {
diff --git a/vl.c b/vl.c
index 7f5fed8..8848506 100644
--- a/vl.c
+++ b/vl.c
@@ -367,6 +367,7 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_RUNNING, RUN_STATE_SAVE_VM },
     { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN },
     { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
+    { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
 
     { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
 
@@ -381,6 +382,9 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
     { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
 
+    { RUN_STATE_GUEST_PANICKED, RUN_STATE_PAUSED },
+    { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
+
     { RUN_STATE_MAX, RUN_STATE_MAX },
 };
 
@@ -1537,7 +1541,8 @@ static bool main_loop_should_exit(void)
         qemu_system_reset(VMRESET_REPORT);
         resume_all_vcpus();
         if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
-            runstate_check(RUN_STATE_SHUTDOWN)) {
+            runstate_check(RUN_STATE_SHUTDOWN) ||
+            runstate_check(RUN_STATE_GUEST_PANICKED)) {
             runstate_set(RUN_STATE_PAUSED);
             vm_start();
         }
@@ -3202,6 +3207,12 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_qtest_log:
                 qtest_log = optarg;
                 break;
+            case QEMU_OPTION_onpanic:
+                if (select_panicked_action(optarg) == -1) {
+                    fprintf(stderr, "Unknown -onpanic parameter\n");
+                    exit(1);
+                }
+                break;
             default:
                 os_parse_cmd_args(popt->index, optarg);
             }
@@ -3634,6 +3645,10 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
+    if (kvm_enabled()) {
+        kvm_pv_port_init();
+    }
+
     if (incoming) {
         Error *errp = NULL;
         int ret = qemu_start_incoming_migration(incoming, &errp);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 50+ messages in thread

* Re: [PATCH 3/3] deal with guest panicked event
  2012-05-21  6:50   ` [Qemu-devel] " Wen Congyang
  (?)
@ 2012-05-22 11:32     ` Jan Kiszka
  -1 siblings, 0 replies; 50+ messages in thread
From: Jan Kiszka @ 2012-05-22 11:32 UTC (permalink / raw)
  To: Wen Congyang
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Gleb Natapov

On 2012-05-21 03:50, Wen Congyang wrote:
> When the guest is panicked, it will write 0x1 to the port 0x505. So if
> qemu reads 0x1 from this port, we can do the folloing three things
> according to the parameter -onpanic:
> 1. emit QEVENT_GUEST_PANICKED only
> 2. emit QEVENT_GUEST_PANICKED and pause VM
> 3. emit QEVENT_GUEST_PANICKED and quit VM
> 
> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
> application does not receive this event(the management may not
> run when the event is emitted), the management won't know the
> guest is panicked.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  kvm-all.c        |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  kvm-stub.c       |    9 ++++++
>  kvm.h            |    3 ++
>  monitor.c        |    3 ++
>  monitor.h        |    1 +
>  qapi-schema.json |    6 +++-
>  qemu-options.hx  |   14 +++++++++
>  qmp.c            |    3 +-
>  vl.c             |   17 ++++++++++-
>  9 files changed, 137 insertions(+), 3 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 9b73ccf..b5b0531 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -19,6 +19,7 @@
>  #include <stdarg.h>
>  
>  #include <linux/kvm.h>
> +#include <linux/kvm_para.h>
>  
>  #include "qemu-common.h"
>  #include "qemu-barrier.h"
> @@ -29,6 +30,8 @@
>  #include "bswap.h"
>  #include "memory.h"
>  #include "exec-memory.h"
> +#include "iorange.h"
> +#include "qemu-objects.h"
>  
>  /* This check must be after config-host.h is included */
>  #ifdef CONFIG_EVENTFD
> @@ -1707,3 +1710,84 @@ int kvm_on_sigbus(int code, void *addr)
>  {
>      return kvm_arch_on_sigbus(code, addr);
>  }
> +
> +/* Possible values for action parameter. */
> +#define PANICKED_REPORT 1   /* emit QEVENT_GUEST_PANICKED only */
> +#define PANICKED_PAUSE  2   /* emit QEVENT_GUEST_PANICKED and pause VM */
> +#define PANICKED_QUIT   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
> +
> +static int panicked_action = PANICKED_REPORT;
> +
> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
> +                             uint64_t *data)
> +{
> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
> +}
> +
> +static void panicked_mon_event(const char *action)
> +{
> +    QObject *data;
> +
> +    data = qobject_from_jsonf("{ 'action': %s }", action);
> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
> +    qobject_decref(data);
> +}
> +
> +static void panicked_perform_action(void)
> +{
> +    switch(panicked_action) {
> +    case PANICKED_REPORT:
> +        panicked_mon_event("report");
> +        break;
> +
> +    case PANICKED_PAUSE:
> +        panicked_mon_event("pause");
> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> +        break;
> +
> +    case PANICKED_QUIT:
> +        panicked_mon_event("quit");
> +        exit(0);
> +        break;
> +    }
> +}
> +
> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
> +                              uint64_t data)
> +{
> +    if (data == KVM_PV_PANICKED)
> +        panicked_perform_action();
> +}
> +
> +static void kvm_pv_port_destructor(IORange *iorange)
> +{
> +    g_free(iorange);
> +}
> +
> +static IORangeOps pv_io_range_ops = {
> +    .read = kvm_pv_port_read,
> +    .write = kvm_pv_port_write,
> +    .destructor = kvm_pv_port_destructor,
> +};
> +
> +void kvm_pv_port_init(void)
> +{
> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
> +
> +    iorange_init(pv_io_range, &pv_io_range_ops, 0x505, 1);
> +    ioport_register(pv_io_range);

Not sure if the discussion about the PV channel already settled, but if
PIO is the way to go, please model this as a proper QEMU device (e.g.
"panic", implemented in hw/kvm/panic.c), not just an open-coded PIO range.

Jan

PS: checkpatch.pl...

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 3/3] deal with guest panicked event
@ 2012-05-22 11:32     ` Jan Kiszka
  0 siblings, 0 replies; 50+ messages in thread
From: Jan Kiszka @ 2012-05-22 11:32 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

On 2012-05-21 03:50, Wen Congyang wrote:
> When the guest is panicked, it will write 0x1 to the port 0x505. So if
> qemu reads 0x1 from this port, we can do the folloing three things
> according to the parameter -onpanic:
> 1. emit QEVENT_GUEST_PANICKED only
> 2. emit QEVENT_GUEST_PANICKED and pause VM
> 3. emit QEVENT_GUEST_PANICKED and quit VM
> 
> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
> application does not receive this event(the management may not
> run when the event is emitted), the management won't know the
> guest is panicked.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  kvm-all.c        |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  kvm-stub.c       |    9 ++++++
>  kvm.h            |    3 ++
>  monitor.c        |    3 ++
>  monitor.h        |    1 +
>  qapi-schema.json |    6 +++-
>  qemu-options.hx  |   14 +++++++++
>  qmp.c            |    3 +-
>  vl.c             |   17 ++++++++++-
>  9 files changed, 137 insertions(+), 3 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 9b73ccf..b5b0531 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -19,6 +19,7 @@
>  #include <stdarg.h>
>  
>  #include <linux/kvm.h>
> +#include <linux/kvm_para.h>
>  
>  #include "qemu-common.h"
>  #include "qemu-barrier.h"
> @@ -29,6 +30,8 @@
>  #include "bswap.h"
>  #include "memory.h"
>  #include "exec-memory.h"
> +#include "iorange.h"
> +#include "qemu-objects.h"
>  
>  /* This check must be after config-host.h is included */
>  #ifdef CONFIG_EVENTFD
> @@ -1707,3 +1710,84 @@ int kvm_on_sigbus(int code, void *addr)
>  {
>      return kvm_arch_on_sigbus(code, addr);
>  }
> +
> +/* Possible values for action parameter. */
> +#define PANICKED_REPORT 1   /* emit QEVENT_GUEST_PANICKED only */
> +#define PANICKED_PAUSE  2   /* emit QEVENT_GUEST_PANICKED and pause VM */
> +#define PANICKED_QUIT   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
> +
> +static int panicked_action = PANICKED_REPORT;
> +
> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
> +                             uint64_t *data)
> +{
> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
> +}
> +
> +static void panicked_mon_event(const char *action)
> +{
> +    QObject *data;
> +
> +    data = qobject_from_jsonf("{ 'action': %s }", action);
> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
> +    qobject_decref(data);
> +}
> +
> +static void panicked_perform_action(void)
> +{
> +    switch(panicked_action) {
> +    case PANICKED_REPORT:
> +        panicked_mon_event("report");
> +        break;
> +
> +    case PANICKED_PAUSE:
> +        panicked_mon_event("pause");
> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> +        break;
> +
> +    case PANICKED_QUIT:
> +        panicked_mon_event("quit");
> +        exit(0);
> +        break;
> +    }
> +}
> +
> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
> +                              uint64_t data)
> +{
> +    if (data == KVM_PV_PANICKED)
> +        panicked_perform_action();
> +}
> +
> +static void kvm_pv_port_destructor(IORange *iorange)
> +{
> +    g_free(iorange);
> +}
> +
> +static IORangeOps pv_io_range_ops = {
> +    .read = kvm_pv_port_read,
> +    .write = kvm_pv_port_write,
> +    .destructor = kvm_pv_port_destructor,
> +};
> +
> +void kvm_pv_port_init(void)
> +{
> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
> +
> +    iorange_init(pv_io_range, &pv_io_range_ops, 0x505, 1);
> +    ioport_register(pv_io_range);

Not sure if the discussion about the PV channel already settled, but if
PIO is the way to go, please model this as a proper QEMU device (e.g.
"panic", implemented in hw/kvm/panic.c), not just an open-coded PIO range.

Jan

PS: checkpatch.pl...

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] deal with guest panicked event
@ 2012-05-22 11:32     ` Jan Kiszka
  0 siblings, 0 replies; 50+ messages in thread
From: Jan Kiszka @ 2012-05-22 11:32 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki

On 2012-05-21 03:50, Wen Congyang wrote:
> When the guest is panicked, it will write 0x1 to the port 0x505. So if
> qemu reads 0x1 from this port, we can do the folloing three things
> according to the parameter -onpanic:
> 1. emit QEVENT_GUEST_PANICKED only
> 2. emit QEVENT_GUEST_PANICKED and pause VM
> 3. emit QEVENT_GUEST_PANICKED and quit VM
> 
> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
> application does not receive this event(the management may not
> run when the event is emitted), the management won't know the
> guest is panicked.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  kvm-all.c        |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  kvm-stub.c       |    9 ++++++
>  kvm.h            |    3 ++
>  monitor.c        |    3 ++
>  monitor.h        |    1 +
>  qapi-schema.json |    6 +++-
>  qemu-options.hx  |   14 +++++++++
>  qmp.c            |    3 +-
>  vl.c             |   17 ++++++++++-
>  9 files changed, 137 insertions(+), 3 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 9b73ccf..b5b0531 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -19,6 +19,7 @@
>  #include <stdarg.h>
>  
>  #include <linux/kvm.h>
> +#include <linux/kvm_para.h>
>  
>  #include "qemu-common.h"
>  #include "qemu-barrier.h"
> @@ -29,6 +30,8 @@
>  #include "bswap.h"
>  #include "memory.h"
>  #include "exec-memory.h"
> +#include "iorange.h"
> +#include "qemu-objects.h"
>  
>  /* This check must be after config-host.h is included */
>  #ifdef CONFIG_EVENTFD
> @@ -1707,3 +1710,84 @@ int kvm_on_sigbus(int code, void *addr)
>  {
>      return kvm_arch_on_sigbus(code, addr);
>  }
> +
> +/* Possible values for action parameter. */
> +#define PANICKED_REPORT 1   /* emit QEVENT_GUEST_PANICKED only */
> +#define PANICKED_PAUSE  2   /* emit QEVENT_GUEST_PANICKED and pause VM */
> +#define PANICKED_QUIT   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
> +
> +static int panicked_action = PANICKED_REPORT;
> +
> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
> +                             uint64_t *data)
> +{
> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
> +}
> +
> +static void panicked_mon_event(const char *action)
> +{
> +    QObject *data;
> +
> +    data = qobject_from_jsonf("{ 'action': %s }", action);
> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
> +    qobject_decref(data);
> +}
> +
> +static void panicked_perform_action(void)
> +{
> +    switch(panicked_action) {
> +    case PANICKED_REPORT:
> +        panicked_mon_event("report");
> +        break;
> +
> +    case PANICKED_PAUSE:
> +        panicked_mon_event("pause");
> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> +        break;
> +
> +    case PANICKED_QUIT:
> +        panicked_mon_event("quit");
> +        exit(0);
> +        break;
> +    }
> +}
> +
> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
> +                              uint64_t data)
> +{
> +    if (data == KVM_PV_PANICKED)
> +        panicked_perform_action();
> +}
> +
> +static void kvm_pv_port_destructor(IORange *iorange)
> +{
> +    g_free(iorange);
> +}
> +
> +static IORangeOps pv_io_range_ops = {
> +    .read = kvm_pv_port_read,
> +    .write = kvm_pv_port_write,
> +    .destructor = kvm_pv_port_destructor,
> +};
> +
> +void kvm_pv_port_init(void)
> +{
> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
> +
> +    iorange_init(pv_io_range, &pv_io_range_ops, 0x505, 1);
> +    ioport_register(pv_io_range);

Not sure if the discussion about the PV channel already settled, but if
PIO is the way to go, please model this as a proper QEMU device (e.g.
"panic", implemented in hw/kvm/panic.c), not just an open-coded PIO range.

Jan

PS: checkpatch.pl...

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] start vm after reseting it
  2012-05-21  6:49   ` [Qemu-devel] " Wen Congyang
@ 2012-05-30 19:17     ` Luiz Capitulino
  -1 siblings, 0 replies; 50+ messages in thread
From: Luiz Capitulino @ 2012-05-30 19:17 UTC (permalink / raw)
  To: Wen Congyang
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

On Mon, 21 May 2012 14:49:32 +0800
Wen Congyang <wency@cn.fujitsu.com> wrote:

> The guest should run after reseting it, but it does not
> run if its old state is RUN_STATE_INTERNAL_ERROR or RUN_STATE_PAUSED.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  vl.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 23ab3a3..7f5fed8 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1539,6 +1539,7 @@ static bool main_loop_should_exit(void)
>          if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
>              runstate_check(RUN_STATE_SHUTDOWN)) {
>              runstate_set(RUN_STATE_PAUSED);
> +            vm_start();

Please, drop the runstate_set() call. I think you also have to
to call bdrv_iostatus_reset(), as qmp_cont() does.

>          }
>      }
>      if (qemu_powerdown_requested()) {


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 1/3] start vm after reseting it
@ 2012-05-30 19:17     ` Luiz Capitulino
  0 siblings, 0 replies; 50+ messages in thread
From: Luiz Capitulino @ 2012-05-30 19:17 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, Jan Kiszka, qemu-devel, linux-kernel,
	Avi Kivity, KAMEZAWA Hiroyuki

On Mon, 21 May 2012 14:49:32 +0800
Wen Congyang <wency@cn.fujitsu.com> wrote:

> The guest should run after reseting it, but it does not
> run if its old state is RUN_STATE_INTERNAL_ERROR or RUN_STATE_PAUSED.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  vl.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 23ab3a3..7f5fed8 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1539,6 +1539,7 @@ static bool main_loop_should_exit(void)
>          if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
>              runstate_check(RUN_STATE_SHUTDOWN)) {
>              runstate_set(RUN_STATE_PAUSED);
> +            vm_start();

Please, drop the runstate_set() call. I think you also have to
to call bdrv_iostatus_reset(), as qmp_cont() does.

>          }
>      }
>      if (qemu_powerdown_requested()) {

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] deal with guest panicked event
  2012-05-21  6:50   ` [Qemu-devel] " Wen Congyang
@ 2012-05-30 19:23     ` Luiz Capitulino
  -1 siblings, 0 replies; 50+ messages in thread
From: Luiz Capitulino @ 2012-05-30 19:23 UTC (permalink / raw)
  To: Wen Congyang
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

On Mon, 21 May 2012 14:50:51 +0800
Wen Congyang <wency@cn.fujitsu.com> wrote:

> When the guest is panicked, it will write 0x1 to the port 0x505. So if
> qemu reads 0x1 from this port, we can do the folloing three things
> according to the parameter -onpanic:
> 1. emit QEVENT_GUEST_PANICKED only
> 2. emit QEVENT_GUEST_PANICKED and pause VM
> 3. emit QEVENT_GUEST_PANICKED and quit VM
> 
> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
> application does not receive this event(the management may not
> run when the event is emitted), the management won't know the
> guest is panicked.

It will if it checks the vm status.

Btw, please, split this further into a patch adding the event, another one
adding the new runstate and then the rest. One more comment below.

> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  kvm-all.c        |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  kvm-stub.c       |    9 ++++++
>  kvm.h            |    3 ++
>  monitor.c        |    3 ++
>  monitor.h        |    1 +
>  qapi-schema.json |    6 +++-
>  qemu-options.hx  |   14 +++++++++
>  qmp.c            |    3 +-
>  vl.c             |   17 ++++++++++-
>  9 files changed, 137 insertions(+), 3 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 9b73ccf..b5b0531 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -19,6 +19,7 @@
>  #include <stdarg.h>
>  
>  #include <linux/kvm.h>
> +#include <linux/kvm_para.h>
>  
>  #include "qemu-common.h"
>  #include "qemu-barrier.h"
> @@ -29,6 +30,8 @@
>  #include "bswap.h"
>  #include "memory.h"
>  #include "exec-memory.h"
> +#include "iorange.h"
> +#include "qemu-objects.h"
>  
>  /* This check must be after config-host.h is included */
>  #ifdef CONFIG_EVENTFD
> @@ -1707,3 +1710,84 @@ int kvm_on_sigbus(int code, void *addr)
>  {
>      return kvm_arch_on_sigbus(code, addr);
>  }
> +
> +/* Possible values for action parameter. */
> +#define PANICKED_REPORT 1   /* emit QEVENT_GUEST_PANICKED only */
> +#define PANICKED_PAUSE  2   /* emit QEVENT_GUEST_PANICKED and pause VM */
> +#define PANICKED_QUIT   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
> +
> +static int panicked_action = PANICKED_REPORT;
> +
> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
> +                             uint64_t *data)
> +{
> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
> +}
> +
> +static void panicked_mon_event(const char *action)
> +{
> +    QObject *data;
> +
> +    data = qobject_from_jsonf("{ 'action': %s }", action);
> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
> +    qobject_decref(data);
> +}
> +
> +static void panicked_perform_action(void)
> +{
> +    switch(panicked_action) {
> +    case PANICKED_REPORT:
> +        panicked_mon_event("report");
> +        break;
> +
> +    case PANICKED_PAUSE:
> +        panicked_mon_event("pause");
> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> +        break;
> +
> +    case PANICKED_QUIT:
> +        panicked_mon_event("quit");
> +        exit(0);
> +        break;
> +    }

Having the data argument is not needed/wanted. The mngt app can guess it if it
needs to know it, but I think it doesn't want to.

> +}
> +
> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
> +                              uint64_t data)
> +{
> +    if (data == KVM_PV_PANICKED)
> +        panicked_perform_action();
> +}
> +
> +static void kvm_pv_port_destructor(IORange *iorange)
> +{
> +    g_free(iorange);
> +}
> +
> +static IORangeOps pv_io_range_ops = {
> +    .read = kvm_pv_port_read,
> +    .write = kvm_pv_port_write,
> +    .destructor = kvm_pv_port_destructor,
> +};
> +
> +void kvm_pv_port_init(void)
> +{
> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
> +
> +    iorange_init(pv_io_range, &pv_io_range_ops, 0x505, 1);
> +    ioport_register(pv_io_range);
> +}
> +
> +int select_panicked_action(const char *p)
> +{
> +    if (strcasecmp(p, "report") == 0)
> +        panicked_action = PANICKED_REPORT;
> +    else if (strcasecmp(p, "pause") == 0)
> +        panicked_action = PANICKED_PAUSE;
> +    else if (strcasecmp(p, "quit") == 0)
> +        panicked_action = PANICKED_QUIT;
> +    else
> +        return -1;
> +
> +    return 0;
> +}
> diff --git a/kvm-stub.c b/kvm-stub.c
> index 47c573d..4cf977e 100644
> --- a/kvm-stub.c
> +++ b/kvm-stub.c
> @@ -128,3 +128,12 @@ int kvm_on_sigbus(int code, void *addr)
>  {
>      return 1;
>  }
> +
> +void kvm_pv_port_init(void)
> +{
> +}
> +
> +int select_panicked_action(const char *p)
> +{
> +    return -1;
> +}
> diff --git a/kvm.h b/kvm.h
> index 4ccae8c..95075cf 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -60,6 +60,9 @@ int kvm_has_gsi_routing(void);
>  
>  int kvm_allows_irq0_override(void);
>  
> +void kvm_pv_port_init(void);
> +int select_panicked_action(const char *p);
> +
>  #ifdef NEED_CPU_H
>  int kvm_init_vcpu(CPUArchState *env);
>  
> diff --git a/monitor.c b/monitor.c
> index 12a6fe2..83cb059 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -493,6 +493,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
>          case QEVENT_WAKEUP:
>              event_name = "WAKEUP";
>              break;
> +        case QEVENT_GUEST_PANICKED:
> +            event_name = "GUEST_PANICKED";
> +            break;
>          default:
>              abort();
>              break;
> diff --git a/monitor.h b/monitor.h
> index 0d49800..94e8a3c 100644
> --- a/monitor.h
> +++ b/monitor.h
> @@ -41,6 +41,7 @@ typedef enum MonitorEvent {
>      QEVENT_DEVICE_TRAY_MOVED,
>      QEVENT_SUSPEND,
>      QEVENT_WAKEUP,
> +    QEVENT_GUEST_PANICKED,
>      QEVENT_MAX,
>  } MonitorEvent;
>  
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 2ca7195..ee5c9e9 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -119,11 +119,15 @@
>  # @suspended: guest is suspended (ACPI S3)
>  #
>  # @watchdog: the watchdog action is configured to pause and has been triggered
> +#
> +# @guest-panicked: the panicked action is configured to pause and has been
> +# triggered.
>  ##
>  { 'enum': 'RunState',
>    'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
>              'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
> -            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
> +            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
> +            'guest-panicked' ] }
>  
>  ##
>  # @StatusInfo:
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 8b66264..d3d21a1 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2743,6 +2743,20 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log,
>      "-qtest-log LOG  specify tracing options\n",
>      QEMU_ARCH_ALL)
>  
> +DEF("onpanic", HAS_ARG, QEMU_OPTION_onpanic, \
> +    "-onpanic report|pause|quit\n" \
> +    "                action when the guest is panicked [default=report]",
> +    QEMU_ARCH_ALL)
> +STEXI
> +@item -onpanic @var{action}
> +
> +The @var{action} controls what QEmu will do when the guest is panicked.
> +The default is @code{report} (emit QEVENT_GUEST_PANICKED only).
> +Other possible actions are:
> +@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
> +@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).
> +ETEXI
> +
>  HXCOMM This is the last statement. Insert new options before this line!
>  STEXI
>  @end table
> diff --git a/qmp.c b/qmp.c
> index fee9fb2..a2d5ce9 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -148,7 +148,8 @@ void qmp_cont(Error **errp)
>          error_set(errp, QERR_MIGRATION_EXPECTED);
>          return;
>      } else if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
> -               runstate_check(RUN_STATE_SHUTDOWN)) {
> +               runstate_check(RUN_STATE_SHUTDOWN) ||
> +               runstate_check(RUN_STATE_GUEST_PANICKED)) {
>          error_set(errp, QERR_RESET_REQUIRED);
>          return;
>      } else if (runstate_check(RUN_STATE_SUSPENDED)) {
> diff --git a/vl.c b/vl.c
> index 7f5fed8..8848506 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -367,6 +367,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_RUNNING, RUN_STATE_SAVE_VM },
>      { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN },
>      { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
> +    { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
>  
>      { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
>  
> @@ -381,6 +382,9 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
>      { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
>  
> +    { RUN_STATE_GUEST_PANICKED, RUN_STATE_PAUSED },
> +    { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
> +
>      { RUN_STATE_MAX, RUN_STATE_MAX },
>  };
>  
> @@ -1537,7 +1541,8 @@ static bool main_loop_should_exit(void)
>          qemu_system_reset(VMRESET_REPORT);
>          resume_all_vcpus();
>          if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
> -            runstate_check(RUN_STATE_SHUTDOWN)) {
> +            runstate_check(RUN_STATE_SHUTDOWN) ||
> +            runstate_check(RUN_STATE_GUEST_PANICKED)) {
>              runstate_set(RUN_STATE_PAUSED);
>              vm_start();
>          }
> @@ -3202,6 +3207,12 @@ int main(int argc, char **argv, char **envp)
>              case QEMU_OPTION_qtest_log:
>                  qtest_log = optarg;
>                  break;
> +            case QEMU_OPTION_onpanic:
> +                if (select_panicked_action(optarg) == -1) {
> +                    fprintf(stderr, "Unknown -onpanic parameter\n");
> +                    exit(1);
> +                }
> +                break;
>              default:
>                  os_parse_cmd_args(popt->index, optarg);
>              }
> @@ -3634,6 +3645,10 @@ int main(int argc, char **argv, char **envp)
>          }
>      }
>  
> +    if (kvm_enabled()) {
> +        kvm_pv_port_init();
> +    }
> +
>      if (incoming) {
>          Error *errp = NULL;
>          int ret = qemu_start_incoming_migration(incoming, &errp);


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 3/3] deal with guest panicked event
@ 2012-05-30 19:23     ` Luiz Capitulino
  0 siblings, 0 replies; 50+ messages in thread
From: Luiz Capitulino @ 2012-05-30 19:23 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, Jan Kiszka, qemu-devel, linux-kernel,
	Avi Kivity, KAMEZAWA Hiroyuki

On Mon, 21 May 2012 14:50:51 +0800
Wen Congyang <wency@cn.fujitsu.com> wrote:

> When the guest is panicked, it will write 0x1 to the port 0x505. So if
> qemu reads 0x1 from this port, we can do the folloing three things
> according to the parameter -onpanic:
> 1. emit QEVENT_GUEST_PANICKED only
> 2. emit QEVENT_GUEST_PANICKED and pause VM
> 3. emit QEVENT_GUEST_PANICKED and quit VM
> 
> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
> application does not receive this event(the management may not
> run when the event is emitted), the management won't know the
> guest is panicked.

It will if it checks the vm status.

Btw, please, split this further into a patch adding the event, another one
adding the new runstate and then the rest. One more comment below.

> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  kvm-all.c        |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  kvm-stub.c       |    9 ++++++
>  kvm.h            |    3 ++
>  monitor.c        |    3 ++
>  monitor.h        |    1 +
>  qapi-schema.json |    6 +++-
>  qemu-options.hx  |   14 +++++++++
>  qmp.c            |    3 +-
>  vl.c             |   17 ++++++++++-
>  9 files changed, 137 insertions(+), 3 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 9b73ccf..b5b0531 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -19,6 +19,7 @@
>  #include <stdarg.h>
>  
>  #include <linux/kvm.h>
> +#include <linux/kvm_para.h>
>  
>  #include "qemu-common.h"
>  #include "qemu-barrier.h"
> @@ -29,6 +30,8 @@
>  #include "bswap.h"
>  #include "memory.h"
>  #include "exec-memory.h"
> +#include "iorange.h"
> +#include "qemu-objects.h"
>  
>  /* This check must be after config-host.h is included */
>  #ifdef CONFIG_EVENTFD
> @@ -1707,3 +1710,84 @@ int kvm_on_sigbus(int code, void *addr)
>  {
>      return kvm_arch_on_sigbus(code, addr);
>  }
> +
> +/* Possible values for action parameter. */
> +#define PANICKED_REPORT 1   /* emit QEVENT_GUEST_PANICKED only */
> +#define PANICKED_PAUSE  2   /* emit QEVENT_GUEST_PANICKED and pause VM */
> +#define PANICKED_QUIT   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
> +
> +static int panicked_action = PANICKED_REPORT;
> +
> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
> +                             uint64_t *data)
> +{
> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
> +}
> +
> +static void panicked_mon_event(const char *action)
> +{
> +    QObject *data;
> +
> +    data = qobject_from_jsonf("{ 'action': %s }", action);
> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
> +    qobject_decref(data);
> +}
> +
> +static void panicked_perform_action(void)
> +{
> +    switch(panicked_action) {
> +    case PANICKED_REPORT:
> +        panicked_mon_event("report");
> +        break;
> +
> +    case PANICKED_PAUSE:
> +        panicked_mon_event("pause");
> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> +        break;
> +
> +    case PANICKED_QUIT:
> +        panicked_mon_event("quit");
> +        exit(0);
> +        break;
> +    }

Having the data argument is not needed/wanted. The mngt app can guess it if it
needs to know it, but I think it doesn't want to.

> +}
> +
> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
> +                              uint64_t data)
> +{
> +    if (data == KVM_PV_PANICKED)
> +        panicked_perform_action();
> +}
> +
> +static void kvm_pv_port_destructor(IORange *iorange)
> +{
> +    g_free(iorange);
> +}
> +
> +static IORangeOps pv_io_range_ops = {
> +    .read = kvm_pv_port_read,
> +    .write = kvm_pv_port_write,
> +    .destructor = kvm_pv_port_destructor,
> +};
> +
> +void kvm_pv_port_init(void)
> +{
> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
> +
> +    iorange_init(pv_io_range, &pv_io_range_ops, 0x505, 1);
> +    ioport_register(pv_io_range);
> +}
> +
> +int select_panicked_action(const char *p)
> +{
> +    if (strcasecmp(p, "report") == 0)
> +        panicked_action = PANICKED_REPORT;
> +    else if (strcasecmp(p, "pause") == 0)
> +        panicked_action = PANICKED_PAUSE;
> +    else if (strcasecmp(p, "quit") == 0)
> +        panicked_action = PANICKED_QUIT;
> +    else
> +        return -1;
> +
> +    return 0;
> +}
> diff --git a/kvm-stub.c b/kvm-stub.c
> index 47c573d..4cf977e 100644
> --- a/kvm-stub.c
> +++ b/kvm-stub.c
> @@ -128,3 +128,12 @@ int kvm_on_sigbus(int code, void *addr)
>  {
>      return 1;
>  }
> +
> +void kvm_pv_port_init(void)
> +{
> +}
> +
> +int select_panicked_action(const char *p)
> +{
> +    return -1;
> +}
> diff --git a/kvm.h b/kvm.h
> index 4ccae8c..95075cf 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -60,6 +60,9 @@ int kvm_has_gsi_routing(void);
>  
>  int kvm_allows_irq0_override(void);
>  
> +void kvm_pv_port_init(void);
> +int select_panicked_action(const char *p);
> +
>  #ifdef NEED_CPU_H
>  int kvm_init_vcpu(CPUArchState *env);
>  
> diff --git a/monitor.c b/monitor.c
> index 12a6fe2..83cb059 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -493,6 +493,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
>          case QEVENT_WAKEUP:
>              event_name = "WAKEUP";
>              break;
> +        case QEVENT_GUEST_PANICKED:
> +            event_name = "GUEST_PANICKED";
> +            break;
>          default:
>              abort();
>              break;
> diff --git a/monitor.h b/monitor.h
> index 0d49800..94e8a3c 100644
> --- a/monitor.h
> +++ b/monitor.h
> @@ -41,6 +41,7 @@ typedef enum MonitorEvent {
>      QEVENT_DEVICE_TRAY_MOVED,
>      QEVENT_SUSPEND,
>      QEVENT_WAKEUP,
> +    QEVENT_GUEST_PANICKED,
>      QEVENT_MAX,
>  } MonitorEvent;
>  
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 2ca7195..ee5c9e9 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -119,11 +119,15 @@
>  # @suspended: guest is suspended (ACPI S3)
>  #
>  # @watchdog: the watchdog action is configured to pause and has been triggered
> +#
> +# @guest-panicked: the panicked action is configured to pause and has been
> +# triggered.
>  ##
>  { 'enum': 'RunState',
>    'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
>              'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
> -            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
> +            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
> +            'guest-panicked' ] }
>  
>  ##
>  # @StatusInfo:
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 8b66264..d3d21a1 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2743,6 +2743,20 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log,
>      "-qtest-log LOG  specify tracing options\n",
>      QEMU_ARCH_ALL)
>  
> +DEF("onpanic", HAS_ARG, QEMU_OPTION_onpanic, \
> +    "-onpanic report|pause|quit\n" \
> +    "                action when the guest is panicked [default=report]",
> +    QEMU_ARCH_ALL)
> +STEXI
> +@item -onpanic @var{action}
> +
> +The @var{action} controls what QEmu will do when the guest is panicked.
> +The default is @code{report} (emit QEVENT_GUEST_PANICKED only).
> +Other possible actions are:
> +@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
> +@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).
> +ETEXI
> +
>  HXCOMM This is the last statement. Insert new options before this line!
>  STEXI
>  @end table
> diff --git a/qmp.c b/qmp.c
> index fee9fb2..a2d5ce9 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -148,7 +148,8 @@ void qmp_cont(Error **errp)
>          error_set(errp, QERR_MIGRATION_EXPECTED);
>          return;
>      } else if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
> -               runstate_check(RUN_STATE_SHUTDOWN)) {
> +               runstate_check(RUN_STATE_SHUTDOWN) ||
> +               runstate_check(RUN_STATE_GUEST_PANICKED)) {
>          error_set(errp, QERR_RESET_REQUIRED);
>          return;
>      } else if (runstate_check(RUN_STATE_SUSPENDED)) {
> diff --git a/vl.c b/vl.c
> index 7f5fed8..8848506 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -367,6 +367,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_RUNNING, RUN_STATE_SAVE_VM },
>      { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN },
>      { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
> +    { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
>  
>      { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
>  
> @@ -381,6 +382,9 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
>      { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
>  
> +    { RUN_STATE_GUEST_PANICKED, RUN_STATE_PAUSED },
> +    { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
> +
>      { RUN_STATE_MAX, RUN_STATE_MAX },
>  };
>  
> @@ -1537,7 +1541,8 @@ static bool main_loop_should_exit(void)
>          qemu_system_reset(VMRESET_REPORT);
>          resume_all_vcpus();
>          if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
> -            runstate_check(RUN_STATE_SHUTDOWN)) {
> +            runstate_check(RUN_STATE_SHUTDOWN) ||
> +            runstate_check(RUN_STATE_GUEST_PANICKED)) {
>              runstate_set(RUN_STATE_PAUSED);
>              vm_start();
>          }
> @@ -3202,6 +3207,12 @@ int main(int argc, char **argv, char **envp)
>              case QEMU_OPTION_qtest_log:
>                  qtest_log = optarg;
>                  break;
> +            case QEMU_OPTION_onpanic:
> +                if (select_panicked_action(optarg) == -1) {
> +                    fprintf(stderr, "Unknown -onpanic parameter\n");
> +                    exit(1);
> +                }
> +                break;
>              default:
>                  os_parse_cmd_args(popt->index, optarg);
>              }
> @@ -3634,6 +3645,10 @@ int main(int argc, char **argv, char **envp)
>          }
>      }
>  
> +    if (kvm_enabled()) {
> +        kvm_pv_port_init();
> +    }
> +
>      if (incoming) {
>          Error *errp = NULL;
>          int ret = qemu_start_incoming_migration(incoming, &errp);

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH] kvm: notify host when guest panicked
  2012-05-21  6:46 ` [Qemu-devel] " Wen Congyang
@ 2012-06-12  6:47   ` Wen Congyang
  -1 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-12  6:47 UTC (permalink / raw)
  To: kvm list, Avi Kivity
  Cc: qemu-devel, linux-kernel, Daniel P. Berrange, KAMEZAWA Hiroyuki,
	Jan Kiszka, Gleb Natapov

Hi, Avi

Do you have any comment to this patch?

Thanks
Wen Congyang

At 05/21/2012 02:46 PM, Wen Congyang Wrote:
> We can know the guest is panicked when the guest runs on xen.
> But we do not have such feature on kvm.
> 
> Another purpose of this feature is: management app(for example:
> libvirt) can do auto dump when the guest is panicked. If management
> app does not do auto dump, the guest's user can do dump by hand if
> he sees the guest is panicked.
> 
> We have three solutions to implement this feature:
> 1. use vmcall
> 2. use I/O port
> 3. use virtio-serial.
> 
> We have decided to avoid touching hypervisor. The reason why I choose
> choose the I/O port is:
> 1. it is easier to implememt
> 2. it does not depend any virtual device
> 3. it can work when startint the kernel
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  arch/ia64/include/asm/kvm_para.h    |    5 +++++
>  arch/powerpc/include/asm/kvm_para.h |    5 +++++
>  arch/s390/include/asm/kvm_para.h    |    5 +++++
>  arch/x86/include/asm/kvm_para.h     |    7 +++++++
>  arch/x86/kernel/kvm.c               |   14 ++++++++++++++
>  include/linux/kvm_para.h            |   15 +++++++++++++++
>  6 files changed, 51 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/ia64/include/asm/kvm_para.h b/arch/ia64/include/asm/kvm_para.h
> index 1588aee..a890096 100644
> --- a/arch/ia64/include/asm/kvm_para.h
> +++ b/arch/ia64/include/asm/kvm_para.h
> @@ -26,6 +26,11 @@ static inline unsigned int kvm_arch_para_features(void)
>  	return 0;
>  }
>  
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> +	return 0;
> +}
> +
>  #endif
>  
>  #endif
> diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h
> index 7b754e7..b5f7c35 100644
> --- a/arch/powerpc/include/asm/kvm_para.h
> +++ b/arch/powerpc/include/asm/kvm_para.h
> @@ -206,6 +206,11 @@ static inline unsigned int kvm_arch_para_features(void)
>  	return r;
>  }
>  
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> +	return 0;
> +}
> +
>  #endif /* __KERNEL__ */
>  
>  #endif /* __POWERPC_KVM_PARA_H__ */
> diff --git a/arch/s390/include/asm/kvm_para.h b/arch/s390/include/asm/kvm_para.h
> index 6964db2..21a8d18 100644
> --- a/arch/s390/include/asm/kvm_para.h
> +++ b/arch/s390/include/asm/kvm_para.h
> @@ -149,6 +149,11 @@ static inline unsigned int kvm_arch_para_features(void)
>  	return 0;
>  }
>  
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> +	return 0;
> +}
> +
>  #endif
>  
>  #endif /* __S390_KVM_PARA_H */
> diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
> index 734c376..f66b16e 100644
> --- a/arch/x86/include/asm/kvm_para.h
> +++ b/arch/x86/include/asm/kvm_para.h
> @@ -208,6 +208,13 @@ static inline void kvm_disable_steal_time(void)
>  }
>  #endif
>  
> +#ifdef KVM_PV_PORT
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> +	return inl(KVM_PV_PORT);
> +}
> +#endif
> +
>  #endif /* __KERNEL__ */
>  
>  #endif /* _ASM_X86_KVM_PARA_H */
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index b8ba6e4..adfde45 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -335,6 +335,17 @@ static struct notifier_block kvm_pv_reboot_nb = {
>  	.notifier_call = kvm_pv_reboot_notify,
>  };
>  
> +static int
> +kvm_pv_panic_notify(struct notifier_block *nb, unsigned long code, void *unused)
> +{
> +	outl(KVM_PV_PANICKED, KVM_PV_PORT);
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block kvm_pv_panic_nb = {
> +	.notifier_call = kvm_pv_panic_notify,
> +};
> +
>  static u64 kvm_steal_clock(int cpu)
>  {
>  	u64 steal;
> @@ -421,6 +432,9 @@ void __init kvm_guest_init(void)
>  
>  	paravirt_ops_setup();
>  	register_reboot_notifier(&kvm_pv_reboot_nb);
> +	if (kvm_pv_has_feature(KVM_PV_FEATURE_PANICKED))
> +		atomic_notifier_chain_register(&panic_notifier_list,
> +			&kvm_pv_panic_nb);
>  	for (i = 0; i < KVM_TASK_SLEEP_HASHSIZE; i++)
>  		spin_lock_init(&async_pf_sleepers[i].lock);
>  	if (kvm_para_has_feature(KVM_FEATURE_ASYNC_PF))
> diff --git a/include/linux/kvm_para.h b/include/linux/kvm_para.h
> index ff476dd..849b0d5 100644
> --- a/include/linux/kvm_para.h
> +++ b/include/linux/kvm_para.h
> @@ -20,6 +20,14 @@
>  #define KVM_HC_FEATURES			3
>  #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
>  
> +#define KVM_PV_PORT	(0x505UL)
> +
> +/* The bit of the value read from KVM_PV_PORT */
> +#define KVM_PV_FEATURE_PANICKED	0
> +
> +/* The value writen to KVM_PV_PORT */
> +#define KVM_PV_PANICKED		1
> +
>  /*
>   * hypercalls use architecture specific
>   */
> @@ -33,5 +41,12 @@ static inline int kvm_para_has_feature(unsigned int feature)
>  		return 1;
>  	return 0;
>  }
> +
> +static inline int kvm_pv_has_feature(unsigned int feature)
> +{
> +	if (kvm_arch_pv_features() & (1UL << feature))
> +		return 1;
> +	return 0;
> +}
>  #endif /* __KERNEL__ */
>  #endif /* __LINUX_KVM_PARA_H */


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH] kvm: notify host when guest panicked
@ 2012-06-12  6:47   ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-12  6:47 UTC (permalink / raw)
  To: kvm list, Avi Kivity
  Cc: Gleb Natapov, Jan Kiszka, qemu-devel, linux-kernel, KAMEZAWA Hiroyuki

Hi, Avi

Do you have any comment to this patch?

Thanks
Wen Congyang

At 05/21/2012 02:46 PM, Wen Congyang Wrote:
> We can know the guest is panicked when the guest runs on xen.
> But we do not have such feature on kvm.
> 
> Another purpose of this feature is: management app(for example:
> libvirt) can do auto dump when the guest is panicked. If management
> app does not do auto dump, the guest's user can do dump by hand if
> he sees the guest is panicked.
> 
> We have three solutions to implement this feature:
> 1. use vmcall
> 2. use I/O port
> 3. use virtio-serial.
> 
> We have decided to avoid touching hypervisor. The reason why I choose
> choose the I/O port is:
> 1. it is easier to implememt
> 2. it does not depend any virtual device
> 3. it can work when startint the kernel
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  arch/ia64/include/asm/kvm_para.h    |    5 +++++
>  arch/powerpc/include/asm/kvm_para.h |    5 +++++
>  arch/s390/include/asm/kvm_para.h    |    5 +++++
>  arch/x86/include/asm/kvm_para.h     |    7 +++++++
>  arch/x86/kernel/kvm.c               |   14 ++++++++++++++
>  include/linux/kvm_para.h            |   15 +++++++++++++++
>  6 files changed, 51 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/ia64/include/asm/kvm_para.h b/arch/ia64/include/asm/kvm_para.h
> index 1588aee..a890096 100644
> --- a/arch/ia64/include/asm/kvm_para.h
> +++ b/arch/ia64/include/asm/kvm_para.h
> @@ -26,6 +26,11 @@ static inline unsigned int kvm_arch_para_features(void)
>  	return 0;
>  }
>  
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> +	return 0;
> +}
> +
>  #endif
>  
>  #endif
> diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h
> index 7b754e7..b5f7c35 100644
> --- a/arch/powerpc/include/asm/kvm_para.h
> +++ b/arch/powerpc/include/asm/kvm_para.h
> @@ -206,6 +206,11 @@ static inline unsigned int kvm_arch_para_features(void)
>  	return r;
>  }
>  
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> +	return 0;
> +}
> +
>  #endif /* __KERNEL__ */
>  
>  #endif /* __POWERPC_KVM_PARA_H__ */
> diff --git a/arch/s390/include/asm/kvm_para.h b/arch/s390/include/asm/kvm_para.h
> index 6964db2..21a8d18 100644
> --- a/arch/s390/include/asm/kvm_para.h
> +++ b/arch/s390/include/asm/kvm_para.h
> @@ -149,6 +149,11 @@ static inline unsigned int kvm_arch_para_features(void)
>  	return 0;
>  }
>  
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> +	return 0;
> +}
> +
>  #endif
>  
>  #endif /* __S390_KVM_PARA_H */
> diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
> index 734c376..f66b16e 100644
> --- a/arch/x86/include/asm/kvm_para.h
> +++ b/arch/x86/include/asm/kvm_para.h
> @@ -208,6 +208,13 @@ static inline void kvm_disable_steal_time(void)
>  }
>  #endif
>  
> +#ifdef KVM_PV_PORT
> +static inline unsigned int kvm_arch_pv_features(void)
> +{
> +	return inl(KVM_PV_PORT);
> +}
> +#endif
> +
>  #endif /* __KERNEL__ */
>  
>  #endif /* _ASM_X86_KVM_PARA_H */
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index b8ba6e4..adfde45 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -335,6 +335,17 @@ static struct notifier_block kvm_pv_reboot_nb = {
>  	.notifier_call = kvm_pv_reboot_notify,
>  };
>  
> +static int
> +kvm_pv_panic_notify(struct notifier_block *nb, unsigned long code, void *unused)
> +{
> +	outl(KVM_PV_PANICKED, KVM_PV_PORT);
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block kvm_pv_panic_nb = {
> +	.notifier_call = kvm_pv_panic_notify,
> +};
> +
>  static u64 kvm_steal_clock(int cpu)
>  {
>  	u64 steal;
> @@ -421,6 +432,9 @@ void __init kvm_guest_init(void)
>  
>  	paravirt_ops_setup();
>  	register_reboot_notifier(&kvm_pv_reboot_nb);
> +	if (kvm_pv_has_feature(KVM_PV_FEATURE_PANICKED))
> +		atomic_notifier_chain_register(&panic_notifier_list,
> +			&kvm_pv_panic_nb);
>  	for (i = 0; i < KVM_TASK_SLEEP_HASHSIZE; i++)
>  		spin_lock_init(&async_pf_sleepers[i].lock);
>  	if (kvm_para_has_feature(KVM_FEATURE_ASYNC_PF))
> diff --git a/include/linux/kvm_para.h b/include/linux/kvm_para.h
> index ff476dd..849b0d5 100644
> --- a/include/linux/kvm_para.h
> +++ b/include/linux/kvm_para.h
> @@ -20,6 +20,14 @@
>  #define KVM_HC_FEATURES			3
>  #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
>  
> +#define KVM_PV_PORT	(0x505UL)
> +
> +/* The bit of the value read from KVM_PV_PORT */
> +#define KVM_PV_FEATURE_PANICKED	0
> +
> +/* The value writen to KVM_PV_PORT */
> +#define KVM_PV_PANICKED		1
> +
>  /*
>   * hypercalls use architecture specific
>   */
> @@ -33,5 +41,12 @@ static inline int kvm_para_has_feature(unsigned int feature)
>  		return 1;
>  	return 0;
>  }
> +
> +static inline int kvm_pv_has_feature(unsigned int feature)
> +{
> +	if (kvm_arch_pv_features() & (1UL << feature))
> +		return 1;
> +	return 0;
> +}
>  #endif /* __KERNEL__ */
>  #endif /* __LINUX_KVM_PARA_H */

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] deal with guest panicked event
  2012-05-30 19:23     ` Luiz Capitulino
@ 2012-06-12  6:55       ` Wen Congyang
  -1 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-12  6:55 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

At 05/31/2012 03:23 AM, Luiz Capitulino Wrote:
> On Mon, 21 May 2012 14:50:51 +0800
> Wen Congyang <wency@cn.fujitsu.com> wrote:
> 
>> When the guest is panicked, it will write 0x1 to the port 0x505. So if
>> qemu reads 0x1 from this port, we can do the folloing three things
>> according to the parameter -onpanic:
>> 1. emit QEVENT_GUEST_PANICKED only
>> 2. emit QEVENT_GUEST_PANICKED and pause VM
>> 3. emit QEVENT_GUEST_PANICKED and quit VM
>>
>> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
>> application does not receive this event(the management may not
>> run when the event is emitted), the management won't know the
>> guest is panicked.
> 
> It will if it checks the vm status.
> 
> Btw, please, split this further into a patch adding the event, another one
> adding the new runstate and then the rest. One more comment below.

OK, I will split it.

> 
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  kvm-all.c        |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  kvm-stub.c       |    9 ++++++
>>  kvm.h            |    3 ++
>>  monitor.c        |    3 ++
>>  monitor.h        |    1 +
>>  qapi-schema.json |    6 +++-
>>  qemu-options.hx  |   14 +++++++++
>>  qmp.c            |    3 +-
>>  vl.c             |   17 ++++++++++-
>>  9 files changed, 137 insertions(+), 3 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 9b73ccf..b5b0531 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -19,6 +19,7 @@
>>  #include <stdarg.h>
>>  
>>  #include <linux/kvm.h>
>> +#include <linux/kvm_para.h>
>>  
>>  #include "qemu-common.h"
>>  #include "qemu-barrier.h"
>> @@ -29,6 +30,8 @@
>>  #include "bswap.h"
>>  #include "memory.h"
>>  #include "exec-memory.h"
>> +#include "iorange.h"
>> +#include "qemu-objects.h"
>>  
>>  /* This check must be after config-host.h is included */
>>  #ifdef CONFIG_EVENTFD
>> @@ -1707,3 +1710,84 @@ int kvm_on_sigbus(int code, void *addr)
>>  {
>>      return kvm_arch_on_sigbus(code, addr);
>>  }
>> +
>> +/* Possible values for action parameter. */
>> +#define PANICKED_REPORT 1   /* emit QEVENT_GUEST_PANICKED only */
>> +#define PANICKED_PAUSE  2   /* emit QEVENT_GUEST_PANICKED and pause VM */
>> +#define PANICKED_QUIT   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
>> +
>> +static int panicked_action = PANICKED_REPORT;
>> +
>> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
>> +                             uint64_t *data)
>> +{
>> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
>> +}
>> +
>> +static void panicked_mon_event(const char *action)
>> +{
>> +    QObject *data;
>> +
>> +    data = qobject_from_jsonf("{ 'action': %s }", action);
>> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
>> +    qobject_decref(data);
>> +}
>> +
>> +static void panicked_perform_action(void)
>> +{
>> +    switch(panicked_action) {
>> +    case PANICKED_REPORT:
>> +        panicked_mon_event("report");
>> +        break;
>> +
>> +    case PANICKED_PAUSE:
>> +        panicked_mon_event("pause");
>> +        vm_stop(RUN_STATE_GUEST_PANICKED);
>> +        break;
>> +
>> +    case PANICKED_QUIT:
>> +        panicked_mon_event("quit");
>> +        exit(0);
>> +        break;
>> +    }
> 
> Having the data argument is not needed/wanted. The mngt app can guess it if it
> needs to know it, but I think it doesn't want to.

Libvirt will do something when the kernel is panicked, so it should know the action
in qemu side.

Thanks
Wen Congyang

> 
>> +}
>> +
>> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
>> +                              uint64_t data)
>> +{
>> +    if (data == KVM_PV_PANICKED)
>> +        panicked_perform_action();
>> +}
>> +
>> +static void kvm_pv_port_destructor(IORange *iorange)
>> +{
>> +    g_free(iorange);
>> +}
>> +
>> +static IORangeOps pv_io_range_ops = {
>> +    .read = kvm_pv_port_read,
>> +    .write = kvm_pv_port_write,
>> +    .destructor = kvm_pv_port_destructor,
>> +};
>> +
>> +void kvm_pv_port_init(void)
>> +{
>> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
>> +
>> +    iorange_init(pv_io_range, &pv_io_range_ops, 0x505, 1);
>> +    ioport_register(pv_io_range);
>> +}
>> +
>> +int select_panicked_action(const char *p)
>> +{
>> +    if (strcasecmp(p, "report") == 0)
>> +        panicked_action = PANICKED_REPORT;
>> +    else if (strcasecmp(p, "pause") == 0)
>> +        panicked_action = PANICKED_PAUSE;
>> +    else if (strcasecmp(p, "quit") == 0)
>> +        panicked_action = PANICKED_QUIT;
>> +    else
>> +        return -1;
>> +
>> +    return 0;
>> +}
>> diff --git a/kvm-stub.c b/kvm-stub.c
>> index 47c573d..4cf977e 100644
>> --- a/kvm-stub.c
>> +++ b/kvm-stub.c
>> @@ -128,3 +128,12 @@ int kvm_on_sigbus(int code, void *addr)
>>  {
>>      return 1;
>>  }
>> +
>> +void kvm_pv_port_init(void)
>> +{
>> +}
>> +
>> +int select_panicked_action(const char *p)
>> +{
>> +    return -1;
>> +}
>> diff --git a/kvm.h b/kvm.h
>> index 4ccae8c..95075cf 100644
>> --- a/kvm.h
>> +++ b/kvm.h
>> @@ -60,6 +60,9 @@ int kvm_has_gsi_routing(void);
>>  
>>  int kvm_allows_irq0_override(void);
>>  
>> +void kvm_pv_port_init(void);
>> +int select_panicked_action(const char *p);
>> +
>>  #ifdef NEED_CPU_H
>>  int kvm_init_vcpu(CPUArchState *env);
>>  
>> diff --git a/monitor.c b/monitor.c
>> index 12a6fe2..83cb059 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -493,6 +493,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
>>          case QEVENT_WAKEUP:
>>              event_name = "WAKEUP";
>>              break;
>> +        case QEVENT_GUEST_PANICKED:
>> +            event_name = "GUEST_PANICKED";
>> +            break;
>>          default:
>>              abort();
>>              break;
>> diff --git a/monitor.h b/monitor.h
>> index 0d49800..94e8a3c 100644
>> --- a/monitor.h
>> +++ b/monitor.h
>> @@ -41,6 +41,7 @@ typedef enum MonitorEvent {
>>      QEVENT_DEVICE_TRAY_MOVED,
>>      QEVENT_SUSPEND,
>>      QEVENT_WAKEUP,
>> +    QEVENT_GUEST_PANICKED,
>>      QEVENT_MAX,
>>  } MonitorEvent;
>>  
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 2ca7195..ee5c9e9 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -119,11 +119,15 @@
>>  # @suspended: guest is suspended (ACPI S3)
>>  #
>>  # @watchdog: the watchdog action is configured to pause and has been triggered
>> +#
>> +# @guest-panicked: the panicked action is configured to pause and has been
>> +# triggered.
>>  ##
>>  { 'enum': 'RunState',
>>    'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
>>              'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
>> -            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
>> +            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
>> +            'guest-panicked' ] }
>>  
>>  ##
>>  # @StatusInfo:
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 8b66264..d3d21a1 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -2743,6 +2743,20 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log,
>>      "-qtest-log LOG  specify tracing options\n",
>>      QEMU_ARCH_ALL)
>>  
>> +DEF("onpanic", HAS_ARG, QEMU_OPTION_onpanic, \
>> +    "-onpanic report|pause|quit\n" \
>> +    "                action when the guest is panicked [default=report]",
>> +    QEMU_ARCH_ALL)
>> +STEXI
>> +@item -onpanic @var{action}
>> +
>> +The @var{action} controls what QEmu will do when the guest is panicked.
>> +The default is @code{report} (emit QEVENT_GUEST_PANICKED only).
>> +Other possible actions are:
>> +@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
>> +@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).
>> +ETEXI
>> +
>>  HXCOMM This is the last statement. Insert new options before this line!
>>  STEXI
>>  @end table
>> diff --git a/qmp.c b/qmp.c
>> index fee9fb2..a2d5ce9 100644
>> --- a/qmp.c
>> +++ b/qmp.c
>> @@ -148,7 +148,8 @@ void qmp_cont(Error **errp)
>>          error_set(errp, QERR_MIGRATION_EXPECTED);
>>          return;
>>      } else if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
>> -               runstate_check(RUN_STATE_SHUTDOWN)) {
>> +               runstate_check(RUN_STATE_SHUTDOWN) ||
>> +               runstate_check(RUN_STATE_GUEST_PANICKED)) {
>>          error_set(errp, QERR_RESET_REQUIRED);
>>          return;
>>      } else if (runstate_check(RUN_STATE_SUSPENDED)) {
>> diff --git a/vl.c b/vl.c
>> index 7f5fed8..8848506 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -367,6 +367,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>>      { RUN_STATE_RUNNING, RUN_STATE_SAVE_VM },
>>      { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN },
>>      { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
>> +    { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
>>  
>>      { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
>>  
>> @@ -381,6 +382,9 @@ static const RunStateTransition runstate_transitions_def[] = {
>>      { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
>>      { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
>>  
>> +    { RUN_STATE_GUEST_PANICKED, RUN_STATE_PAUSED },
>> +    { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
>> +
>>      { RUN_STATE_MAX, RUN_STATE_MAX },
>>  };
>>  
>> @@ -1537,7 +1541,8 @@ static bool main_loop_should_exit(void)
>>          qemu_system_reset(VMRESET_REPORT);
>>          resume_all_vcpus();
>>          if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
>> -            runstate_check(RUN_STATE_SHUTDOWN)) {
>> +            runstate_check(RUN_STATE_SHUTDOWN) ||
>> +            runstate_check(RUN_STATE_GUEST_PANICKED)) {
>>              runstate_set(RUN_STATE_PAUSED);
>>              vm_start();
>>          }
>> @@ -3202,6 +3207,12 @@ int main(int argc, char **argv, char **envp)
>>              case QEMU_OPTION_qtest_log:
>>                  qtest_log = optarg;
>>                  break;
>> +            case QEMU_OPTION_onpanic:
>> +                if (select_panicked_action(optarg) == -1) {
>> +                    fprintf(stderr, "Unknown -onpanic parameter\n");
>> +                    exit(1);
>> +                }
>> +                break;
>>              default:
>>                  os_parse_cmd_args(popt->index, optarg);
>>              }
>> @@ -3634,6 +3645,10 @@ int main(int argc, char **argv, char **envp)
>>          }
>>      }
>>  
>> +    if (kvm_enabled()) {
>> +        kvm_pv_port_init();
>> +    }
>> +
>>      if (incoming) {
>>          Error *errp = NULL;
>>          int ret = qemu_start_incoming_migration(incoming, &errp);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] deal with guest panicked event
@ 2012-06-12  6:55       ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-12  6:55 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: Gleb Natapov, kvm list, Jan Kiszka, qemu-devel, linux-kernel,
	Avi Kivity, KAMEZAWA Hiroyuki

At 05/31/2012 03:23 AM, Luiz Capitulino Wrote:
> On Mon, 21 May 2012 14:50:51 +0800
> Wen Congyang <wency@cn.fujitsu.com> wrote:
> 
>> When the guest is panicked, it will write 0x1 to the port 0x505. So if
>> qemu reads 0x1 from this port, we can do the folloing three things
>> according to the parameter -onpanic:
>> 1. emit QEVENT_GUEST_PANICKED only
>> 2. emit QEVENT_GUEST_PANICKED and pause VM
>> 3. emit QEVENT_GUEST_PANICKED and quit VM
>>
>> Note: if we emit QEVENT_GUEST_PANICKED only, and the management
>> application does not receive this event(the management may not
>> run when the event is emitted), the management won't know the
>> guest is panicked.
> 
> It will if it checks the vm status.
> 
> Btw, please, split this further into a patch adding the event, another one
> adding the new runstate and then the rest. One more comment below.

OK, I will split it.

> 
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  kvm-all.c        |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  kvm-stub.c       |    9 ++++++
>>  kvm.h            |    3 ++
>>  monitor.c        |    3 ++
>>  monitor.h        |    1 +
>>  qapi-schema.json |    6 +++-
>>  qemu-options.hx  |   14 +++++++++
>>  qmp.c            |    3 +-
>>  vl.c             |   17 ++++++++++-
>>  9 files changed, 137 insertions(+), 3 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 9b73ccf..b5b0531 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -19,6 +19,7 @@
>>  #include <stdarg.h>
>>  
>>  #include <linux/kvm.h>
>> +#include <linux/kvm_para.h>
>>  
>>  #include "qemu-common.h"
>>  #include "qemu-barrier.h"
>> @@ -29,6 +30,8 @@
>>  #include "bswap.h"
>>  #include "memory.h"
>>  #include "exec-memory.h"
>> +#include "iorange.h"
>> +#include "qemu-objects.h"
>>  
>>  /* This check must be after config-host.h is included */
>>  #ifdef CONFIG_EVENTFD
>> @@ -1707,3 +1710,84 @@ int kvm_on_sigbus(int code, void *addr)
>>  {
>>      return kvm_arch_on_sigbus(code, addr);
>>  }
>> +
>> +/* Possible values for action parameter. */
>> +#define PANICKED_REPORT 1   /* emit QEVENT_GUEST_PANICKED only */
>> +#define PANICKED_PAUSE  2   /* emit QEVENT_GUEST_PANICKED and pause VM */
>> +#define PANICKED_QUIT   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
>> +
>> +static int panicked_action = PANICKED_REPORT;
>> +
>> +static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
>> +                             uint64_t *data)
>> +{
>> +    *data = (1 << KVM_PV_FEATURE_PANICKED);
>> +}
>> +
>> +static void panicked_mon_event(const char *action)
>> +{
>> +    QObject *data;
>> +
>> +    data = qobject_from_jsonf("{ 'action': %s }", action);
>> +    monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
>> +    qobject_decref(data);
>> +}
>> +
>> +static void panicked_perform_action(void)
>> +{
>> +    switch(panicked_action) {
>> +    case PANICKED_REPORT:
>> +        panicked_mon_event("report");
>> +        break;
>> +
>> +    case PANICKED_PAUSE:
>> +        panicked_mon_event("pause");
>> +        vm_stop(RUN_STATE_GUEST_PANICKED);
>> +        break;
>> +
>> +    case PANICKED_QUIT:
>> +        panicked_mon_event("quit");
>> +        exit(0);
>> +        break;
>> +    }
> 
> Having the data argument is not needed/wanted. The mngt app can guess it if it
> needs to know it, but I think it doesn't want to.

Libvirt will do something when the kernel is panicked, so it should know the action
in qemu side.

Thanks
Wen Congyang

> 
>> +}
>> +
>> +static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned width,
>> +                              uint64_t data)
>> +{
>> +    if (data == KVM_PV_PANICKED)
>> +        panicked_perform_action();
>> +}
>> +
>> +static void kvm_pv_port_destructor(IORange *iorange)
>> +{
>> +    g_free(iorange);
>> +}
>> +
>> +static IORangeOps pv_io_range_ops = {
>> +    .read = kvm_pv_port_read,
>> +    .write = kvm_pv_port_write,
>> +    .destructor = kvm_pv_port_destructor,
>> +};
>> +
>> +void kvm_pv_port_init(void)
>> +{
>> +    IORange *pv_io_range = g_malloc(sizeof(IORange));
>> +
>> +    iorange_init(pv_io_range, &pv_io_range_ops, 0x505, 1);
>> +    ioport_register(pv_io_range);
>> +}
>> +
>> +int select_panicked_action(const char *p)
>> +{
>> +    if (strcasecmp(p, "report") == 0)
>> +        panicked_action = PANICKED_REPORT;
>> +    else if (strcasecmp(p, "pause") == 0)
>> +        panicked_action = PANICKED_PAUSE;
>> +    else if (strcasecmp(p, "quit") == 0)
>> +        panicked_action = PANICKED_QUIT;
>> +    else
>> +        return -1;
>> +
>> +    return 0;
>> +}
>> diff --git a/kvm-stub.c b/kvm-stub.c
>> index 47c573d..4cf977e 100644
>> --- a/kvm-stub.c
>> +++ b/kvm-stub.c
>> @@ -128,3 +128,12 @@ int kvm_on_sigbus(int code, void *addr)
>>  {
>>      return 1;
>>  }
>> +
>> +void kvm_pv_port_init(void)
>> +{
>> +}
>> +
>> +int select_panicked_action(const char *p)
>> +{
>> +    return -1;
>> +}
>> diff --git a/kvm.h b/kvm.h
>> index 4ccae8c..95075cf 100644
>> --- a/kvm.h
>> +++ b/kvm.h
>> @@ -60,6 +60,9 @@ int kvm_has_gsi_routing(void);
>>  
>>  int kvm_allows_irq0_override(void);
>>  
>> +void kvm_pv_port_init(void);
>> +int select_panicked_action(const char *p);
>> +
>>  #ifdef NEED_CPU_H
>>  int kvm_init_vcpu(CPUArchState *env);
>>  
>> diff --git a/monitor.c b/monitor.c
>> index 12a6fe2..83cb059 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -493,6 +493,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
>>          case QEVENT_WAKEUP:
>>              event_name = "WAKEUP";
>>              break;
>> +        case QEVENT_GUEST_PANICKED:
>> +            event_name = "GUEST_PANICKED";
>> +            break;
>>          default:
>>              abort();
>>              break;
>> diff --git a/monitor.h b/monitor.h
>> index 0d49800..94e8a3c 100644
>> --- a/monitor.h
>> +++ b/monitor.h
>> @@ -41,6 +41,7 @@ typedef enum MonitorEvent {
>>      QEVENT_DEVICE_TRAY_MOVED,
>>      QEVENT_SUSPEND,
>>      QEVENT_WAKEUP,
>> +    QEVENT_GUEST_PANICKED,
>>      QEVENT_MAX,
>>  } MonitorEvent;
>>  
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 2ca7195..ee5c9e9 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -119,11 +119,15 @@
>>  # @suspended: guest is suspended (ACPI S3)
>>  #
>>  # @watchdog: the watchdog action is configured to pause and has been triggered
>> +#
>> +# @guest-panicked: the panicked action is configured to pause and has been
>> +# triggered.
>>  ##
>>  { 'enum': 'RunState',
>>    'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
>>              'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
>> -            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
>> +            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
>> +            'guest-panicked' ] }
>>  
>>  ##
>>  # @StatusInfo:
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 8b66264..d3d21a1 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -2743,6 +2743,20 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log,
>>      "-qtest-log LOG  specify tracing options\n",
>>      QEMU_ARCH_ALL)
>>  
>> +DEF("onpanic", HAS_ARG, QEMU_OPTION_onpanic, \
>> +    "-onpanic report|pause|quit\n" \
>> +    "                action when the guest is panicked [default=report]",
>> +    QEMU_ARCH_ALL)
>> +STEXI
>> +@item -onpanic @var{action}
>> +
>> +The @var{action} controls what QEmu will do when the guest is panicked.
>> +The default is @code{report} (emit QEVENT_GUEST_PANICKED only).
>> +Other possible actions are:
>> +@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
>> +@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).
>> +ETEXI
>> +
>>  HXCOMM This is the last statement. Insert new options before this line!
>>  STEXI
>>  @end table
>> diff --git a/qmp.c b/qmp.c
>> index fee9fb2..a2d5ce9 100644
>> --- a/qmp.c
>> +++ b/qmp.c
>> @@ -148,7 +148,8 @@ void qmp_cont(Error **errp)
>>          error_set(errp, QERR_MIGRATION_EXPECTED);
>>          return;
>>      } else if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
>> -               runstate_check(RUN_STATE_SHUTDOWN)) {
>> +               runstate_check(RUN_STATE_SHUTDOWN) ||
>> +               runstate_check(RUN_STATE_GUEST_PANICKED)) {
>>          error_set(errp, QERR_RESET_REQUIRED);
>>          return;
>>      } else if (runstate_check(RUN_STATE_SUSPENDED)) {
>> diff --git a/vl.c b/vl.c
>> index 7f5fed8..8848506 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -367,6 +367,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>>      { RUN_STATE_RUNNING, RUN_STATE_SAVE_VM },
>>      { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN },
>>      { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
>> +    { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
>>  
>>      { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
>>  
>> @@ -381,6 +382,9 @@ static const RunStateTransition runstate_transitions_def[] = {
>>      { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
>>      { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
>>  
>> +    { RUN_STATE_GUEST_PANICKED, RUN_STATE_PAUSED },
>> +    { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
>> +
>>      { RUN_STATE_MAX, RUN_STATE_MAX },
>>  };
>>  
>> @@ -1537,7 +1541,8 @@ static bool main_loop_should_exit(void)
>>          qemu_system_reset(VMRESET_REPORT);
>>          resume_all_vcpus();
>>          if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
>> -            runstate_check(RUN_STATE_SHUTDOWN)) {
>> +            runstate_check(RUN_STATE_SHUTDOWN) ||
>> +            runstate_check(RUN_STATE_GUEST_PANICKED)) {
>>              runstate_set(RUN_STATE_PAUSED);
>>              vm_start();
>>          }
>> @@ -3202,6 +3207,12 @@ int main(int argc, char **argv, char **envp)
>>              case QEMU_OPTION_qtest_log:
>>                  qtest_log = optarg;
>>                  break;
>> +            case QEMU_OPTION_onpanic:
>> +                if (select_panicked_action(optarg) == -1) {
>> +                    fprintf(stderr, "Unknown -onpanic parameter\n");
>> +                    exit(1);
>> +                }
>> +                break;
>>              default:
>>                  os_parse_cmd_args(popt->index, optarg);
>>              }
>> @@ -3634,6 +3645,10 @@ int main(int argc, char **argv, char **envp)
>>          }
>>      }
>>  
>> +    if (kvm_enabled()) {
>> +        kvm_pv_port_init();
>> +    }
>> +
>>      if (incoming) {
>>          Error *errp = NULL;
>>          int ret = qemu_start_incoming_migration(incoming, &errp);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] start vm after reseting it
  2012-05-30 19:17     ` Luiz Capitulino
@ 2012-06-12  7:23       ` Wen Congyang
  -1 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-12  7:23 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

At 05/31/2012 03:17 AM, Luiz Capitulino Wrote:
> On Mon, 21 May 2012 14:49:32 +0800
> Wen Congyang <wency@cn.fujitsu.com> wrote:
> 
>> The guest should run after reseting it, but it does not
>> run if its old state is RUN_STATE_INTERNAL_ERROR or RUN_STATE_PAUSED.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  vl.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 23ab3a3..7f5fed8 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1539,6 +1539,7 @@ static bool main_loop_should_exit(void)
>>          if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
>>              runstate_check(RUN_STATE_SHUTDOWN)) {
>>              runstate_set(RUN_STATE_PAUSED);
>> +            vm_start();
> 
> Please, drop the runstate_set() call. I think you also have to
> to call bdrv_iostatus_reset(), as qmp_cont() does.

I am not sure whether we should call it here. I think you may be right,
so I will do it as you suggested.

Thanks
Wen Congyang

> 
>>          }
>>      }
>>      if (qemu_powerdown_requested()) {
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] start vm after reseting it
@ 2012-06-12  7:23       ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-12  7:23 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: Gleb Natapov, kvm list, Jan Kiszka, qemu-devel, linux-kernel,
	Avi Kivity, KAMEZAWA Hiroyuki

At 05/31/2012 03:17 AM, Luiz Capitulino Wrote:
> On Mon, 21 May 2012 14:49:32 +0800
> Wen Congyang <wency@cn.fujitsu.com> wrote:
> 
>> The guest should run after reseting it, but it does not
>> run if its old state is RUN_STATE_INTERNAL_ERROR or RUN_STATE_PAUSED.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  vl.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 23ab3a3..7f5fed8 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1539,6 +1539,7 @@ static bool main_loop_should_exit(void)
>>          if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
>>              runstate_check(RUN_STATE_SHUTDOWN)) {
>>              runstate_set(RUN_STATE_PAUSED);
>> +            vm_start();
> 
> Please, drop the runstate_set() call. I think you also have to
> to call bdrv_iostatus_reset(), as qmp_cont() does.

I am not sure whether we should call it here. I think you may be right,
so I will do it as you suggested.

Thanks
Wen Congyang

> 
>>          }
>>      }
>>      if (qemu_powerdown_requested()) {
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH] kvm: notify host when guest panicked
  2012-06-12  6:47   ` [Qemu-devel] " Wen Congyang
@ 2012-06-12  7:49     ` Christian Borntraeger
  -1 siblings, 0 replies; 50+ messages in thread
From: Christian Borntraeger @ 2012-06-12  7:49 UTC (permalink / raw)
  To: Wen Congyang
  Cc: kvm list, Avi Kivity, qemu-devel, linux-kernel,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

>> We have three solutions to implement this feature:
>> 1. use vmcall
>> 2. use I/O port
>> 3. use virtio-serial.
> 
>> We have decided to avoid touching hypervisor. The reason why I choose
>> choose the I/O port is:
>> 1. it is easier to implememt
>> 2. it does not depend any virtual device
>> 3. it can work when startint the kernel

Havent looked deeply into that, butz wouldnt kvm_hypercall0 also fulfill all of these
points? You just have to define the number for each architecture and then it should be 
possible to write a simple panic driver that works on all architectures.

>> --- a/include/linux/kvm_para.h
>> +++ b/include/linux/kvm_para.h
>> @@ -20,6 +20,14 @@
>>  #define KVM_HC_FEATURES			3
>>  #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
>>  
>> +#define KVM_PV_PORT	(0x505UL)
>> +

ignoring the above, shouldnt a port number go into an architecture specific file?


Christian


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH] kvm: notify host when guest panicked
@ 2012-06-12  7:49     ` Christian Borntraeger
  0 siblings, 0 replies; 50+ messages in thread
From: Christian Borntraeger @ 2012-06-12  7:49 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, Jan Kiszka, linux-kernel, qemu-devel,
	Avi Kivity, KAMEZAWA Hiroyuki

>> We have three solutions to implement this feature:
>> 1. use vmcall
>> 2. use I/O port
>> 3. use virtio-serial.
> 
>> We have decided to avoid touching hypervisor. The reason why I choose
>> choose the I/O port is:
>> 1. it is easier to implememt
>> 2. it does not depend any virtual device
>> 3. it can work when startint the kernel

Havent looked deeply into that, butz wouldnt kvm_hypercall0 also fulfill all of these
points? You just have to define the number for each architecture and then it should be 
possible to write a simple panic driver that works on all architectures.

>> --- a/include/linux/kvm_para.h
>> +++ b/include/linux/kvm_para.h
>> @@ -20,6 +20,14 @@
>>  #define KVM_HC_FEATURES			3
>>  #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
>>  
>> +#define KVM_PV_PORT	(0x505UL)
>> +

ignoring the above, shouldnt a port number go into an architecture specific file?


Christian

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH] kvm: notify host when guest panicked
  2012-06-12  7:49     ` [Qemu-devel] " Christian Borntraeger
@ 2012-06-12  8:07       ` Wen Congyang
  -1 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-12  8:07 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: kvm list, Avi Kivity, qemu-devel, linux-kernel,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

At 06/12/2012 03:49 PM, Christian Borntraeger Wrote:
>>> We have three solutions to implement this feature:
>>> 1. use vmcall
>>> 2. use I/O port
>>> 3. use virtio-serial.
>>
>>> We have decided to avoid touching hypervisor. The reason why I choose
>>> choose the I/O port is:
>>> 1. it is easier to implememt
>>> 2. it does not depend any virtual device
>>> 3. it can work when startint the kernel
> 
> Havent looked deeply into that, butz wouldnt kvm_hypercall0 also fulfill all of these
> points? You just have to define the number for each architecture and then it should be 
> possible to write a simple panic driver that works on all architectures.

kvm_hypercall0? Avi said that we should avoid touching hypercall.

Thanks
Wen Congyang

> 
>>> --- a/include/linux/kvm_para.h
>>> +++ b/include/linux/kvm_para.h
>>> @@ -20,6 +20,14 @@
>>>  #define KVM_HC_FEATURES			3
>>>  #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
>>>  
>>> +#define KVM_PV_PORT	(0x505UL)
>>> +
> 
> ignoring the above, shouldnt a port number go into an architecture specific file?
> 
> 
> Christian
> 
> 


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH] kvm: notify host when guest panicked
@ 2012-06-12  8:07       ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-12  8:07 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: Gleb Natapov, kvm list, Jan Kiszka, linux-kernel, qemu-devel,
	Avi Kivity, KAMEZAWA Hiroyuki

At 06/12/2012 03:49 PM, Christian Borntraeger Wrote:
>>> We have three solutions to implement this feature:
>>> 1. use vmcall
>>> 2. use I/O port
>>> 3. use virtio-serial.
>>
>>> We have decided to avoid touching hypervisor. The reason why I choose
>>> choose the I/O port is:
>>> 1. it is easier to implememt
>>> 2. it does not depend any virtual device
>>> 3. it can work when startint the kernel
> 
> Havent looked deeply into that, butz wouldnt kvm_hypercall0 also fulfill all of these
> points? You just have to define the number for each architecture and then it should be 
> possible to write a simple panic driver that works on all architectures.

kvm_hypercall0? Avi said that we should avoid touching hypercall.

Thanks
Wen Congyang

> 
>>> --- a/include/linux/kvm_para.h
>>> +++ b/include/linux/kvm_para.h
>>> @@ -20,6 +20,14 @@
>>>  #define KVM_HC_FEATURES			3
>>>  #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
>>>  
>>> +#define KVM_PV_PORT	(0x505UL)
>>> +
> 
> ignoring the above, shouldnt a port number go into an architecture specific file?
> 
> 
> Christian
> 
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH] kvm: notify host when guest panicked
  2012-06-12  7:49     ` [Qemu-devel] " Christian Borntraeger
  (?)
@ 2012-06-12  8:26       ` Wen Congyang
  -1 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-12  8:26 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: kvm list, Avi Kivity, qemu-devel, linux-kernel,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

At 06/12/2012 03:49 PM, Christian Borntraeger Wrote:
>>> We have three solutions to implement this feature:
>>> 1. use vmcall
>>> 2. use I/O port
>>> 3. use virtio-serial.
>>
>>> We have decided to avoid touching hypervisor. The reason why I choose
>>> choose the I/O port is:
>>> 1. it is easier to implememt
>>> 2. it does not depend any virtual device
>>> 3. it can work when startint the kernel
> 
> Havent looked deeply into that, butz wouldnt kvm_hypercall0 also fulfill all of these
> points? You just have to define the number for each architecture and then it should be 
> possible to write a simple panic driver that works on all architectures.
> 
>>> --- a/include/linux/kvm_para.h
>>> +++ b/include/linux/kvm_para.h
>>> @@ -20,6 +20,14 @@
>>>  #define KVM_HC_FEATURES			3
>>>  #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
>>>  
>>> +#define KVM_PV_PORT	(0x505UL)
>>> +
> 
> ignoring the above, shouldnt a port number go into an architecture specific file?

I am not sure whether it should be put into an architecture specific file.

Thanks
Wen Congyang

> 
> 
> Christian
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH] kvm: notify host when guest panicked
@ 2012-06-12  8:26       ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-12  8:26 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: Gleb Natapov, kvm list, Jan Kiszka, linux-kernel, qemu-devel,
	Avi Kivity, KAMEZAWA Hiroyuki

At 06/12/2012 03:49 PM, Christian Borntraeger Wrote:
>>> We have three solutions to implement this feature:
>>> 1. use vmcall
>>> 2. use I/O port
>>> 3. use virtio-serial.
>>
>>> We have decided to avoid touching hypervisor. The reason why I choose
>>> choose the I/O port is:
>>> 1. it is easier to implememt
>>> 2. it does not depend any virtual device
>>> 3. it can work when startint the kernel
> 
> Havent looked deeply into that, butz wouldnt kvm_hypercall0 also fulfill all of these
> points? You just have to define the number for each architecture and then it should be 
> possible to write a simple panic driver that works on all architectures.
> 
>>> --- a/include/linux/kvm_para.h
>>> +++ b/include/linux/kvm_para.h
>>> @@ -20,6 +20,14 @@
>>>  #define KVM_HC_FEATURES			3
>>>  #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
>>>  
>>> +#define KVM_PV_PORT	(0x505UL)
>>> +
> 
> ignoring the above, shouldnt a port number go into an architecture specific file?

I am not sure whether it should be put into an architecture specific file.

Thanks
Wen Congyang

> 
> 
> Christian
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH] kvm: notify host when guest panicked
@ 2012-06-12  8:26       ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-12  8:26 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: Gleb Natapov, kvm list, Jan Kiszka, linux-kernel, qemu-devel,
	Avi Kivity, KAMEZAWA Hiroyuki

At 06/12/2012 03:49 PM, Christian Borntraeger Wrote:
>>> We have three solutions to implement this feature:
>>> 1. use vmcall
>>> 2. use I/O port
>>> 3. use virtio-serial.
>>
>>> We have decided to avoid touching hypervisor. The reason why I choose
>>> choose the I/O port is:
>>> 1. it is easier to implememt
>>> 2. it does not depend any virtual device
>>> 3. it can work when startint the kernel
> 
> Havent looked deeply into that, butz wouldnt kvm_hypercall0 also fulfill all of these
> points? You just have to define the number for each architecture and then it should be 
> possible to write a simple panic driver that works on all architectures.
> 
>>> --- a/include/linux/kvm_para.h
>>> +++ b/include/linux/kvm_para.h
>>> @@ -20,6 +20,14 @@
>>>  #define KVM_HC_FEATURES			3
>>>  #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
>>>  
>>> +#define KVM_PV_PORT	(0x505UL)
>>> +
> 
> ignoring the above, shouldnt a port number go into an architecture specific file?

I am not sure whether it should be put into an architecture specific file.

Thanks
Wen Congyang

> 
> 
> Christian
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH] kvm: notify host when guest panicked
  2012-06-12  8:26       ` Wen Congyang
  (?)
@ 2012-06-12  9:49         ` Gleb Natapov
  -1 siblings, 0 replies; 50+ messages in thread
From: Gleb Natapov @ 2012-06-12  9:49 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Christian Borntraeger, kvm list, Avi Kivity, qemu-devel,
	linux-kernel, Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka

On Tue, Jun 12, 2012 at 04:26:35PM +0800, Wen Congyang wrote:
> At 06/12/2012 03:49 PM, Christian Borntraeger Wrote:
> >>> We have three solutions to implement this feature:
> >>> 1. use vmcall
> >>> 2. use I/O port
> >>> 3. use virtio-serial.
> >>
> >>> We have decided to avoid touching hypervisor. The reason why I choose
> >>> choose the I/O port is:
> >>> 1. it is easier to implememt
> >>> 2. it does not depend any virtual device
> >>> 3. it can work when startint the kernel
> > 
> > Havent looked deeply into that, butz wouldnt kvm_hypercall0 also fulfill all of these
> > points? You just have to define the number for each architecture and then it should be 
> > possible to write a simple panic driver that works on all architectures.
> > 
> >>> --- a/include/linux/kvm_para.h
> >>> +++ b/include/linux/kvm_para.h
> >>> @@ -20,6 +20,14 @@
> >>>  #define KVM_HC_FEATURES			3
> >>>  #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
> >>>  
> >>> +#define KVM_PV_PORT	(0x505UL)
> >>> +
> > 
> > ignoring the above, shouldnt a port number go into an architecture specific file?
> 
> I am not sure whether it should be put into an architecture specific file.
> 
Because PIO is x86 thing.

--
			Gleb.

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH] kvm: notify host when guest panicked
@ 2012-06-12  9:49         ` Gleb Natapov
  0 siblings, 0 replies; 50+ messages in thread
From: Gleb Natapov @ 2012-06-12  9:49 UTC (permalink / raw)
  To: Wen Congyang
  Cc: kvm list, Jan Kiszka, linux-kernel, qemu-devel,
	Christian Borntraeger, Avi Kivity, KAMEZAWA Hiroyuki

On Tue, Jun 12, 2012 at 04:26:35PM +0800, Wen Congyang wrote:
> At 06/12/2012 03:49 PM, Christian Borntraeger Wrote:
> >>> We have three solutions to implement this feature:
> >>> 1. use vmcall
> >>> 2. use I/O port
> >>> 3. use virtio-serial.
> >>
> >>> We have decided to avoid touching hypervisor. The reason why I choose
> >>> choose the I/O port is:
> >>> 1. it is easier to implememt
> >>> 2. it does not depend any virtual device
> >>> 3. it can work when startint the kernel
> > 
> > Havent looked deeply into that, butz wouldnt kvm_hypercall0 also fulfill all of these
> > points? You just have to define the number for each architecture and then it should be 
> > possible to write a simple panic driver that works on all architectures.
> > 
> >>> --- a/include/linux/kvm_para.h
> >>> +++ b/include/linux/kvm_para.h
> >>> @@ -20,6 +20,14 @@
> >>>  #define KVM_HC_FEATURES			3
> >>>  #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
> >>>  
> >>> +#define KVM_PV_PORT	(0x505UL)
> >>> +
> > 
> > ignoring the above, shouldnt a port number go into an architecture specific file?
> 
> I am not sure whether it should be put into an architecture specific file.
> 
Because PIO is x86 thing.

--
			Gleb.

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH] kvm: notify host when guest panicked
@ 2012-06-12  9:49         ` Gleb Natapov
  0 siblings, 0 replies; 50+ messages in thread
From: Gleb Natapov @ 2012-06-12  9:49 UTC (permalink / raw)
  To: Wen Congyang
  Cc: kvm list, Jan Kiszka, linux-kernel, qemu-devel,
	Christian Borntraeger, Avi Kivity, KAMEZAWA Hiroyuki

On Tue, Jun 12, 2012 at 04:26:35PM +0800, Wen Congyang wrote:
> At 06/12/2012 03:49 PM, Christian Borntraeger Wrote:
> >>> We have three solutions to implement this feature:
> >>> 1. use vmcall
> >>> 2. use I/O port
> >>> 3. use virtio-serial.
> >>
> >>> We have decided to avoid touching hypervisor. The reason why I choose
> >>> choose the I/O port is:
> >>> 1. it is easier to implememt
> >>> 2. it does not depend any virtual device
> >>> 3. it can work when startint the kernel
> > 
> > Havent looked deeply into that, butz wouldnt kvm_hypercall0 also fulfill all of these
> > points? You just have to define the number for each architecture and then it should be 
> > possible to write a simple panic driver that works on all architectures.
> > 
> >>> --- a/include/linux/kvm_para.h
> >>> +++ b/include/linux/kvm_para.h
> >>> @@ -20,6 +20,14 @@
> >>>  #define KVM_HC_FEATURES			3
> >>>  #define KVM_HC_PPC_MAP_MAGIC_PAGE	4
> >>>  
> >>> +#define KVM_PV_PORT	(0x505UL)
> >>> +
> > 
> > ignoring the above, shouldnt a port number go into an architecture specific file?
> 
> I am not sure whether it should be put into an architecture specific file.
> 
Because PIO is x86 thing.

--
			Gleb.

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] deal with guest panicked event
  2012-06-12  6:55       ` Wen Congyang
@ 2012-06-12 12:35         ` Luiz Capitulino
  -1 siblings, 0 replies; 50+ messages in thread
From: Luiz Capitulino @ 2012-06-12 12:35 UTC (permalink / raw)
  To: Wen Congyang
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

On Tue, 12 Jun 2012 14:55:37 +0800
Wen Congyang <wency@cn.fujitsu.com> wrote:

> >> +static void panicked_perform_action(void)
> >> +{
> >> +    switch(panicked_action) {
> >> +    case PANICKED_REPORT:
> >> +        panicked_mon_event("report");
> >> +        break;
> >> +
> >> +    case PANICKED_PAUSE:
> >> +        panicked_mon_event("pause");
> >> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> >> +        break;
> >> +
> >> +    case PANICKED_QUIT:
> >> +        panicked_mon_event("quit");
> >> +        exit(0);
> >> +        break;
> >> +    }
> > 
> > Having the data argument is not needed/wanted. The mngt app can guess it if it
> > needs to know it, but I think it doesn't want to.
> 
> Libvirt will do something when the kernel is panicked, so it should know the action
> in qemu side.

But the action will be set by libvirt itself, no?

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] deal with guest panicked event
@ 2012-06-12 12:35         ` Luiz Capitulino
  0 siblings, 0 replies; 50+ messages in thread
From: Luiz Capitulino @ 2012-06-12 12:35 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, Jan Kiszka, qemu-devel, linux-kernel,
	Avi Kivity, KAMEZAWA Hiroyuki

On Tue, 12 Jun 2012 14:55:37 +0800
Wen Congyang <wency@cn.fujitsu.com> wrote:

> >> +static void panicked_perform_action(void)
> >> +{
> >> +    switch(panicked_action) {
> >> +    case PANICKED_REPORT:
> >> +        panicked_mon_event("report");
> >> +        break;
> >> +
> >> +    case PANICKED_PAUSE:
> >> +        panicked_mon_event("pause");
> >> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> >> +        break;
> >> +
> >> +    case PANICKED_QUIT:
> >> +        panicked_mon_event("quit");
> >> +        exit(0);
> >> +        break;
> >> +    }
> > 
> > Having the data argument is not needed/wanted. The mngt app can guess it if it
> > needs to know it, but I think it doesn't want to.
> 
> Libvirt will do something when the kernel is panicked, so it should know the action
> in qemu side.

But the action will be set by libvirt itself, no?

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] deal with guest panicked event
  2012-06-12 12:35         ` Luiz Capitulino
@ 2012-06-12 12:40           ` Daniel P. Berrange
  -1 siblings, 0 replies; 50+ messages in thread
From: Daniel P. Berrange @ 2012-06-12 12:40 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: Wen Congyang, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

On Tue, Jun 12, 2012 at 09:35:04AM -0300, Luiz Capitulino wrote:
> On Tue, 12 Jun 2012 14:55:37 +0800
> Wen Congyang <wency@cn.fujitsu.com> wrote:
> 
> > >> +static void panicked_perform_action(void)
> > >> +{
> > >> +    switch(panicked_action) {
> > >> +    case PANICKED_REPORT:
> > >> +        panicked_mon_event("report");
> > >> +        break;
> > >> +
> > >> +    case PANICKED_PAUSE:
> > >> +        panicked_mon_event("pause");
> > >> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> > >> +        break;
> > >> +
> > >> +    case PANICKED_QUIT:
> > >> +        panicked_mon_event("quit");
> > >> +        exit(0);
> > >> +        break;
> > >> +    }
> > > 
> > > Having the data argument is not needed/wanted. The mngt app can guess it if it
> > > needs to know it, but I think it doesn't want to.
> > 
> > Libvirt will do something when the kernel is panicked, so it should know the action
> > in qemu side.
> 
> But the action will be set by libvirt itself, no?

Sure, but the whole world isn't libvirt. If the process listening to the
monitor is not the same as the process which launched the VM, then I
think including the action is worthwhile. Besides, the way Wen has done
this is identical to what we already do with QEVENT_WATCHDOG and I think
it is desirable to keep consistency here.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] deal with guest panicked event
@ 2012-06-12 12:40           ` Daniel P. Berrange
  0 siblings, 0 replies; 50+ messages in thread
From: Daniel P. Berrange @ 2012-06-12 12:40 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: kvm list, Jan Kiszka, linux-kernel, Gleb Natapov, qemu-devel,
	Avi Kivity, KAMEZAWA Hiroyuki

On Tue, Jun 12, 2012 at 09:35:04AM -0300, Luiz Capitulino wrote:
> On Tue, 12 Jun 2012 14:55:37 +0800
> Wen Congyang <wency@cn.fujitsu.com> wrote:
> 
> > >> +static void panicked_perform_action(void)
> > >> +{
> > >> +    switch(panicked_action) {
> > >> +    case PANICKED_REPORT:
> > >> +        panicked_mon_event("report");
> > >> +        break;
> > >> +
> > >> +    case PANICKED_PAUSE:
> > >> +        panicked_mon_event("pause");
> > >> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> > >> +        break;
> > >> +
> > >> +    case PANICKED_QUIT:
> > >> +        panicked_mon_event("quit");
> > >> +        exit(0);
> > >> +        break;
> > >> +    }
> > > 
> > > Having the data argument is not needed/wanted. The mngt app can guess it if it
> > > needs to know it, but I think it doesn't want to.
> > 
> > Libvirt will do something when the kernel is panicked, so it should know the action
> > in qemu side.
> 
> But the action will be set by libvirt itself, no?

Sure, but the whole world isn't libvirt. If the process listening to the
monitor is not the same as the process which launched the VM, then I
think including the action is worthwhile. Besides, the way Wen has done
this is identical to what we already do with QEVENT_WATCHDOG and I think
it is desirable to keep consistency here.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] deal with guest panicked event
  2012-06-12 12:40           ` Daniel P. Berrange
@ 2012-06-12 13:21             ` Luiz Capitulino
  -1 siblings, 0 replies; 50+ messages in thread
From: Luiz Capitulino @ 2012-06-12 13:21 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: Wen Congyang, kvm list, qemu-devel, linux-kernel, Avi Kivity,
	KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

On Tue, 12 Jun 2012 13:40:45 +0100
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Tue, Jun 12, 2012 at 09:35:04AM -0300, Luiz Capitulino wrote:
> > On Tue, 12 Jun 2012 14:55:37 +0800
> > Wen Congyang <wency@cn.fujitsu.com> wrote:
> > 
> > > >> +static void panicked_perform_action(void)
> > > >> +{
> > > >> +    switch(panicked_action) {
> > > >> +    case PANICKED_REPORT:
> > > >> +        panicked_mon_event("report");
> > > >> +        break;
> > > >> +
> > > >> +    case PANICKED_PAUSE:
> > > >> +        panicked_mon_event("pause");
> > > >> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> > > >> +        break;
> > > >> +
> > > >> +    case PANICKED_QUIT:
> > > >> +        panicked_mon_event("quit");
> > > >> +        exit(0);
> > > >> +        break;
> > > >> +    }
> > > > 
> > > > Having the data argument is not needed/wanted. The mngt app can guess it if it
> > > > needs to know it, but I think it doesn't want to.
> > > 
> > > Libvirt will do something when the kernel is panicked, so it should know the action
> > > in qemu side.
> > 
> > But the action will be set by libvirt itself, no?
> 
> Sure, but the whole world isn't libvirt. If the process listening to the
> monitor is not the same as the process which launched the VM, then I
> think including the action is worthwhile. Besides, the way Wen has done
> this is identical to what we already do with QEVENT_WATCHDOG and I think
> it is desirable to keep consistency here.

That's right, I had forgotten about the WATCHDOG event. Maybe it would
make more sense to have this info in a query- command though, specially if
we plan to have a command to change that setting.

But I won't oppose having it in the event.

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] deal with guest panicked event
@ 2012-06-12 13:21             ` Luiz Capitulino
  0 siblings, 0 replies; 50+ messages in thread
From: Luiz Capitulino @ 2012-06-12 13:21 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: kvm list, Jan Kiszka, linux-kernel, Gleb Natapov, qemu-devel,
	Avi Kivity, KAMEZAWA Hiroyuki

On Tue, 12 Jun 2012 13:40:45 +0100
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Tue, Jun 12, 2012 at 09:35:04AM -0300, Luiz Capitulino wrote:
> > On Tue, 12 Jun 2012 14:55:37 +0800
> > Wen Congyang <wency@cn.fujitsu.com> wrote:
> > 
> > > >> +static void panicked_perform_action(void)
> > > >> +{
> > > >> +    switch(panicked_action) {
> > > >> +    case PANICKED_REPORT:
> > > >> +        panicked_mon_event("report");
> > > >> +        break;
> > > >> +
> > > >> +    case PANICKED_PAUSE:
> > > >> +        panicked_mon_event("pause");
> > > >> +        vm_stop(RUN_STATE_GUEST_PANICKED);
> > > >> +        break;
> > > >> +
> > > >> +    case PANICKED_QUIT:
> > > >> +        panicked_mon_event("quit");
> > > >> +        exit(0);
> > > >> +        break;
> > > >> +    }
> > > > 
> > > > Having the data argument is not needed/wanted. The mngt app can guess it if it
> > > > needs to know it, but I think it doesn't want to.
> > > 
> > > Libvirt will do something when the kernel is panicked, so it should know the action
> > > in qemu side.
> > 
> > But the action will be set by libvirt itself, no?
> 
> Sure, but the whole world isn't libvirt. If the process listening to the
> monitor is not the same as the process which launched the VM, then I
> think including the action is worthwhile. Besides, the way Wen has done
> this is identical to what we already do with QEVENT_WATCHDOG and I think
> it is desirable to keep consistency here.

That's right, I had forgotten about the WATCHDOG event. Maybe it would
make more sense to have this info in a query- command though, specially if
we plan to have a command to change that setting.

But I won't oppose having it in the event.

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 3/3] deal with guest panicked event
  2012-05-21  6:50   ` [Qemu-devel] " Wen Congyang
@ 2012-06-12 13:29     ` Paolo Bonzini
  -1 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2012-06-12 13:29 UTC (permalink / raw)
  To: Wen Congyang
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

Il 21/05/2012 08:50, Wen Congyang ha scritto:
> +DEF("onpanic", HAS_ARG, QEMU_OPTION_onpanic, \
> +    "-onpanic report|pause|quit\n" \
> +    "                action when the guest is panicked [default=report]",
> +    QEMU_ARCH_ALL)
> +STEXI
> +@item -onpanic @var{action}
> +
> +The @var{action} controls what QEmu will do when the guest is panicked.
> +The default is @code{report} (emit QEVENT_GUEST_PANICKED only).

-watchdog-action just calls this "none".

> +Other possible actions are:
> +@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
> +@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).

Rather than just "quit", there should be choices for "shutdown",
"poweroff" and "reset" for consistency with -watchdog-action.

Also, this option should also be accessible from "-machine", please.

Paolo

> +ETEXI
> +
>  HXCOMM This is the last statement. Insert new options before this line!
>  STEXI
>  @end table

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] deal with guest panicked event
@ 2012-06-12 13:29     ` Paolo Bonzini
  0 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2012-06-12 13:29 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, Jan Kiszka, qemu-devel, linux-kernel,
	Avi Kivity, KAMEZAWA Hiroyuki

Il 21/05/2012 08:50, Wen Congyang ha scritto:
> +DEF("onpanic", HAS_ARG, QEMU_OPTION_onpanic, \
> +    "-onpanic report|pause|quit\n" \
> +    "                action when the guest is panicked [default=report]",
> +    QEMU_ARCH_ALL)
> +STEXI
> +@item -onpanic @var{action}
> +
> +The @var{action} controls what QEmu will do when the guest is panicked.
> +The default is @code{report} (emit QEVENT_GUEST_PANICKED only).

-watchdog-action just calls this "none".

> +Other possible actions are:
> +@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
> +@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).

Rather than just "quit", there should be choices for "shutdown",
"poweroff" and "reset" for consistency with -watchdog-action.

Also, this option should also be accessible from "-machine", please.

Paolo

> +ETEXI
> +
>  HXCOMM This is the last statement. Insert new options before this line!
>  STEXI
>  @end table

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] start vm after reseting it
  2012-05-30 19:17     ` Luiz Capitulino
  (?)
@ 2012-06-13  6:54       ` Wen Congyang
  -1 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-13  6:54 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

At 05/31/2012 03:17 AM, Luiz Capitulino Wrote:
> On Mon, 21 May 2012 14:49:32 +0800
> Wen Congyang <wency@cn.fujitsu.com> wrote:
> 
>> The guest should run after reseting it, but it does not
>> run if its old state is RUN_STATE_INTERNAL_ERROR or RUN_STATE_PAUSED.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  vl.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 23ab3a3..7f5fed8 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1539,6 +1539,7 @@ static bool main_loop_should_exit(void)
>>          if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
>>              runstate_check(RUN_STATE_SHUTDOWN)) {
>>              runstate_set(RUN_STATE_PAUSED);
>> +            vm_start();
> 
> Please, drop the runstate_set() call. I think you also have to
> to call bdrv_iostatus_reset(), as qmp_cont() does.

Hmm, if we drop the runstate_set() call here, we should update
runstate_transitions_def too.

Thanks
Wen Congyang

> 
>>          }
>>      }
>>      if (qemu_powerdown_requested()) {
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 1/3] start vm after reseting it
@ 2012-06-13  6:54       ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-13  6:54 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: Gleb Natapov, kvm list, Jan Kiszka, qemu-devel, linux-kernel,
	Avi Kivity, KAMEZAWA Hiroyuki

At 05/31/2012 03:17 AM, Luiz Capitulino Wrote:
> On Mon, 21 May 2012 14:49:32 +0800
> Wen Congyang <wency@cn.fujitsu.com> wrote:
> 
>> The guest should run after reseting it, but it does not
>> run if its old state is RUN_STATE_INTERNAL_ERROR or RUN_STATE_PAUSED.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  vl.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 23ab3a3..7f5fed8 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1539,6 +1539,7 @@ static bool main_loop_should_exit(void)
>>          if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
>>              runstate_check(RUN_STATE_SHUTDOWN)) {
>>              runstate_set(RUN_STATE_PAUSED);
>> +            vm_start();
> 
> Please, drop the runstate_set() call. I think you also have to
> to call bdrv_iostatus_reset(), as qmp_cont() does.

Hmm, if we drop the runstate_set() call here, we should update
runstate_transitions_def too.

Thanks
Wen Congyang

> 
>>          }
>>      }
>>      if (qemu_powerdown_requested()) {
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] start vm after reseting it
@ 2012-06-13  6:54       ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-13  6:54 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: Gleb Natapov, kvm list, Jan Kiszka, qemu-devel, linux-kernel,
	Avi Kivity, KAMEZAWA Hiroyuki

At 05/31/2012 03:17 AM, Luiz Capitulino Wrote:
> On Mon, 21 May 2012 14:49:32 +0800
> Wen Congyang <wency@cn.fujitsu.com> wrote:
> 
>> The guest should run after reseting it, but it does not
>> run if its old state is RUN_STATE_INTERNAL_ERROR or RUN_STATE_PAUSED.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  vl.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 23ab3a3..7f5fed8 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1539,6 +1539,7 @@ static bool main_loop_should_exit(void)
>>          if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
>>              runstate_check(RUN_STATE_SHUTDOWN)) {
>>              runstate_set(RUN_STATE_PAUSED);
>> +            vm_start();
> 
> Please, drop the runstate_set() call. I think you also have to
> to call bdrv_iostatus_reset(), as qmp_cont() does.

Hmm, if we drop the runstate_set() call here, we should update
runstate_transitions_def too.

Thanks
Wen Congyang

> 
>>          }
>>      }
>>      if (qemu_powerdown_requested()) {
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 3/3] deal with guest panicked event
  2012-06-12 13:29     ` [Qemu-devel] " Paolo Bonzini
@ 2012-06-13  7:02       ` Wen Congyang
  -1 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-13  7:02 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

At 06/12/2012 09:29 PM, Paolo Bonzini Wrote:
> Il 21/05/2012 08:50, Wen Congyang ha scritto:
>> +DEF("onpanic", HAS_ARG, QEMU_OPTION_onpanic, \
>> +    "-onpanic report|pause|quit\n" \
>> +    "                action when the guest is panicked [default=report]",
>> +    QEMU_ARCH_ALL)
>> +STEXI
>> +@item -onpanic @var{action}
>> +
>> +The @var{action} controls what QEmu will do when the guest is panicked.
>> +The default is @code{report} (emit QEVENT_GUEST_PANICKED only).
> 
> -watchdog-action just calls this "none".

Yes, I will change the action name.

> 
>> +Other possible actions are:
>> +@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
>> +@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).
> 
> Rather than just "quit", there should be choices for "shutdown",
> "poweroff" and "reset" for consistency with -watchdog-action.

Hmm, quit means poweroff. I will add reset, but I think we donot
need shutdown, because we cannot shutdown the system when it is
panicked.

> 
> Also, this option should also be accessible from "-machine", please.

I don't understand this. Do you mean we can also specify this action
in -machine option?

Thanks
Wen Congyang

> 
> Paolo
> 
>> +ETEXI
>> +
>>  HXCOMM This is the last statement. Insert new options before this line!
>>  STEXI
>>  @end table
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] deal with guest panicked event
@ 2012-06-13  7:02       ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-13  7:02 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Gleb Natapov, kvm list, Jan Kiszka, qemu-devel, linux-kernel,
	Avi Kivity, KAMEZAWA Hiroyuki

At 06/12/2012 09:29 PM, Paolo Bonzini Wrote:
> Il 21/05/2012 08:50, Wen Congyang ha scritto:
>> +DEF("onpanic", HAS_ARG, QEMU_OPTION_onpanic, \
>> +    "-onpanic report|pause|quit\n" \
>> +    "                action when the guest is panicked [default=report]",
>> +    QEMU_ARCH_ALL)
>> +STEXI
>> +@item -onpanic @var{action}
>> +
>> +The @var{action} controls what QEmu will do when the guest is panicked.
>> +The default is @code{report} (emit QEVENT_GUEST_PANICKED only).
> 
> -watchdog-action just calls this "none".

Yes, I will change the action name.

> 
>> +Other possible actions are:
>> +@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
>> +@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).
> 
> Rather than just "quit", there should be choices for "shutdown",
> "poweroff" and "reset" for consistency with -watchdog-action.

Hmm, quit means poweroff. I will add reset, but I think we donot
need shutdown, because we cannot shutdown the system when it is
panicked.

> 
> Also, this option should also be accessible from "-machine", please.

I don't understand this. Do you mean we can also specify this action
in -machine option?

Thanks
Wen Congyang

> 
> Paolo
> 
>> +ETEXI
>> +
>>  HXCOMM This is the last statement. Insert new options before this line!
>>  STEXI
>>  @end table
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 3/3] deal with guest panicked event
  2012-06-13  7:02       ` [Qemu-devel] " Wen Congyang
  (?)
@ 2012-06-13  7:53         ` Paolo Bonzini
  -1 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2012-06-13  7:53 UTC (permalink / raw)
  To: Wen Congyang
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

Il 13/06/2012 09:02, Wen Congyang ha scritto:
>>> >> +Other possible actions are:
>>> >> +@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
>>> >> +@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).
>> > 
>> > Rather than just "quit", there should be choices for "shutdown",
>> > "poweroff" and "reset" for consistency with -watchdog-action.
> Hmm, quit means poweroff. I will add reset, but I think we donot
> need shutdown, because we cannot shutdown the system when it is
> panicked.

Right. :)

Can you also rename quit to poweroff?

>> > Also, this option should also be accessible from "-machine", please.
> I don't understand this. Do you mean we can also specify this action
> in -machine option?

Yes, like -machine panic_action=poweroff.

Paolo

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 3/3] deal with guest panicked event
@ 2012-06-13  7:53         ` Paolo Bonzini
  0 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2012-06-13  7:53 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, Jan Kiszka, qemu-devel, linux-kernel,
	Avi Kivity, KAMEZAWA Hiroyuki

Il 13/06/2012 09:02, Wen Congyang ha scritto:
>>> >> +Other possible actions are:
>>> >> +@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
>>> >> +@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).
>> > 
>> > Rather than just "quit", there should be choices for "shutdown",
>> > "poweroff" and "reset" for consistency with -watchdog-action.
> Hmm, quit means poweroff. I will add reset, but I think we donot
> need shutdown, because we cannot shutdown the system when it is
> panicked.

Right. :)

Can you also rename quit to poweroff?

>> > Also, this option should also be accessible from "-machine", please.
> I don't understand this. Do you mean we can also specify this action
> in -machine option?

Yes, like -machine panic_action=poweroff.

Paolo

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] deal with guest panicked event
@ 2012-06-13  7:53         ` Paolo Bonzini
  0 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2012-06-13  7:53 UTC (permalink / raw)
  To: Wen Congyang
  Cc: Gleb Natapov, kvm list, Jan Kiszka, qemu-devel, linux-kernel,
	Avi Kivity, KAMEZAWA Hiroyuki

Il 13/06/2012 09:02, Wen Congyang ha scritto:
>>> >> +Other possible actions are:
>>> >> +@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
>>> >> +@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).
>> > 
>> > Rather than just "quit", there should be choices for "shutdown",
>> > "poweroff" and "reset" for consistency with -watchdog-action.
> Hmm, quit means poweroff. I will add reset, but I think we donot
> need shutdown, because we cannot shutdown the system when it is
> panicked.

Right. :)

Can you also rename quit to poweroff?

>> > Also, this option should also be accessible from "-machine", please.
> I don't understand this. Do you mean we can also specify this action
> in -machine option?

Yes, like -machine panic_action=poweroff.

Paolo

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 3/3] deal with guest panicked event
  2012-06-13  7:53         ` Paolo Bonzini
  (?)
@ 2012-06-13  8:00           ` Wen Congyang
  -1 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-13  8:00 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm list, qemu-devel, linux-kernel, Avi Kivity,
	Daniel P. Berrange, KAMEZAWA Hiroyuki, Jan Kiszka, Gleb Natapov

At 06/13/2012 03:53 PM, Paolo Bonzini Wrote:
> Il 13/06/2012 09:02, Wen Congyang ha scritto:
>>>>>> +Other possible actions are:
>>>>>> +@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
>>>>>> +@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).
>>>>
>>>> Rather than just "quit", there should be choices for "shutdown",
>>>> "poweroff" and "reset" for consistency with -watchdog-action.
>> Hmm, quit means poweroff. I will add reset, but I think we donot
>> need shutdown, because we cannot shutdown the system when it is
>> panicked.
> 
> Right. :)
> 
> Can you also rename quit to poweroff?

OK.

Thanks
Wen Congyang
> 
>>>> Also, this option should also be accessible from "-machine", please.
>> I don't understand this. Do you mean we can also specify this action
>> in -machine option?
> 
> Yes, like -machine panic_action=poweroff.
> 
> Paolo
> 


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH 3/3] deal with guest panicked event
@ 2012-06-13  8:00           ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-13  8:00 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Gleb Natapov, kvm list, Jan Kiszka, qemu-devel, linux-kernel,
	Avi Kivity, KAMEZAWA Hiroyuki

At 06/13/2012 03:53 PM, Paolo Bonzini Wrote:
> Il 13/06/2012 09:02, Wen Congyang ha scritto:
>>>>>> +Other possible actions are:
>>>>>> +@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
>>>>>> +@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).
>>>>
>>>> Rather than just "quit", there should be choices for "shutdown",
>>>> "poweroff" and "reset" for consistency with -watchdog-action.
>> Hmm, quit means poweroff. I will add reset, but I think we donot
>> need shutdown, because we cannot shutdown the system when it is
>> panicked.
> 
> Right. :)
> 
> Can you also rename quit to poweroff?

OK.

Thanks
Wen Congyang
> 
>>>> Also, this option should also be accessible from "-machine", please.
>> I don't understand this. Do you mean we can also specify this action
>> in -machine option?
> 
> Yes, like -machine panic_action=poweroff.
> 
> Paolo
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] deal with guest panicked event
@ 2012-06-13  8:00           ` Wen Congyang
  0 siblings, 0 replies; 50+ messages in thread
From: Wen Congyang @ 2012-06-13  8:00 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Gleb Natapov, kvm list, Jan Kiszka, qemu-devel, linux-kernel,
	Avi Kivity, KAMEZAWA Hiroyuki

At 06/13/2012 03:53 PM, Paolo Bonzini Wrote:
> Il 13/06/2012 09:02, Wen Congyang ha scritto:
>>>>>> +Other possible actions are:
>>>>>> +@code{pause} (emit QEVENT_GUEST_PANICKED and pause VM),
>>>>>> +@code{quit} (emit QEVENT_GUEST_PANICKED and quit VM).
>>>>
>>>> Rather than just "quit", there should be choices for "shutdown",
>>>> "poweroff" and "reset" for consistency with -watchdog-action.
>> Hmm, quit means poweroff. I will add reset, but I think we donot
>> need shutdown, because we cannot shutdown the system when it is
>> panicked.
> 
> Right. :)
> 
> Can you also rename quit to poweroff?

OK.

Thanks
Wen Congyang
> 
>>>> Also, this option should also be accessible from "-machine", please.
>> I don't understand this. Do you mean we can also specify this action
>> in -machine option?
> 
> Yes, like -machine panic_action=poweroff.
> 
> Paolo
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

end of thread, other threads:[~2012-06-13  8:00 UTC | newest]

Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-21  6:46 [PATCH] kvm: notify host when guest panicked Wen Congyang
2012-05-21  6:46 ` [Qemu-devel] " Wen Congyang
2012-05-21  6:49 ` [PATCH 1/3] start vm after reseting it Wen Congyang
2012-05-21  6:49   ` [Qemu-devel] " Wen Congyang
2012-05-30 19:17   ` Luiz Capitulino
2012-05-30 19:17     ` Luiz Capitulino
2012-06-12  7:23     ` [Qemu-devel] " Wen Congyang
2012-06-12  7:23       ` Wen Congyang
2012-06-13  6:54     ` Wen Congyang
2012-06-13  6:54       ` Wen Congyang
2012-06-13  6:54       ` Wen Congyang
2012-05-21  6:50 ` [PATCH 2/3] update linux headers Wen Congyang
2012-05-21  6:50   ` [Qemu-devel] " Wen Congyang
2012-05-21  6:50 ` [PATCH 3/3] deal with guest panicked event Wen Congyang
2012-05-21  6:50   ` [Qemu-devel] " Wen Congyang
2012-05-22 11:32   ` Jan Kiszka
2012-05-22 11:32     ` [Qemu-devel] " Jan Kiszka
2012-05-22 11:32     ` Jan Kiszka
2012-05-30 19:23   ` [Qemu-devel] " Luiz Capitulino
2012-05-30 19:23     ` Luiz Capitulino
2012-06-12  6:55     ` [Qemu-devel] " Wen Congyang
2012-06-12  6:55       ` Wen Congyang
2012-06-12 12:35       ` Luiz Capitulino
2012-06-12 12:35         ` Luiz Capitulino
2012-06-12 12:40         ` Daniel P. Berrange
2012-06-12 12:40           ` Daniel P. Berrange
2012-06-12 13:21           ` Luiz Capitulino
2012-06-12 13:21             ` Luiz Capitulino
2012-06-12 13:29   ` Paolo Bonzini
2012-06-12 13:29     ` [Qemu-devel] " Paolo Bonzini
2012-06-13  7:02     ` Wen Congyang
2012-06-13  7:02       ` [Qemu-devel] " Wen Congyang
2012-06-13  7:53       ` Paolo Bonzini
2012-06-13  7:53         ` [Qemu-devel] " Paolo Bonzini
2012-06-13  7:53         ` Paolo Bonzini
2012-06-13  8:00         ` Wen Congyang
2012-06-13  8:00           ` [Qemu-devel] " Wen Congyang
2012-06-13  8:00           ` Wen Congyang
2012-06-12  6:47 ` [PATCH] kvm: notify host when guest panicked Wen Congyang
2012-06-12  6:47   ` [Qemu-devel] " Wen Congyang
2012-06-12  7:49   ` Christian Borntraeger
2012-06-12  7:49     ` [Qemu-devel] " Christian Borntraeger
2012-06-12  8:07     ` Wen Congyang
2012-06-12  8:07       ` [Qemu-devel] " Wen Congyang
2012-06-12  8:26     ` Wen Congyang
2012-06-12  8:26       ` [Qemu-devel] " Wen Congyang
2012-06-12  8:26       ` Wen Congyang
2012-06-12  9:49       ` Gleb Natapov
2012-06-12  9:49         ` [Qemu-devel] " Gleb Natapov
2012-06-12  9:49         ` Gleb Natapov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.