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