[libvirt] [PATCH 3/8] Add APIs to allow management of callbacks purely with virDomainEventState

[Eric Blake]

On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> While virDomainEventState has APIs for managing removal of callbacks,
> while locked, adding callbacks in the first place requires direct
> access to the virDomainEventCallbackList structure. This is not
> threadsafe since it is bypassing the virDomainEventState locks
> 
> * src/conf/domain_event.c, src/conf/domain_event.h,
>   src/libvirt_private.syms: Add APIs for managing callbacks
>   via virDomainEventState.
> ---
>  src/conf/domain_event.c  |   99 ++++++++++++++++++++++++++++++++++++++++++++++
>  src/conf/domain_event.h  |   24 +++++++++++
>  src/libvirt_private.syms |    4 ++
>  3 files changed, 127 insertions(+), 0 deletions(-)

>  
>  /**
> + * virDomainEventStateRegister:
> + * @state: domain event state
> + * @conn: connection to associate with callback

This order...

> + * @callback: function to remove from event
> + * @opaque: data blob to pass to callback
> + * @freecb: callback to free @opaque
> + *
> + * Register the function @callback with connection @conn,
> + * from @state, for lifecycle events.
> + *
> + * Returns: the number of lifecycle callbacks now registered, or -1 on error
> + */
> +int virDomainEventStateRegister(virConnectPtr conn,
> +                                virDomainEventStatePtr state,

...doesn't match this.

> +/**
> + * virDomainEventStateRegisterID:
> + * @state: domain event state
> + * @conn: connection to associate with callback

Again ordering doesn't match.  And you missed @dom.

> +/**
> + * virDomainEventStateDeregisterConn:
> + * @state: domain event state
> + * @conn: connection to associate with callbacks

and again.

> +
> +
> +int
> +virDomainEventStateEventID(virConnectPtr conn,
> +                           virDomainEventStatePtr state,
> +                           int callbackID)

No doc comments?

> +++ b/src/libvirt_private.syms
> @@ -511,6 +511,10 @@ virDomainEventRebootNewFromDom;
>  virDomainEventRebootNewFromObj;
>  virDomainEventStateDeregister;
>  virDomainEventStateDeregisterID;
> +virDomainEventStateDeregisterConn;

Not quite sorted :)

But all my findings are trivial, so ACK with nits fixed.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111214/54ab335e/attachment-0001.sig>