On Tue, Dec 31, 2013 at 08:21:29AM -0700, Eric Blake wrote:
Since libvirt 0.9.3, the entire virevent.c file has been a public
API, so improve the documentation in this file. Also, fix a
potential core dump - it could only be triggered by bogus use of
the API and would only affect the caller (not libvirtd), but we
might as well be nice.
* src/libvirt.c (virConnectDomainEventRegister)
(virConnectDomainEventRegisterAny)
(virConnectNetworkEventRegisterAny): Document event loop requirement.
* src/util/virevent.c (virEventAddHandle, virEventRemoveHandle)
(virEventAddTimeout, virEventRemoveTimeout): Likewise.
(virEventUpdateHandle, virEventUpdateTimeout): Likewise, and avoid
core dump if caller didn't register handler.
(virEventRunDefaultImpl): Expand example, and set up code block in
html docs.
(virEventRegisterImpl, virEventRegisterDefaultImpl): Document more
on the use of the event loop.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/libvirt.c | 24 ++++++++++------
src/util/virevent.c | 82 +++++++++++++++++++++++++++++++++++------------------
2 files changed, 70 insertions(+), 36 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 66841c8..f43718d 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -16121,11 +16121,13 @@ error:
* @freecb: optional function to deallocate opaque when not used anymore
*
* Adds a callback to receive notifications of domain lifecycle events
- * occurring on a connection
+ * occurring on a connection. This function requires that an event loop
+ * has been previously registered with virEventRegisterImpl() or
+ * virEventRegisterDefaultImpl().
*
* Use of this method is no longer recommended. Instead applications
* should try virConnectDomainEventRegisterAny() which has a more flexible
- * API contract
+ * API contract.
*
* The virDomainPtr object handle passed into the callback upon delivery
* of an event is only valid for the duration of execution of the callback.
@@ -16134,7 +16136,7 @@ error:
* The reference can be released once the object is no longer required
* by calling virDomainFree.
*
- * Returns 0 on success, -1 on failure
+ * Returns 0 on success, -1 on failure.
*/
int
virConnectDomainEventRegister(virConnectPtr conn,
@@ -19064,10 +19066,12 @@ error:
* @freecb: optional function to deallocate opaque when not used anymore
*
* Adds a callback to receive notifications of arbitrary domain events
- * occurring on a domain.
+ * occurring on a domain. This function requires that an event loop
+ * has been previously registered with virEventRegisterImpl() or
+ * virEventRegisterDefaultImpl().
*
* If @dom is NULL, then events will be monitored for any domain. If @dom
- * is non-NULL, then only the specific domain will be monitored
+ * is non-NULL, then only the specific domain will be monitored.
*
* Most types of event have a callback providing a custom set of parameters
* for the event. When registering an event, it is thus necessary to use
@@ -19085,7 +19089,7 @@ error:
* for the callback. To unregister a callback, this callback ID should
* be passed to the virConnectDomainEventDeregisterAny() method.
*
- * Returns a callback identifier on success, -1 on failure
+ * Returns a callback identifier on success, -1 on failure.
*/
int
virConnectDomainEventRegisterAny(virConnectPtr conn,
@@ -19183,10 +19187,12 @@ error:
* @freecb: optional function to deallocate opaque when not used anymore
*
* Adds a callback to receive notifications of arbitrary network events
- * occurring on a network.
+ * occurring on a network. This function requires that an event loop
+ * has been previously registered with virEventRegisterImpl() or
+ * virEventRegisterDefaultImpl().
*
* If @net is NULL, then events will be monitored for any network. If @net
- * is non-NULL, then only the specific network will be monitored
+ * is non-NULL, then only the specific network will be monitored.
*
* Most types of event have a callback providing a custom set of parameters
* for the event. When registering an event, it is thus necessary to use
@@ -19204,7 +19210,7 @@ error:
* for the callback. To unregister a callback, this callback ID should
* be passed to the virConnectNetworkEventDeregisterAny() method.
*
- * Returns a callback identifier on success, -1 on failure
+ * Returns a callback identifier on success, -1 on failure.
*/
int
virConnectNetworkEventRegisterAny(virConnectPtr conn,
diff --git a/src/util/virevent.c b/src/util/virevent.c
index fde29a2..0c94946 100644
--- a/src/util/virevent.c
+++ b/src/util/virevent.c
@@ -37,6 +37,16 @@ static virEventAddTimeoutFunc addTimeoutImpl = NULL;
static virEventUpdateTimeoutFunc updateTimeoutImpl = NULL;
static virEventRemoveTimeoutFunc removeTimeoutImpl = NULL;
+
+/*****************************************************
+ *
+ * Below this point are *PUBLIC* APIs for event
+ * loop integration with applications using libvirt.
+ * These API contracts cannot be changed.
+ *
+ *****************************************************/
+
+
/**
* virEventAddHandle:
*
@@ -46,10 +56,12 @@ static virEventRemoveTimeoutFunc removeTimeoutImpl = NULL;
* @opaque: user data to pass to callback
* @ff: callback to free opaque when handle is removed
*
- * Register a callback for monitoring file handle events.
+ * Register a callback for monitoring file handle events. This function
+ * requires that an event loop has previously been registered with
+ * virEventRegisterImpl() or virEventRegisterDefaultImpl().
*
* Returns -1 if the file handle cannot be registered, otherwise a handle
- * watch number to be used for updating and unregistering for events
+ * watch number to be used for updating and unregistering for events.
*/
int
virEventAddHandle(int fd,
@@ -70,14 +82,17 @@ virEventAddHandle(int fd,
* @watch: watch whose file handle to update
* @events: bitset of events to watch from virEventHandleType constants
*
- * Change event set for a monitored file handle.
+ * Change event set for a monitored file handle. This function
+ * requires that an event loop has previously been registered with
+ * virEventRegisterImpl() or virEventRegisterDefaultImpl().
*
- * Will not fail if fd exists
+ * Will not fail if fd exists.
*/
void
virEventUpdateHandle(int watch, int events)
{
- updateHandleImpl(watch, events);
+ if (updateHandleImpl)
+ updateHandleImpl(watch, events);
}
/**
@@ -85,7 +100,9 @@ virEventUpdateHandle(int watch, int events)
*
* @watch: watch whose file handle to remove
*
- * Unregister a callback from a file handle.
+ * Unregister a callback from a file handle. This function
+ * requires that an event loop has previously been registered with
+ * virEventRegisterImpl() or virEventRegisterDefaultImpl().
*
* Returns -1 if the file handle was not registered, 0 upon success.
*/
@@ -106,7 +123,9 @@ virEventRemoveHandle(int watch)
* @opaque: user data to pass to callback
* @ff: callback to free opaque when timeout is removed
*
- * Register a callback for a timer event.
+ * Register a callback for a timer event. This function
+ * requires that an event loop has previously been registered with
+ * virEventRegisterImpl() or virEventRegisterDefaultImpl().
*
* Setting timeout to -1 will disable the timer. Setting the timeout
* to zero will cause it to fire on every event loop iteration.
@@ -132,17 +151,20 @@ virEventAddTimeout(int timeout,
* @timer: timer id to change
* @timeout: time between events in milliseconds
*
- * Change frequency for a timer.
+ * Change frequency for a timer. This function
+ * requires that an event loop has previously been registered with
+ * virEventRegisterImpl() or virEventRegisterDefaultImpl().
*
* Setting frequency to -1 will disable the timer. Setting the frequency
* to zero will cause it to fire on every event loop iteration.
*
- * Will not fail if timer exists
+ * Will not fail if timer exists.
*/
void
virEventUpdateTimeout(int timer, int timeout)
{
- updateTimeoutImpl(timer, timeout);
+ if (updateTimeoutImpl)
+ updateTimeoutImpl(timer, timeout);
}
/**
@@ -150,7 +172,9 @@ virEventUpdateTimeout(int timer, int timeout)
*
* @timer: the timer id to remove
*
- * Unregister a callback for a timer.
+ * Unregister a callback for a timer. This function
+ * requires that an event loop has previously been registered with
+ * virEventRegisterImpl() or virEventRegisterDefaultImpl().
*
* Returns -1 if the timer was not registered, 0 upon success.
*/
@@ -164,14 +188,6 @@ virEventRemoveTimeout(int timer)
}
-/*****************************************************
- *
- * Below this point are 3 *PUBLIC* APIs for event
- * loop integration with applications using libvirt.
- * These API contracts cannot be changed.
- *
- *****************************************************/
-
/**
* virEventRegisterImpl:
* @addHandle: the callback to add fd handles
@@ -186,9 +202,14 @@ virEventRemoveTimeout(int timer)
* to integrate with the libglib2 event loop, or libevent
* or the QT event loop.
*
+ * Use of the virEventAddHandle() and similar APIs require that the
+ * corresponding handler be registered. Use of the
+ * virConnectDomainEventRegisterAny() and similar APIs requires that
+ * the three timeout handlers be registered.
s/be/to be/ or s/be/are/ maybe?
+ *
* If an application does not need to integrate with an
* existing event loop implementation, then the
- * virEventRegisterDefaultImpl method can be used to setup
+ * virEventRegisterDefaultImpl() method can be used to setup
* the generic libvirt implementation.
*/
void virEventRegisterImpl(virEventAddHandleFunc addHandle,
@@ -220,9 +241,12 @@ void virEventRegisterImpl(virEventAddHandleFunc addHandle,
* not have a need to integrate with an external event
* loop impl.
*
- * Once registered, the application has to invoke virEventRunDefaultImpl in
+ * Once registered, the application has to invoke virEventRunDefaultImpl() in
* a loop to process events. Failure to do so may result in connections being
- * closed unexpectedly as a result of keepalive timeout.
+ * closed unexpectedly as a result of keepalive timeout. The default
+ * event loop fully supports handle and and timeout events, but only
s/and and/and/
+ * wakes up on events registered by libvirt API calls such as
+ * virEventAddHandle() or virConnectDomainEventRegisterAny().
*
* Returns 0 on success, -1 on failure.
*/
@@ -255,14 +279,18 @@ int virEventRegisterDefaultImpl(void)
*
* Run one iteration of the event loop. Applications
* will generally want to have a thread which invokes
- * this method in an infinite loop
+ * this method in an infinite loop. Furthermore, it is wise
+ * to set up a pipe-to-self handler (via virEventAddHandle())
+ * or a timeout (via virEventAddTimeout()) before calling this
+ * function, as it will block forever if there are no
+ * registered events.
*
- * static bool quit = false;
+ * static bool quit = false;
*
- * while (!quit) {
- * if (virEventRunDefaultImpl() < 0)
+ * while (!quit) {
+ * if (virEventRunDefaultImpl() < 0)
* ...print error...
- * }
+ * }
*
* Returns 0 on success, -1 on failure.
*/
--
1.8.4.2
ACK, I'll have a look at the fixup in a second.
Martin