Re: [libvirt] RFC: Add further domain event callbacks

On Thu, Mar 04, 2010 at 03:25:28PM +0000, Daniel P. Berrange wrote: > Option 2 > -------- > > GLib/GObject take a very loosely typed approach to registering/unregistering > events. The have a single pair of methods that work for any event & a generic > callback signature, requiring application casts. > > typedef int (*virConnectEventCallback)(void *opaque); > > int virConnectEventRegister(virConnectPtr conn, > const char *eventname, > virConnectEventCallback cb, > void *opaque, > virFreeCallback freecb); > > int virCOnnectEventUnregister(virConnectPtr conn, > int eventID); > > In this model, the register method returns a unique integer ID for the > callback which can be used to unregister it. Application's using this > will still need a strongly typed callback for receiving the event, but > when calling virConnectEventRegister(), the would do an explicit 'bad' > cast to 'virConnectEventCallback' That fine to me except that for example if you want to bind to a specific object (a domain, an interface) to catch his mutation, then you need to pass this too, ending up with int virConnectEventRegister(virConnectPtr conn, void * object, const char *eventname, virConnectEventCallback cb, void *opaque, virFreeCallback freecb); that means two casts. I like the unregister based on the ID token though, that part can be completely generic.

That still doesn't solve the opaque callback object allocation, I'm afraid, unless libvirt itself doesn't touch anything, but then that mean that libvirt can only pass an event, but not provide any contextual information (any of the extra stuff we use for Domain callbacks). > > Option 3 > -------- > > A hybrid of both approaches. Have a new 'register' method for each type of > event that takes a strongly typed callback, but have a generic 'unregister' > method that just uses the 'int eventID' > > int virConnectDomainBlockIOEventRegister(virConnectPtr conn, > virConnectDomainBlockIOEventCallback cb, > void *opaque, > virFreeCallback freecb); > > int virCOnnectEventUnregister(virConnectPtr conn, > int eventID); > > Doesn't solve the real problem. > Option 4 > -------- > > Have one pair of register/unregister events, but instead of passing diffeerent > parameters to each callback, have a generic callback that takes a single > parameter. This parameter would be declared as a union. So depending on > the type of event being received, you'd access different parts of the union > > > typedef union { > virConnectDomainBlockIOEvent blockio; > virConnectDomainWatchdogEvent watchdog; > ...other events... > } virConnectEvent; > > > Either we could include a dummy member in the union with padding to 1024 > bytes in size for future expansion, or we could simply declare that apps > must never allocate this data type themselves, thus allowing us to enlarge > it at will. yeah, I know that glib has been doing that padding to ensure forward compatibility over time. I find that a bit fishy. At least that solves the allocation problem for data passed by libvirt as part of the callback. > typedef int (*virConnectEventCallback)(int eventType, virConnectEvent, void *opaque); > > int virConnectEventRegister(virConnectPtr conn, > const char *eventname, > virConnectEventCallback cb, > void *opaque, > virFreeCallback freecb); > > int virConnectEventUnregister(virConnectPtr conn, > int eventID); > > I'm undecided yet between 1/ 2/ and 4/ there are pros and cons to all of them in terms of API. > There is one final question unrelated to these 4 options. For the lifecycle > events we always registered against the 'virConnectPtr' since that is > needed to capture 'domain created' events where there's no virDomainPtr > to register a callback against yet. > > Do we want to always register all events aganist the virConnectPtr, and > then pass a 'virDomainPtr' as a parameter to the callbacks as needed. Or yes I don't see how we could avoid adding an extra target object > should we allow registering events against the virDomainPtr directly. > The latter might make it simpler to map libvirt into GLib/GObjects event > system in the future. Then IMHO in that case we're stuck with 1/ , if we have per target objects type of registrations, or as I suggested 2/ with an extra void *object