[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v7 5/5] shutdown: Expose bool cause
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v7 5/5] shutdown: Expose bool cause in SHUTDOWN and RESET events |
Date: |
Tue, 9 May 2017 09:18:41 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
On 05/09/2017 07:07 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> Libvirt would like to be able to distinguish between a SHUTDOWN
>> event triggered solely by guest request and one triggered by a
>> SIGTERM or other action on the host. While qemu_kill_report() was
>> already able to give different output to stderr based on whether a
>> shutdown was triggered by a host signal (but NOT by a host UI event,
>> such as clicking the X on the window), that information was then
>> lost to management. The previous patches improved things to use an
>> enum throughout all callsites, so now we have something ready to
>> expose through QMP.
>>
>> Here is output from 'virsh qemu-monitor-event --loop' with the
>> patch installed:
>>
>> event SHUTDOWN at 1492639680.731251 for domain fedora_13: {"guest":true}
>> event STOP at 1492639680.732116 for domain fedora_13: <null>
>> event SHUTDOWN at 1492639680.732830 for domain fedora_13: {"guest":false}
>>
>> Note that libvirt runs qemu with -no-quit: the first SHUTDOWN event
>
> Do you mean -no-shutdown?
Oh, right. (Too many synonyms to choose from).
>
>> was triggered by an action I took directly in the guest (shutdown -h),
>> at which point qemu stops the vcpus and waits for libvirt to do any
>> final cleanups; the second SHUTDOWN event is the result of libvirt
>> sending SIGTERM now that it has completed cleanup.
>
> The double shutdown is a bit weird. To avoid it, we'd have to
> distinguish between guest shutdown and QEMU termination. shutdown -h in
> the guest triggers termination only when QEMU is configured that way.
> SIGTERM triggers shutdown only when the guest is running. The result
> would be neater, but I'm not sure it's worth the extra effort.
And libvirt is already handling the double event emission - it only
exposes the first event to the end-user (which is the one that will have
the correct guest-vs-host flag); the second event (which will always be
host) is elided because libvirt already knows it passed on the first
event. So changing it is outside the scope of this series.
>> +++ b/vl.c
>> @@ -1705,8 +1705,8 @@ void qemu_system_reset(ShutdownCause reason)
>> qemu_devices_reset();
>> }
>> if (reason) {
>> - /* FIXME update event based on reason */
>> - qapi_event_send_reset(&error_abort);
>> + qapi_event_send_reset(reason >= SHUTDOWN_CAUSE_GUEST_SHUTDOWN,
>> + &error_abort);
>
> Exploits enum ordering. A comment in the enum definition warning not to
> reorder its members would be in order. Defining a suitable predicate
> right next to it would do, too.
As in, adding this to sysemu.h?
static inline bool shutdown_caused_by_guest(ShutdownCause cause) {
return cause >= SHUTDOWN_CAUSE_GUEST_SHUTDOWN;
}
I can do that, if you like it.
>
> With at least the -no-quit in the commit message fixed (assuming it
> needs fixing):
Yes, it does need fixing.
>
> Reviewed-by: Markus Armbruster <address@hidden>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature