Daniel Veillard wrote on 10/22/2008 05:14 AM:
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.
Are these changes worth a 4th round of submissions - or is this something that can be taken care of in a follow-up patch?
+/** + * 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