[libvirt] [PATCH v2] qemu: Report shutdown event details
Daniel P. Berrange
berrange at redhat.com
Tue May 16 17:20:24 UTC 2017
On Tue, May 16, 2017 at 12:10:23PM -0500, Eric Blake wrote:
> On 05/16/2017 08:49 AM, Martin Kletzander wrote:
> > QEMU will likely report the details of it shutting down, particularly
> > whether the shutdown was initiated by the guest or host. We should
> > forward that information along, at least for shutdown events. Reset
> > has that as well, however that is not a lifecycle event and would add
> > extra constants that might not be used. It can be added later on.
> >
> > Since the only way we can extend information provided to the user is
> > adding event details, we might as well emit multiple events (one with
> > the reason for the shutdown and keep the one for the shutdown being
> > finished for clarity and compatibility).
> >
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1384007
> >
> > Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> > ---
> > v2:
> > - Adapt to new message format
> >
> > Patch in QEMU: https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03624.html
> > Applied to qapi-next: https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03742.html
>
> Not in qemu master yet, but should land there prior to the next libvirt
> release.
>
>
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -2983,7 +2983,16 @@ typedef enum {
> > * Details on the cause of a 'shutdown' lifecycle event
> > */
> > typedef enum {
> > - VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED = 0, /* Guest finished shutdown sequence */
> > + /* Guest finished shutdown sequence */
> > + VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED = 0,
> > +
> > + /* Guest is shutting down due to request from guest (e.g. hardware-specific
> > + * action) */
> > + VIR_DOMAIN_EVENT_SHUTDOWN_GUEST = 1,
> > +
> > + /* Guest is shutting down due to request from host (e.g. killed by a
> > + * signal) */
> > + VIR_DOMAIN_EVENT_SHUTDOWN_HOST = 2,
> >
>
> Looks reasonable.
>
> > +++ b/src/qemu/qemu_monitor_json.c
> > @@ -523,9 +523,15 @@ qemuMonitorJSONKeywordStringToJSON(const char *str, const char *firstkeyword)
> > }
> >
> >
> > -static void qemuMonitorJSONHandleShutdown(qemuMonitorPtr mon, virJSONValuePtr data ATTRIBUTE_UNUSED)
> > +static void qemuMonitorJSONHandleShutdown(qemuMonitorPtr mon, virJSONValuePtr data)
> > {
> > - qemuMonitorEmitShutdown(mon);
> > + bool guest = false;
> > + virTristateBool guest_initiated = VIR_TRISTATE_BOOL_ABSENT;
> > +
> > + if (virJSONValueObjectGetBoolean(data, "guest", &guest) == 0)
> > + guest_initiated = guest ? VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
> > +
> > + qemuMonitorEmitShutdown(mon, guest_initiated);
>
> Yes, that uses the new qemu interface correctly.
>
> > @@ -678,6 +699,7 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
> >
> > unlock:
> > virObjectUnlock(vm);
> > + qemuDomainEventQueue(driver, pre_event);
> > qemuDomainEventQueue(driver, event);
> > virObjectUnref(cfg);
>
> Nice - you send the same event as always so old clients don't break, but
> new clients can now look for the new cause.
I don't think that's right actually. IMHO, we should onyl be sending the
new event, not the original event. These are intended to indicate state
changes, and having two VIR_DOMAIN_EVENT_SHUTDOWN events sent with
different details is not really representing a state change.
WRT to compatibility, clients should always expect that 'detail' may
change or new 'detail' enum values may be added - indeed we've done
that many many times int he past. So I don't think we need to duplicate
the existing event
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
More information about the libvir-list
mailing list