[libvirt] [PATCH 4/8] Convert drivers to thread safe APIs for adding callbacks

[Daniel P. Berrange]

On Wed, Dec 14, 2011 at 01:15:58PM -0700, Eric Blake wrote:
> On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange at redhat.com>
> > 
> > * src/libxl/libxl_driver.c, src/lxc/lxc_driver.c,
> >   src/qemu/qemu_driver.c, src/remote/remote_driver.c,
> >   src/test/test_driver.c, src/uml/uml_driver.c,
> >   src/vbox/vbox_tmpl.c, src/xen/xen_driver.c: Convert
> >   to threadsafe APIs
> > ---
> >  src/libxl/libxl_driver.c   |   18 ++++++------
> >  src/lxc/lxc_driver.c       |   18 ++++++------
> >  src/qemu/qemu_driver.c     |   18 ++++++------
> >  src/remote/remote_driver.c |   64 ++++++++++++++++++-------------------------
> >  src/test/test_driver.c     |   14 +++++-----
> >  src/uml/uml_driver.c       |   18 ++++++------
> >  src/vbox/vbox_tmpl.c       |   24 ++++++++--------
> >  src/xen/xen_driver.c       |   10 +++---
> >  8 files changed, 87 insertions(+), 97 deletions(-)
> 
> Safer and slightly smaller - a nice mix :)
> 
> > +++ b/src/remote/remote_driver.c
> > @@ -3124,13 +3124,8 @@ static int remoteDomainEventRegister(virConnectPtr conn,
> >  
> >      remoteDriverLock(priv);
> >  
> > -    if (priv->domainEventState->timer < 0) {
> > -         remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event support"));
> > -         goto done;
> > -    }
> 
> Does this hunk belong here, or in a later patch?

The next patch removes the contents of virDomainEventState from
public view, so this has to be removed now. A completely different
solution to the problem appears in a later patch, because instead
of registering a timer upfront when virDomainEventStatePtr is
allocated, we will register the timer at first use, so can report
the error directly, instead of having a delayed report as the old
code did.

> > +    if ((count = virDomainEventStateRegisterID(conn,
> > +                                               priv->domainEventState,
> > +                                               dom, eventID,
> > +                                               callback, opaque, freecb,
> > +                                               &callbackID)) < 0) {
> > +         remoteError(VIR_ERR_RPC, "%s", _("adding cb to list"));
> >           goto done;
> 
> Indentation is off here (9 instead of 8 spaces, on both instances of
> this error message).
> 
> ACK - the bulk of this patch is mechanical; and while I think you should
> rebase the remote_driver.c stuff slightly differently, at least this one
> compiled (unlike the rebase disaster on 1/8).

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|