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@xxxxxxxxxx> > > --- > > 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
