On Thu, Oct 13, 2011 at 10:54:13AM +0100, Daniel P. Berrange wrote: > On Tue, Oct 11, 2011 at 08:46:06AM -0500, Adam Litke wrote: > > On Mon, Oct 10, 2011 at 11:54:08PM +0800, shaohef@xxxxxxxxxxxxxxxxxx wrote: > > > From: Shaohe Feng <shaohef@xxxxxxxxxxxxxxxxxx> > > > > > > Basically, this feature can go along with qemu monitor passthrough. > > > That way, if we use new commands in the monitor that generate new events, we want > > > some way to receive those new events too. > > > > I agree that this patch is very complimentary to the existing tools for qemu > > interaction (qemu_monitor_command, qemu_attach, etc). It allows API users to > > subscribe to events that are not yet handled by libvirt. I have a couple of > > design questions about this feature. > > > > 1) This feature seems to be a bit qemu specific. Other qemu-specific APIs > > (mentioned above) are build into libvirt-qemu.so so that they are kept apart > > from the hypervisor-neutral parts of the API. Is it possible to do that for > > an event handler like this patch implements? Do we want to enforce such a > > limit? > > Yep, I agree it ought to be in the QEMU specific library. > > > 2) This patch causes a string representing a raw JSON event object to be > > passed to the callbacks that are registered for the default event. This seems > > fine to me. I do wonder if relying on a 'default' event is a bad thing for an > > application to do. Let's say an app decides to handle NEW_EVENT using this > > default handler. Then, libvirt gains official support for NEW_EVENT. When the > > libvirt package is updated in the application's environment, NEW_EVENT will no > > longer trigger via the default handler. Thus, the application is broken by a > > libvirt upgrade. Would it be better to have a 'raw event' sink where all events > > (whether supported or not) would be sent? > > I expect applications will want the event to be dispatched to them, > regardless of whether libvirt has handled it itself, so that they > don't have to update themselves to the latest libvirt event API > immediately. > > Also I think it would be better if we were able to connect to explicit JSON > events, rather than a catch all. THe number of events from QEMU is only > going to increase over time & as it does, a 'catch all' becomes increasingly > bandwidth wasteful. Agreed on both of these. > I'd reckon we want something like this: > > > typedef void (*virConnectDomainQemuEventCallback)(virConnectPtr conn, > virDomainPtr dom, > const char *eventName, /* The JSON event name */ > const char *eventArgs, /* The JSON string of args */ > void *opaque); > > virConnectDomainQemuEventRegister(virConnectPtr conn, > virDomainPtr dom, /* option to filter */ > const char *eventName, /* JSON event name */ > virConnectDomainQemuEventCallback cb, > void *opaque, > virFreeCallback freecb); > virConnectDomainQemuEventDeregister(virConnectPtr conn, > int callbackID); > > > in libvirt-qemu.h, and libvirt-qemu.so Looks good to me. -- Adam Litke <agl@xxxxxxxxxx> IBM Linux Technology Center -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list