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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list