On 06/05/2013 03:54 AM, Chen Fan wrote:
Add monitor callback API domainGUESTPanicked, that implements the 'on_crash' behavior in the XML when domain crashed. --- src/qemu/qemu_monitor.c | 14 +++++- src/qemu/qemu_monitor.h | 5 +++ src/qemu/qemu_monitor_json.c | 7 +++ src/qemu/qemu_process.c | 103 ++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 127 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 4e35f79..e0cd62c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -113,7 +113,7 @@ VIR_ENUM_IMPL(qemuMonitorVMStatus, QEMU_MONITOR_VM_STATUS_LAST, "debug", "inmigrate", "internal-error", "io-error", "paused", "postmigrate", "prelaunch", "finish-migrate", "restore-vm", - "running", "save-vm", "shutdown", "watchdog") + "running", "save-vm", "shutdown", "watchdog", "guest-panicked")
If this is internal-only, can we use the shorter name 'guest-panic'?
typedef enum { QEMU_MONITOR_BLOCK_IO_STATUS_OK, @@ -1032,6 +1032,15 @@ int qemuMonitorEmitResume(qemuMonitorPtr mon) }
+int qemuMonitorEmitGUESTPanicked(qemuMonitorPtr mon)
WHY THE ALL CAPS? qemuMonitorEmitGuestPanic() is sufficient for naming purposes.
+{ + int ret = -1; + VIR_DEBUG("mon=%p", mon); + QEMU_MONITOR_CALLBACK(mon, ret, domainGUESTPanicked, mon->vm); + return ret; +} + + int qemuMonitorEmitRTCChange(qemuMonitorPtr mon, long long offset)
If it was because of copy-and-paste, remember that RTC is an acronym, but Guest is not.
+++ b/src/qemu/qemu_monitor.h @@ -140,6 +140,8 @@ struct _qemuMonitorCallbacks { unsigned long long actual); int (*domainPMSuspendDisk)(qemuMonitorPtr mon, virDomainObjPtr vm); + int (*domainGUESTPanicked)(qemuMonitorPtr mon,
Again, s/GUEST/Guest/
+ virDomainObjPtr vm); };
char *qemuMonitorEscapeArg(const char *in); @@ -220,6 +222,8 @@ int qemuMonitorEmitBlockJob(qemuMonitorPtr mon, int qemuMonitorEmitBalloonChange(qemuMonitorPtr mon, unsigned long long actual); int qemuMonitorEmitPMSuspendDisk(qemuMonitorPtr mon); +int qemuMonitorEmitGUESTPanicked(qemuMonitorPtr mon);
and again.
+++ b/src/qemu/qemu_monitor_json.c @@ -74,6 +74,7 @@ static void qemuMonitorJSONHandleBlockJobCanceled(qemuMonitorPtr mon, virJSONVal static void qemuMonitorJSONHandleBlockJobReady(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleBalloonChange(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandlePMSuspendDisk(qemuMonitorPtr mon, virJSONValuePtr data); +static void qemuMonitorJSONHandleGUESTPanicked(qemuMonitorPtr mon, virJSONValuePtr data);
and again
+static int +qemuProcessHandleGUESTPanicked(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm) +{
+ + if (vm->def->onCrash == VIR_DOMAIN_LIFECYCLE_CRASH_PRESERVE) { ... + goto cleanup; + } + + if (vm->def->onCrash == VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY) {
Worth doing this as 'else if'?
+ isReboot = false; + VIR_INFO("Domain on_crash setting overridden, shutting down");
That sounds like fake reboot is overriding on_crash settings. I'd word this: on_crash overrides fake reboot request, shutting down instead and maybe be careful to only output that info message only if isReboot actually changed from true to false.
+ + if (! virDomainObjIsActive(vm)) {
Generally no space after '!'.
@@ -2673,6 +2770,10 @@ qemuProcessUpdateState(virQEMUDriverPtr driver, virDomainObjPtr vm) newState = VIR_DOMAIN_SHUTDOWN; newReason = VIR_DOMAIN_SHUTDOWN_UNKNOWN; ignore_value(VIR_STRDUP_QUIET(msg, "shutdown")); + } else if (reason == VIR_DOMAIN_PAUSED_GUEST_PANICKED) { + newState = VIR_DOMAIN_CRASHED; + newReason = VIR_DOMAIN_CRASHED_PANICKED; + ignore_value(VIR_STRDUP_QUIET(msg, "was crashed"));
s/was // Overall, the mechanics seem like they will work, but the spelling to use Guest instead of GUEST in function names probably warrants a respin. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org