[libvirt] RFC: Add further domain event callbacks

[Daniel P. Berrange]

On Tue, Mar 16, 2010 at 11:16:43AM +0100, Daniel Veillard wrote:
> 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.

We could still provide the register method against the domain object
directly

  int virDomainEventRegister(virDomainPtr dom,
                             const char *eventname,
                             virConnectEventCallback cb,
                             void *opaque,
                             virFreeCallback freecb);

We might end up having virStoragePoolEventRegister too, but that's
not soo bad, since its a fairly finite set of methods.

> 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

I think option 2/ would map most easily into the GLib/Qt event models, if
we registered directly against virDomainPtr instead of virConnectPtr+void*domain

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|