Re: [libvirt] [PATCH 03/13] Refactor domain events to handle multiple event types

On Fri, Mar 19, 2010 at 03:38:51PM +0000, Daniel P. Berrange wrote: > The internal domain events APIs are designed to handle the lifecycle > events. This needs to be refactored to allow arbitrary new event > types to be handled. > > * The signature of virDomainEventDispatchFunc changes to use > virConnectDomainEventGenericCallback instead of the lifecycle > event specific virConnectDomainEventCallback > * Every registered callback gains a unique ID to allow its > removal based on ID, instead of function pointer > * Every registered callback gains an 'eventID' to allow callbacks > for different types of events to be distinguished > * virDomainEventDispatch is adapted to filter out callbacks > whose eventID does not match the eventID of the event being > dispatched > * virDomainEventDispatch is adapted to filter based on the > domain name and uuid, if this filter is set for a callback. > * virDomainEvent type/detail fields are moved into a union to > allow different data fields for other types of events to be > added later > > * src/conf/domain_event.h, src/conf/domain_event.c: Refactor > to allow handling of different types of events > * src/lxc/lxc_driver.c, src/qemu/qemu_driver.c, > src/remote/remote_driver.c, src/test/test_driver.c, > src/xen/xen_driver.c: Change dispatch function signature > to use virConnectDomainEventGenericCallback [...] > diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c > index b520232..9d47eeb 100644 > --- a/src/conf/domain_event.c > +++ b/src/conf/domain_event.c > @@ -35,20 +35,36 @@ > virReportErrorHelper(conn, VIR_FROM_THIS, code, __FILE__, \ > __FUNCTION__, __LINE__, __VA_ARGS__) > > +struct _virDomainMeta { > + int id; > + char *name; > + unsigned char uuid[VIR_UUID_BUFLEN]; > +}; > +typedef struct _virDomainMeta virDomainMeta; > +typedef virDomainMeta *virDomainMetaPtr; > + > struct _virDomainEventCallback { > + int callbackID; > + int eventID; > virConnectPtr conn; > - virConnectDomainEventCallback cb; > + virDomainMetaPtr dom; > + virConnectDomainEventGenericCallback cb; > void *opaque; > virFreeCallback freecb; > int deleted; > }; > > struct _virDomainEvent { > - int id; > - char *name; > - unsigned char uuid[VIR_UUID_BUFLEN]; > - int type; > - int detail; > + int eventID; > + > + virDomainMeta dom; > + > + union { > + struct { > + int type; > + int detail; > + } lifecycle; > + } data; > }; > > /** Okay, that's logical, and most of the patch derives from that change, [...] > +static int virDomainEventDispatchMatchCallback(virDomainEventPtr event, > + virDomainEventCallbackPtr cb) > +{ > + if (!cb) > + return 0; > + if (cb->deleted) > + return 0; > + if (cb->eventID != event->eventID) > + return 0; > + > + if (cb->dom) { > + /* Delibrately ignoring 'id' for matching, since that > + * will cause problems when a domain switches between > + * running & shutoff states */ > + > + if (STREQ(event->dom.name, cb->dom->name) && > + (memcmp(event->dom.uuid, cb->dom->uuid, VIR_UUID_BUFLEN) == 0)) > + return 1; Do we really need to match for both ?