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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]