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@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list