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

On 12/13/2011 05:38 PM, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; > > * 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?

> + 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).