[libvirt] Re: [PATCH 01/12] Domain Events - Public API

[Daniel Veillard]

On Tue, Oct 21, 2008 at 03:11:08PM -0400, Ben Guthro wrote:
> [PATCH 01/12] Domain Events - Public API
> 
> This patch does the following:
>    -implements the Event register/deregister code
>    -Adds some callback lists, and queue functions used by drivers
>    -Move EventImpl definitions into the public

thanks for the changes! I think the prototypes are good to go, maybe a couple
tiny changes but I can make them myself based on the 'make rebuild'
output in the doc directory
functions in src/libvirt.c still need arguments checkings however, but
I can add that later.

> +/**
> + * virEventHandleCallback: callback for receiving file handle events

  description just need to be moved after the parameters, can be
  trivially done later.

> + * @fd: file handle on which the event occurred
> + * @events: bitset of events from virEventHandleType constants
> + * @opaque: user data registered with handle
> + */
> +typedef void (*virEventHandleCallback)(int fd, virEventHandleType events,
> +                                       void *opaque);
> +/**
> + * virConnectDomainEventRegister:
> + * @conn: pointer to the connection
> + * @cb: callback to the function handling domain events
> + * @opaque: opaque data to pass on to the callback
> + *
> + * Adds a Domain Event Callback
> + *
> + * Returns 0 on success, -1 on failure
> + */
> +int
> +virConnectDomainEventRegister(virConnectPtr conn,
> +                              virConnectDomainEventCallback cb,
> +                              void *opaque)
> +{

   conn still need to be checked  with the standard block

    if (!VIR_IS_CONNECT(conn)) {
        virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
        return (-1);
    }

like every other public functions with a conn parameter in src/libvirt.c
What if cb is NULL, that probably need to be rejected

> +    /* Registering for a domain callback will enable delivery by default */
> +    if (conn->driver && conn->driver->domainEventRegister)
> +        return conn->driver->domainEventRegister (conn, cb, opaque);
> +    return -1;
> +}
> +
> +/**
> + * virConnectDomainEventDeregister:
> + * @conn: pointer to the connection
> + * @cb: callback to the function handling domain events
> + *
> + * Removes a Domain Event Callback
> + *
> + * Returns 0 on success, -1 on failure
> + */
> +int
> +virConnectDomainEventDeregister(virConnectPtr conn,
> +                                virConnectDomainEventCallback cb)
> +{

  same thing for VIR_IS_CONNECT and cb being NULL tests

otherwise looks fine to me.

Daniel


-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/