[libvirt] [RFC v2 1/2] vireventpoll: isolate common code

Derbyshev Dmitriy dderbyshev at virtuozzo.com
Tue Feb 14 18:04:20 UTC 2017


From: Derbyshev Dmitry <dderbyshev at virtuozzo.com>

To introduce new implementation (ex. with epoll) some code should be
substituted. This patch divides vireventpoll into common and
implementation-specific parts.
---
 src/Makefile.am                                   |   4 +-
 src/util/vireventpoll.c                           | 700 ++--------------------
 src/util/{vireventpoll.c => vireventpollcommon.c} | 231 ++-----
 src/util/vireventpollinternal.h                   |  91 +++
 4 files changed, 209 insertions(+), 817 deletions(-)
 copy src/util/{vireventpoll.c => vireventpollcommon.c} (78%)
 create mode 100644 src/util/vireventpollinternal.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 2f32d41..f2643ea 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -112,7 +112,8 @@ UTIL_SOURCES =							\
 		util/virendian.h				\
 		util/virerror.c util/virerror.h			\
 		util/virevent.c util/virevent.h			\
-		util/vireventpoll.c util/vireventpoll.h		\
+		util/vireventpollcommon.c util/vireventpoll.h		\
+		util/vireventpoll.c util/vireventpollinternal.h		\
 		util/virfile.c util/virfile.h			\
 		util/virfirewall.c util/virfirewall.h		\
 		util/virfirewallpriv.h				\
@@ -2376,6 +2377,7 @@ libvirt_setuid_rpc_client_la_SOURCES = 		\
 		util/virerror.c			\
 		util/virevent.c			\
 		util/vireventpoll.c		\
+		util/vireventpollcommon.c			\
 		util/virfile.c			\
 		util/virgettext.c		\
 		util/virhash.c			\
diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
index 81ecab4..e429e98 100644
--- a/src/util/vireventpoll.c
+++ b/src/util/vireventpoll.c
@@ -23,23 +23,12 @@
 
 #include <config.h>
 
-#include <stdlib.h>
-#include <string.h>
 #include <poll.h>
-#include <sys/time.h>
-#include <errno.h>
-#include <unistd.h>
-#include <fcntl.h>
 
-#include "virthread.h"
+#include "viralloc.h"
 #include "virlog.h"
 #include "vireventpoll.h"
-#include "viralloc.h"
-#include "virutil.h"
-#include "virfile.h"
-#include "virerror.h"
-#include "virprobe.h"
-#include "virtime.h"
+#include "vireventpollinternal.h"
 
 #define EVENT_DEBUG(fmt, ...) VIR_DEBUG(fmt, __VA_ARGS__)
 
@@ -47,329 +36,26 @@
 
 VIR_LOG_INIT("util.eventpoll");
 
-static int virEventPollInterruptLocked(void);
-
-/* State for a single file handle being monitored */
-struct virEventPollHandle {
-    int watch;
-    int fd;
-    int events;
-    virEventHandleCallback cb;
-    virFreeCallback ff;
-    void *opaque;
-    int deleted;
-};
-
-/* State for a single timer being generated */
-struct virEventPollTimeout {
-    int timer;
-    int frequency;
-    unsigned long long expiresAt;
-    virEventTimeoutCallback cb;
-    virFreeCallback ff;
-    void *opaque;
-    int deleted;
-};
-
-/* Allocate extra slots for virEventPollHandle/virEventPollTimeout
-   records in this multiple */
-#define EVENT_ALLOC_EXTENT 10
-
-/* State for the main event loop */
-struct virEventPollLoop {
-    virMutex lock;
-    int running;
-    virThread leader;
-    int wakeupfd[2];
-    size_t handlesCount;
-    size_t handlesAlloc;
-    struct virEventPollHandle *handles;
-    size_t timeoutsCount;
-    size_t timeoutsAlloc;
-    struct virEventPollTimeout *timeouts;
-};
-
-/* Only have one event loop */
-static struct virEventPollLoop eventLoop;
-
-/* Unique ID for the next FD watch to be registered */
-static int nextWatch = 1;
-
-/* Unique ID for the next timer to be registered */
-static int nextTimer = 1;
-
-/*
- * Register a callback for monitoring file handle events.
- * NB, it *must* be safe to call this from within a callback
- * For this reason we only ever append to existing list.
- */
-int virEventPollAddHandle(int fd, int events,
-                          virEventHandleCallback cb,
-                          void *opaque,
-                          virFreeCallback ff)
+int virEventPollAddHandleInternal(int watch ATTRIBUTE_UNUSED,
+                                  int fd ATTRIBUTE_UNUSED,
+                                  int nativeevents ATTRIBUTE_UNUSED)
 {
-    int watch;
-    virMutexLock(&eventLoop.lock);
-    if (eventLoop.handlesCount == eventLoop.handlesAlloc) {
-        EVENT_DEBUG("Used %zu handle slots, adding at least %d more",
-                    eventLoop.handlesAlloc, EVENT_ALLOC_EXTENT);
-        if (VIR_RESIZE_N(eventLoop.handles, eventLoop.handlesAlloc,
-                         eventLoop.handlesCount, EVENT_ALLOC_EXTENT) < 0) {
-            virMutexUnlock(&eventLoop.lock);
-            return -1;
-        }
-    }
-
-    watch = nextWatch++;
-
-    eventLoop.handles[eventLoop.handlesCount].watch = watch;
-    eventLoop.handles[eventLoop.handlesCount].fd = fd;
-    eventLoop.handles[eventLoop.handlesCount].events =
-                                         virEventPollToNativeEvents(events);
-    eventLoop.handles[eventLoop.handlesCount].cb = cb;
-    eventLoop.handles[eventLoop.handlesCount].ff = ff;
-    eventLoop.handles[eventLoop.handlesCount].opaque = opaque;
-    eventLoop.handles[eventLoop.handlesCount].deleted = 0;
-
-    eventLoop.handlesCount++;
-
     virEventPollInterruptLocked();
-
-    PROBE(EVENT_POLL_ADD_HANDLE,
-          "watch=%d fd=%d events=%d cb=%p opaque=%p ff=%p",
-          watch, fd, events, cb, opaque, ff);
-    virMutexUnlock(&eventLoop.lock);
-
-    return watch;
-}
-
-void virEventPollUpdateHandle(int watch, int events)
-{
-    size_t i;
-    bool found = false;
-    PROBE(EVENT_POLL_UPDATE_HANDLE,
-          "watch=%d events=%d",
-          watch, events);
-
-    if (watch <= 0) {
-        VIR_WARN("Ignoring invalid update watch %d", watch);
-        return;
-    }
-
-    virMutexLock(&eventLoop.lock);
-    for (i = 0; i < eventLoop.handlesCount; i++) {
-        if (eventLoop.handles[i].watch == watch) {
-            eventLoop.handles[i].events =
-                    virEventPollToNativeEvents(events);
-            virEventPollInterruptLocked();
-            found = true;
-            break;
-        }
-    }
-    virMutexUnlock(&eventLoop.lock);
-
-    if (!found)
-        VIR_WARN("Got update for non-existent handle watch %d", watch);
-}
-
-/*
- * Unregister a callback from a file handle
- * NB, it *must* be safe to call this from within a callback
- * For this reason we only ever set a flag in the existing list.
- * Actual deletion will be done out-of-band
- */
-int virEventPollRemoveHandle(int watch)
-{
-    size_t i;
-    PROBE(EVENT_POLL_REMOVE_HANDLE,
-          "watch=%d",
-          watch);
-
-    if (watch <= 0) {
-        VIR_WARN("Ignoring invalid remove watch %d", watch);
-        return -1;
-    }
-
-    virMutexLock(&eventLoop.lock);
-    for (i = 0; i < eventLoop.handlesCount; i++) {
-        if (eventLoop.handles[i].deleted)
-            continue;
-
-        if (eventLoop.handles[i].watch == watch) {
-            EVENT_DEBUG("mark delete %zu %d", i, eventLoop.handles[i].fd);
-            eventLoop.handles[i].deleted = 1;
-            virEventPollInterruptLocked();
-            virMutexUnlock(&eventLoop.lock);
-            return 0;
-        }
-    }
-    virMutexUnlock(&eventLoop.lock);
-    return -1;
+    return 0;
 }
 
-
-/*
- * Register a callback for a timer event
- * NB, it *must* be safe to call this from within a callback
- * For this reason we only ever append to existing list.
- */
-int virEventPollAddTimeout(int frequency,
-                           virEventTimeoutCallback cb,
-                           void *opaque,
-                           virFreeCallback ff)
+int virEventPollUpdateHandleInternal(int watch ATTRIBUTE_UNUSED,
+                                     int fd ATTRIBUTE_UNUSED,
+                                     int nativeevents ATTRIBUTE_UNUSED)
 {
-    unsigned long long now;
-    int ret;
-
-    if (virTimeMillisNow(&now) < 0)
-        return -1;
-
-    virMutexLock(&eventLoop.lock);
-    if (eventLoop.timeoutsCount == eventLoop.timeoutsAlloc) {
-        EVENT_DEBUG("Used %zu timeout slots, adding at least %d more",
-                    eventLoop.timeoutsAlloc, EVENT_ALLOC_EXTENT);
-        if (VIR_RESIZE_N(eventLoop.timeouts, eventLoop.timeoutsAlloc,
-                         eventLoop.timeoutsCount, EVENT_ALLOC_EXTENT) < 0) {
-            virMutexUnlock(&eventLoop.lock);
-            return -1;
-        }
-    }
-
-    eventLoop.timeouts[eventLoop.timeoutsCount].timer = nextTimer++;
-    eventLoop.timeouts[eventLoop.timeoutsCount].frequency = frequency;
-    eventLoop.timeouts[eventLoop.timeoutsCount].cb = cb;
-    eventLoop.timeouts[eventLoop.timeoutsCount].ff = ff;
-    eventLoop.timeouts[eventLoop.timeoutsCount].opaque = opaque;
-    eventLoop.timeouts[eventLoop.timeoutsCount].deleted = 0;
-    eventLoop.timeouts[eventLoop.timeoutsCount].expiresAt =
-        frequency >= 0 ? frequency + now : 0;
-
-    eventLoop.timeoutsCount++;
-    ret = nextTimer-1;
     virEventPollInterruptLocked();
-
-    PROBE(EVENT_POLL_ADD_TIMEOUT,
-          "timer=%d frequency=%d cb=%p opaque=%p ff=%p",
-          ret, frequency, cb, opaque, ff);
-    virMutexUnlock(&eventLoop.lock);
-    return ret;
-}
-
-void virEventPollUpdateTimeout(int timer, int frequency)
-{
-    unsigned long long now;
-    size_t i;
-    bool found = false;
-    PROBE(EVENT_POLL_UPDATE_TIMEOUT,
-          "timer=%d frequency=%d",
-          timer, frequency);
-
-    if (timer <= 0) {
-        VIR_WARN("Ignoring invalid update timer %d", timer);
-        return;
-    }
-
-    if (virTimeMillisNow(&now) < 0)
-        return;
-
-    virMutexLock(&eventLoop.lock);
-    for (i = 0; i < eventLoop.timeoutsCount; i++) {
-        if (eventLoop.timeouts[i].timer == timer) {
-            eventLoop.timeouts[i].frequency = frequency;
-            eventLoop.timeouts[i].expiresAt =
-                frequency >= 0 ? frequency + now : 0;
-            VIR_DEBUG("Set timer freq=%d expires=%llu", frequency,
-                      eventLoop.timeouts[i].expiresAt);
-            virEventPollInterruptLocked();
-            found = true;
-            break;
-        }
-    }
-    virMutexUnlock(&eventLoop.lock);
-
-    if (!found)
-        VIR_WARN("Got update for non-existent timer %d", timer);
-}
-
-/*
- * Unregister a callback for a timer
- * NB, it *must* be safe to call this from within a callback
- * For this reason we only ever set a flag in the existing list.
- * Actual deletion will be done out-of-band
- */
-int virEventPollRemoveTimeout(int timer)
-{
-    size_t i;
-    PROBE(EVENT_POLL_REMOVE_TIMEOUT,
-          "timer=%d",
-          timer);
-
-    if (timer <= 0) {
-        VIR_WARN("Ignoring invalid remove timer %d", timer);
-        return -1;
-    }
-
-    virMutexLock(&eventLoop.lock);
-    for (i = 0; i < eventLoop.timeoutsCount; i++) {
-        if (eventLoop.timeouts[i].deleted)
-            continue;
-
-        if (eventLoop.timeouts[i].timer == timer) {
-            eventLoop.timeouts[i].deleted = 1;
-            virEventPollInterruptLocked();
-            virMutexUnlock(&eventLoop.lock);
-            return 0;
-        }
-    }
-    virMutexUnlock(&eventLoop.lock);
-    return -1;
+    return 0;
 }
 
-/* Iterates over all registered timeouts and determine which
- * will be the first to expire.
- * @timeout: filled with expiry time of soonest timer, or -1 if
- *           no timeout is pending
- * returns: 0 on success, -1 on error
- */
-static int virEventPollCalculateTimeout(int *timeout)
+int virEventPollRemoveHandleInternal(int watch ATTRIBUTE_UNUSED,
+                                     int fd ATTRIBUTE_UNUSED)
 {
-    unsigned long long then = 0;
-    size_t i;
-    EVENT_DEBUG("Calculate expiry of %zu timers", eventLoop.timeoutsCount);
-    /* Figure out if we need a timeout */
-    for (i = 0; i < eventLoop.timeoutsCount; i++) {
-        if (eventLoop.timeouts[i].deleted)
-            continue;
-        if (eventLoop.timeouts[i].frequency < 0)
-            continue;
-
-        EVENT_DEBUG("Got a timeout scheduled for %llu", eventLoop.timeouts[i].expiresAt);
-        if (then == 0 ||
-            eventLoop.timeouts[i].expiresAt < then)
-            then = eventLoop.timeouts[i].expiresAt;
-    }
-
-    /* Calculate how long we should wait for a timeout if needed */
-    if (then > 0) {
-        unsigned long long now;
-
-        if (virTimeMillisNow(&now) < 0)
-            return -1;
-
-        EVENT_DEBUG("Schedule timeout then=%llu now=%llu", then, now);
-        if (then <= now)
-            *timeout = 0;
-        else
-            *timeout = ((then - now) > INT_MAX) ? INT_MAX : (then - now);
-    } else {
-        *timeout = -1;
-    }
-
-    if (*timeout > -1)
-        EVENT_DEBUG("Timeout at %llu due in %d ms", then, *timeout);
-    else
-        EVENT_DEBUG("%s", "No timeout is pending");
-
+    // virEventPollInterruptLocked() is called in common code.
     return 0;
 }
 
@@ -410,332 +96,14 @@ static struct pollfd *virEventPollMakePollFDs(int *nfds) {
     return fds;
 }
 
-
-/*
- * Iterate over all timers and determine if any have expired.
- * Invoke the user supplied callback for each timer whose
- * expiry time is met, and schedule the next timeout. Does
- * not try to 'catch up' on time if the actual expiry time
- * was later than the requested time.
- *
- * This method must cope with new timers being registered
- * by a callback, and must skip any timers marked as deleted.
- *
- * Returns 0 upon success, -1 if an error occurred
- */
-static int virEventPollDispatchTimeouts(void)
-{
-    unsigned long long now;
-    size_t i;
-    /* Save this now - it may be changed during dispatch */
-    int ntimeouts = eventLoop.timeoutsCount;
-    VIR_DEBUG("Dispatch %d", ntimeouts);
-
-    if (virTimeMillisNow(&now) < 0)
-        return -1;
-
-    for (i = 0; i < ntimeouts; i++) {
-        if (eventLoop.timeouts[i].deleted || eventLoop.timeouts[i].frequency < 0)
-            continue;
-
-        /* Add 20ms fuzz so we don't pointlessly spin doing
-         * <10ms sleeps, particularly on kernels with low HZ
-         * it is fine that a timer expires 20ms earlier than
-         * requested
-         */
-        if (eventLoop.timeouts[i].expiresAt <= (now+20)) {
-            virEventTimeoutCallback cb = eventLoop.timeouts[i].cb;
-            int timer = eventLoop.timeouts[i].timer;
-            void *opaque = eventLoop.timeouts[i].opaque;
-            eventLoop.timeouts[i].expiresAt =
-                now + eventLoop.timeouts[i].frequency;
-
-            PROBE(EVENT_POLL_DISPATCH_TIMEOUT,
-                  "timer=%d",
-                  timer);
-            virMutexUnlock(&eventLoop.lock);
-            (cb)(timer, opaque);
-            virMutexLock(&eventLoop.lock);
-        }
-    }
-    return 0;
-}
-
-
-/* Iterate over all file handles and dispatch any which
- * have pending events listed in the poll() data. Invoke
- * the user supplied callback for each handle which has
- * pending events
- *
- * This method must cope with new handles being registered
- * by a callback, and must skip any handles marked as deleted.
- *
- * Returns 0 upon success, -1 if an error occurred
- */
-static int virEventPollDispatchHandles(int nfds, struct pollfd *fds)
-{
-    size_t i, n;
-    VIR_DEBUG("Dispatch %d", nfds);
-
-    /* NB, use nfds not eventLoop.handlesCount, because new
-     * fds might be added on end of list, and they're not
-     * in the fds array we've got */
-    for (i = 0, n = 0; n < nfds && i < eventLoop.handlesCount; n++) {
-        while (i < eventLoop.handlesCount &&
-               (eventLoop.handles[i].fd != fds[n].fd ||
-                eventLoop.handles[i].events == 0)) {
-            i++;
-        }
-        if (i == eventLoop.handlesCount)
-            break;
-
-        VIR_DEBUG("i=%zu w=%d", i, eventLoop.handles[i].watch);
-        if (eventLoop.handles[i].deleted) {
-            EVENT_DEBUG("Skip deleted n=%zu w=%d f=%d", i,
-                        eventLoop.handles[i].watch, eventLoop.handles[i].fd);
-            continue;
-        }
-
-        if (fds[n].revents) {
-            virEventHandleCallback cb = eventLoop.handles[i].cb;
-            int watch = eventLoop.handles[i].watch;
-            void *opaque = eventLoop.handles[i].opaque;
-            int hEvents = virEventPollFromNativeEvents(fds[n].revents);
-            PROBE(EVENT_POLL_DISPATCH_HANDLE,
-                  "watch=%d events=%d",
-                  watch, hEvents);
-            virMutexUnlock(&eventLoop.lock);
-            (cb)(watch, fds[n].fd, hEvents, opaque);
-            virMutexLock(&eventLoop.lock);
-        }
-    }
-
-    return 0;
-}
-
-
-/* Used post dispatch to actually remove any timers that
- * were previously marked as deleted. This asynchronous
- * cleanup is needed to make dispatch re-entrant safe.
- */
-static void virEventPollCleanupTimeouts(void)
-{
-    size_t i;
-    size_t gap;
-    VIR_DEBUG("Cleanup %zu", eventLoop.timeoutsCount);
-
-    /* Remove deleted entries, shuffling down remaining
-     * entries as needed to form contiguous series
-     */
-    for (i = 0; i < eventLoop.timeoutsCount;) {
-        if (!eventLoop.timeouts[i].deleted) {
-            i++;
-            continue;
-        }
-
-        PROBE(EVENT_POLL_PURGE_TIMEOUT,
-              "timer=%d",
-              eventLoop.timeouts[i].timer);
-        if (eventLoop.timeouts[i].ff) {
-            virFreeCallback ff = eventLoop.timeouts[i].ff;
-            void *opaque = eventLoop.timeouts[i].opaque;
-            virMutexUnlock(&eventLoop.lock);
-            ff(opaque);
-            virMutexLock(&eventLoop.lock);
-        }
-
-        if ((i+1) < eventLoop.timeoutsCount) {
-            memmove(eventLoop.timeouts+i,
-                    eventLoop.timeouts+i+1,
-                    sizeof(struct virEventPollTimeout)*(eventLoop.timeoutsCount
-                                                    -(i+1)));
-        }
-        eventLoop.timeoutsCount--;
-    }
-
-    /* Release some memory if we've got a big chunk free */
-    gap = eventLoop.timeoutsAlloc - eventLoop.timeoutsCount;
-    if (eventLoop.timeoutsCount == 0 ||
-        (gap > eventLoop.timeoutsCount && gap > EVENT_ALLOC_EXTENT)) {
-        EVENT_DEBUG("Found %zu out of %zu timeout slots used, releasing %zu",
-                    eventLoop.timeoutsCount, eventLoop.timeoutsAlloc, gap);
-        VIR_SHRINK_N(eventLoop.timeouts, eventLoop.timeoutsAlloc, gap);
-    }
-}
-
-/* Used post dispatch to actually remove any handles that
- * were previously marked as deleted. This asynchronous
- * cleanup is needed to make dispatch re-entrant safe.
- */
-static void virEventPollCleanupHandles(void)
-{
-    size_t i;
-    size_t gap;
-    VIR_DEBUG("Cleanup %zu", eventLoop.handlesCount);
-
-    /* Remove deleted entries, shuffling down remaining
-     * entries as needed to form contiguous series
-     */
-    for (i = 0; i < eventLoop.handlesCount;) {
-        if (!eventLoop.handles[i].deleted) {
-            i++;
-            continue;
-        }
-
-        PROBE(EVENT_POLL_PURGE_HANDLE,
-              "watch=%d",
-              eventLoop.handles[i].watch);
-        if (eventLoop.handles[i].ff) {
-            virFreeCallback ff = eventLoop.handles[i].ff;
-            void *opaque = eventLoop.handles[i].opaque;
-            virMutexUnlock(&eventLoop.lock);
-            ff(opaque);
-            virMutexLock(&eventLoop.lock);
-        }
-
-        if ((i+1) < eventLoop.handlesCount) {
-            memmove(eventLoop.handles+i,
-                    eventLoop.handles+i+1,
-                    sizeof(struct virEventPollHandle)*(eventLoop.handlesCount
-                                                   -(i+1)));
-        }
-        eventLoop.handlesCount--;
-    }
-
-    /* Release some memory if we've got a big chunk free */
-    gap = eventLoop.handlesAlloc - eventLoop.handlesCount;
-    if (eventLoop.handlesCount == 0 ||
-        (gap > eventLoop.handlesCount && gap > EVENT_ALLOC_EXTENT)) {
-        EVENT_DEBUG("Found %zu out of %zu handles slots used, releasing %zu",
-                    eventLoop.handlesCount, eventLoop.handlesAlloc, gap);
-        VIR_SHRINK_N(eventLoop.handles, eventLoop.handlesAlloc, gap);
-    }
-}
-
-/*
- * Run a single iteration of the event loop, blocking until
- * at least one file handle has an event, or a timer expires
- */
-int virEventPollRunOnce(void)
+int virEventPollInitInternal(void)
 {
-    struct pollfd *fds = NULL;
-    int ret, timeout, nfds;
-
-    virMutexLock(&eventLoop.lock);
-    eventLoop.running = 1;
-    virThreadSelf(&eventLoop.leader);
-
-    virEventPollCleanupTimeouts();
-    virEventPollCleanupHandles();
-
-    if (!(fds = virEventPollMakePollFDs(&nfds)) ||
-        virEventPollCalculateTimeout(&timeout) < 0)
-        goto error;
-
-    virMutexUnlock(&eventLoop.lock);
-
- retry:
-    PROBE(EVENT_POLL_RUN,
-          "nhandles=%d timeout=%d",
-          nfds, timeout);
-    ret = poll(fds, nfds, timeout);
-    if (ret < 0) {
-        EVENT_DEBUG("Poll got error event %d", errno);
-        if (errno == EINTR || errno == EAGAIN)
-            goto retry;
-        virReportSystemError(errno, "%s",
-                             _("Unable to poll on file handles"));
-        goto error_unlocked;
-    }
-    EVENT_DEBUG("Poll got %d event(s)", ret);
-
-    virMutexLock(&eventLoop.lock);
-    if (virEventPollDispatchTimeouts() < 0)
-        goto error;
-
-    if (ret > 0 &&
-        virEventPollDispatchHandles(nfds, fds) < 0)
-        goto error;
-
-    virEventPollCleanupTimeouts();
-    virEventPollCleanupHandles();
-
-    eventLoop.running = 0;
-    virMutexUnlock(&eventLoop.lock);
-    VIR_FREE(fds);
-    return 0;
-
- error:
-    virMutexUnlock(&eventLoop.lock);
- error_unlocked:
-    VIR_FREE(fds);
-    return -1;
-}
-
-
-static void virEventPollHandleWakeup(int watch ATTRIBUTE_UNUSED,
-                                     int fd,
-                                     int events ATTRIBUTE_UNUSED,
-                                     void *opaque ATTRIBUTE_UNUSED)
-{
-    char c;
-    virMutexLock(&eventLoop.lock);
-    ignore_value(saferead(fd, &c, sizeof(c)));
-    virMutexUnlock(&eventLoop.lock);
-}
-
-int virEventPollInit(void)
-{
-    if (virMutexInit(&eventLoop.lock) < 0) {
-        virReportSystemError(errno, "%s",
-                             _("Unable to initialize mutex"));
-        return -1;
-    }
-
-    if (pipe2(eventLoop.wakeupfd, O_CLOEXEC | O_NONBLOCK) < 0) {
-        virReportSystemError(errno, "%s",
-                             _("Unable to setup wakeup pipe"));
-        return -1;
-    }
-
-    if (virEventPollAddHandle(eventLoop.wakeupfd[0],
-                              VIR_EVENT_HANDLE_READABLE,
-                              virEventPollHandleWakeup, NULL, NULL) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Unable to add handle %d to event loop"),
-                       eventLoop.wakeupfd[0]);
-        VIR_FORCE_CLOSE(eventLoop.wakeupfd[0]);
-        VIR_FORCE_CLOSE(eventLoop.wakeupfd[1]);
-        return -1;
-    }
-
     return 0;
 }
 
-static int virEventPollInterruptLocked(void)
+void virEventPollDeinitInternal(void)
 {
-    char c = '\0';
-
-    if (!eventLoop.running ||
-        virThreadIsSelf(&eventLoop.leader)) {
-        VIR_DEBUG("Skip interrupt, %d %llu", eventLoop.running,
-                  virThreadID(&eventLoop.leader));
-        return 0;
-    }
-
-    VIR_DEBUG("Interrupting");
-    if (safewrite(eventLoop.wakeupfd[1], &c, sizeof(c)) != sizeof(c))
-        return -1;
-    return 0;
-}
-
-int virEventPollInterrupt(void)
-{
-    int ret;
-    virMutexLock(&eventLoop.lock);
-    ret = virEventPollInterruptLocked();
-    virMutexUnlock(&eventLoop.lock);
-    return ret;
+    return;
 }
 
 int
@@ -769,3 +137,39 @@ virEventPollFromNativeEvents(int events)
         ret |= VIR_EVENT_HANDLE_HANGUP;
     return ret;
 }
+
+int virEventPollWait(int timeout, void **opaque)
+{
+    int ret, nfds;
+    struct pollfd *fds = NULL;
+
+    if (!(fds = virEventPollMakePollFDs(&nfds)))
+        return -1;
+    *opaque = fds;
+
+ retry:
+    ret = poll(fds, nfds, timeout);
+    if (ret < 0) {
+        EVENT_DEBUG("Poll got error event %d", errno);
+        if (errno == EINTR || errno == EAGAIN)
+            goto retry;
+        virReportSystemError(errno, "%s",
+                             _("Unable to poll on file handles"));
+    }
+    return nfds;
+}
+
+void virEventPollOpaqueFree(void *opaque)
+{
+    VIR_FREE(opaque);
+}
+
+int VirWokenFD(void *opaque, size_t n)
+{
+    return ((struct pollfd *)opaque)[n].fd;
+}
+
+int VirWokenEvents(void *opaque, size_t n)
+{
+    return ((struct pollfd *)opaque)[n].revents;
+}
diff --git a/src/util/vireventpoll.c b/src/util/vireventpollcommon.c
similarity index 78%
copy from src/util/vireventpoll.c
copy to src/util/vireventpollcommon.c
index 81ecab4..6a3a372 100644
--- a/src/util/vireventpoll.c
+++ b/src/util/vireventpollcommon.c
@@ -25,7 +25,6 @@
 
 #include <stdlib.h>
 #include <string.h>
-#include <poll.h>
 #include <sys/time.h>
 #include <errno.h>
 #include <unistd.h>
@@ -34,6 +33,7 @@
 #include "virthread.h"
 #include "virlog.h"
 #include "vireventpoll.h"
+#include "vireventpollinternal.h"
 #include "viralloc.h"
 #include "virutil.h"
 #include "virfile.h"
@@ -47,56 +47,9 @@
 
 VIR_LOG_INIT("util.eventpoll");
 
-static int virEventPollInterruptLocked(void);
-
-/* State for a single file handle being monitored */
-struct virEventPollHandle {
-    int watch;
-    int fd;
-    int events;
-    virEventHandleCallback cb;
-    virFreeCallback ff;
-    void *opaque;
-    int deleted;
-};
-
-/* State for a single timer being generated */
-struct virEventPollTimeout {
-    int timer;
-    int frequency;
-    unsigned long long expiresAt;
-    virEventTimeoutCallback cb;
-    virFreeCallback ff;
-    void *opaque;
-    int deleted;
-};
-
-/* Allocate extra slots for virEventPollHandle/virEventPollTimeout
-   records in this multiple */
-#define EVENT_ALLOC_EXTENT 10
-
-/* State for the main event loop */
-struct virEventPollLoop {
-    virMutex lock;
-    int running;
-    virThread leader;
-    int wakeupfd[2];
-    size_t handlesCount;
-    size_t handlesAlloc;
-    struct virEventPollHandle *handles;
-    size_t timeoutsCount;
-    size_t timeoutsAlloc;
-    struct virEventPollTimeout *timeouts;
-};
-
-/* Only have one event loop */
-static struct virEventPollLoop eventLoop;
-
-/* Unique ID for the next FD watch to be registered */
-static int nextWatch = 1;
-
-/* Unique ID for the next timer to be registered */
-static int nextTimer = 1;
+struct virEventPollLoop eventLoop;
+int nextWatch = 1;
+int nextTimer = 1;
 
 /*
  * Register a callback for monitoring file handle events.
@@ -109,6 +62,7 @@ int virEventPollAddHandle(int fd, int events,
                           virFreeCallback ff)
 {
     int watch;
+    int native = virEventPollToNativeEvents(events);
     virMutexLock(&eventLoop.lock);
     if (eventLoop.handlesCount == eventLoop.handlesAlloc) {
         EVENT_DEBUG("Used %zu handle slots, adding at least %d more",
@@ -124,8 +78,7 @@ int virEventPollAddHandle(int fd, int events,
 
     eventLoop.handles[eventLoop.handlesCount].watch = watch;
     eventLoop.handles[eventLoop.handlesCount].fd = fd;
-    eventLoop.handles[eventLoop.handlesCount].events =
-                                         virEventPollToNativeEvents(events);
+    eventLoop.handles[eventLoop.handlesCount].events = native;
     eventLoop.handles[eventLoop.handlesCount].cb = cb;
     eventLoop.handles[eventLoop.handlesCount].ff = ff;
     eventLoop.handles[eventLoop.handlesCount].opaque = opaque;
@@ -133,7 +86,10 @@ int virEventPollAddHandle(int fd, int events,
 
     eventLoop.handlesCount++;
 
-    virEventPollInterruptLocked();
+    if (virEventPollAddHandleInternal(watch, fd, native) < 0) {
+        virMutexUnlock(&eventLoop.lock);
+        return -1;
+    }
 
     PROBE(EVENT_POLL_ADD_HANDLE,
           "watch=%d fd=%d events=%d cb=%p opaque=%p ff=%p",
@@ -147,6 +103,7 @@ void virEventPollUpdateHandle(int watch, int events)
 {
     size_t i;
     bool found = false;
+    int native = virEventPollToNativeEvents(events);
     PROBE(EVENT_POLL_UPDATE_HANDLE,
           "watch=%d events=%d",
           watch, events);
@@ -159,17 +116,21 @@ void virEventPollUpdateHandle(int watch, int events)
     virMutexLock(&eventLoop.lock);
     for (i = 0; i < eventLoop.handlesCount; i++) {
         if (eventLoop.handles[i].watch == watch) {
-            eventLoop.handles[i].events =
-                    virEventPollToNativeEvents(events);
-            virEventPollInterruptLocked();
+            eventLoop.handles[i].events = native;
             found = true;
             break;
         }
     }
-    virMutexUnlock(&eventLoop.lock);
 
-    if (!found)
+    if (!found) {
         VIR_WARN("Got update for non-existent handle watch %d", watch);
+    }
+    else if (virEventPollUpdateHandleInternal(watch,
+                                         eventLoop.handles[i].fd, native) < 0) {
+        VIR_WARN("Update for existing handle watch %d failed", watch);
+    }
+
+    virMutexUnlock(&eventLoop.lock);
 }
 
 /*
@@ -196,15 +157,25 @@ int virEventPollRemoveHandle(int watch)
             continue;
 
         if (eventLoop.handles[i].watch == watch) {
-            EVENT_DEBUG("mark delete %zu %d", i, eventLoop.handles[i].fd);
-            eventLoop.handles[i].deleted = 1;
-            virEventPollInterruptLocked();
-            virMutexUnlock(&eventLoop.lock);
-            return 0;
+            break;
         }
     }
+
+    if (i == eventLoop.handlesCount) {
+        virMutexUnlock(&eventLoop.lock);
+        return -1;
+    }
+
+    if (virEventPollRemoveHandleInternal(watch, eventLoop.handles[i].fd) < 0) {
+        virMutexUnlock(&eventLoop.lock);
+        return -1;
+    }
+
+    EVENT_DEBUG("mark delete %zu %d", i, eventLoop.handles[i].fd);
+    eventLoop.handles[i].deleted = 1;
+    virEventPollInterruptLocked();
     virMutexUnlock(&eventLoop.lock);
-    return -1;
+    return 0;
 }
 
 
@@ -374,44 +345,6 @@ static int virEventPollCalculateTimeout(int *timeout)
 }
 
 /*
- * Allocate a pollfd array containing data for all registered
- * file handles. The caller must free the returned data struct
- * returns: the pollfd array, or NULL on error
- */
-static struct pollfd *virEventPollMakePollFDs(int *nfds) {
-    struct pollfd *fds;
-    size_t i;
-
-    *nfds = 0;
-    for (i = 0; i < eventLoop.handlesCount; i++) {
-        if (eventLoop.handles[i].events && !eventLoop.handles[i].deleted)
-            (*nfds)++;
-    }
-
-    /* Setup the poll file handle data structs */
-    if (VIR_ALLOC_N(fds, *nfds) < 0)
-        return NULL;
-
-    *nfds = 0;
-    for (i = 0; i < eventLoop.handlesCount; i++) {
-        EVENT_DEBUG("Prepare n=%zu w=%d, f=%d e=%d d=%d", i,
-                    eventLoop.handles[i].watch,
-                    eventLoop.handles[i].fd,
-                    eventLoop.handles[i].events,
-                    eventLoop.handles[i].deleted);
-        if (!eventLoop.handles[i].events || eventLoop.handles[i].deleted)
-            continue;
-        fds[*nfds].fd = eventLoop.handles[i].fd;
-        fds[*nfds].events = eventLoop.handles[i].events;
-        fds[*nfds].revents = 0;
-        (*nfds)++;
-    }
-
-    return fds;
-}
-
-
-/*
  * Iterate over all timers and determine if any have expired.
  * Invoke the user supplied callback for each timer whose
  * expiry time is met, and schedule the next timeout. Does
@@ -472,7 +405,7 @@ static int virEventPollDispatchTimeouts(void)
  *
  * Returns 0 upon success, -1 if an error occurred
  */
-static int virEventPollDispatchHandles(int nfds, struct pollfd *fds)
+static int virEventPollDispatchHandles(int nfds, void *opaque)
 {
     size_t i, n;
     VIR_DEBUG("Dispatch %d", nfds);
@@ -480,14 +413,14 @@ static int virEventPollDispatchHandles(int nfds, struct pollfd *fds)
     /* NB, use nfds not eventLoop.handlesCount, because new
      * fds might be added on end of list, and they're not
      * in the fds array we've got */
-    for (i = 0, n = 0; n < nfds && i < eventLoop.handlesCount; n++) {
-        while (i < eventLoop.handlesCount &&
-               (eventLoop.handles[i].fd != fds[n].fd ||
-                eventLoop.handles[i].events == 0)) {
-            i++;
+    for (n = 0; n < nfds; n++) {
+        for (i = 0; i < eventLoop.handlesCount &&
+             (eventLoop.handles[i].fd != VirWokenFD(opaque, n) ||
+              eventLoop.handles[i].events == 0) ; i++) {
+            ;
         }
         if (i == eventLoop.handlesCount)
-            break;
+            continue;
 
         VIR_DEBUG("i=%zu w=%d", i, eventLoop.handles[i].watch);
         if (eventLoop.handles[i].deleted) {
@@ -496,16 +429,16 @@ static int virEventPollDispatchHandles(int nfds, struct pollfd *fds)
             continue;
         }
 
-        if (fds[n].revents) {
+        if (VirWokenEvents(opaque, n)) {
+            int hEvents = virEventPollFromNativeEvents(VirWokenEvents(opaque, n));
             virEventHandleCallback cb = eventLoop.handles[i].cb;
             int watch = eventLoop.handles[i].watch;
-            void *opaque = eventLoop.handles[i].opaque;
-            int hEvents = virEventPollFromNativeEvents(fds[n].revents);
+            void *cbopaque = eventLoop.handles[i].opaque;
             PROBE(EVENT_POLL_DISPATCH_HANDLE,
                   "watch=%d events=%d",
                   watch, hEvents);
             virMutexUnlock(&eventLoop.lock);
-            (cb)(watch, fds[n].fd, hEvents, opaque);
+            (cb)(watch, VirWokenFD(opaque, n), hEvents, cbopaque);
             virMutexLock(&eventLoop.lock);
         }
     }
@@ -618,8 +551,8 @@ static void virEventPollCleanupHandles(void)
  */
 int virEventPollRunOnce(void)
 {
-    struct pollfd *fds = NULL;
-    int ret, timeout, nfds;
+    int nfds, timeout;
+    void *opaque = NULL;
 
     virMutexLock(&eventLoop.lock);
     eventLoop.running = 1;
@@ -628,33 +561,20 @@ int virEventPollRunOnce(void)
     virEventPollCleanupTimeouts();
     virEventPollCleanupHandles();
 
-    if (!(fds = virEventPollMakePollFDs(&nfds)) ||
-        virEventPollCalculateTimeout(&timeout) < 0)
+    if (virEventPollCalculateTimeout(&timeout) < 0)
         goto error;
 
     virMutexUnlock(&eventLoop.lock);
 
- retry:
-    PROBE(EVENT_POLL_RUN,
-          "nhandles=%d timeout=%d",
-          nfds, timeout);
-    ret = poll(fds, nfds, timeout);
-    if (ret < 0) {
-        EVENT_DEBUG("Poll got error event %d", errno);
-        if (errno == EINTR || errno == EAGAIN)
-            goto retry;
-        virReportSystemError(errno, "%s",
-                             _("Unable to poll on file handles"));
+    if ((nfds = virEventPollWait(timeout, &opaque)) < 0)
         goto error_unlocked;
-    }
-    EVENT_DEBUG("Poll got %d event(s)", ret);
+    EVENT_DEBUG("Poll got %d event(s)", nfds);
 
     virMutexLock(&eventLoop.lock);
     if (virEventPollDispatchTimeouts() < 0)
         goto error;
 
-    if (ret > 0 &&
-        virEventPollDispatchHandles(nfds, fds) < 0)
+    if (nfds > 0 && virEventPollDispatchHandles(nfds, opaque) < 0)
         goto error;
 
     virEventPollCleanupTimeouts();
@@ -662,13 +582,13 @@ int virEventPollRunOnce(void)
 
     eventLoop.running = 0;
     virMutexUnlock(&eventLoop.lock);
-    VIR_FREE(fds);
+    virEventPollOpaqueFree(opaque);
     return 0;
 
  error:
     virMutexUnlock(&eventLoop.lock);
  error_unlocked:
-    VIR_FREE(fds);
+    virEventPollOpaqueFree(opaque);
     return -1;
 }
 
@@ -698,6 +618,12 @@ int virEventPollInit(void)
         return -1;
     }
 
+    if (virEventPollInitInternal() < 0) {
+        VIR_FORCE_CLOSE(eventLoop.wakeupfd[0]);
+        VIR_FORCE_CLOSE(eventLoop.wakeupfd[1]);
+        return -1;
+    }
+
     if (virEventPollAddHandle(eventLoop.wakeupfd[0],
                               VIR_EVENT_HANDLE_READABLE,
                               virEventPollHandleWakeup, NULL, NULL) < 0) {
@@ -706,13 +632,14 @@ int virEventPollInit(void)
                        eventLoop.wakeupfd[0]);
         VIR_FORCE_CLOSE(eventLoop.wakeupfd[0]);
         VIR_FORCE_CLOSE(eventLoop.wakeupfd[1]);
+        virEventPollDeinitInternal();
         return -1;
     }
 
     return 0;
 }
 
-static int virEventPollInterruptLocked(void)
+int virEventPollInterruptLocked(void)
 {
     char c = '\0';
 
@@ -737,35 +664,3 @@ int virEventPollInterrupt(void)
     virMutexUnlock(&eventLoop.lock);
     return ret;
 }
-
-int
-virEventPollToNativeEvents(int events)
-{
-    int ret = 0;
-    if (events & VIR_EVENT_HANDLE_READABLE)
-        ret |= POLLIN;
-    if (events & VIR_EVENT_HANDLE_WRITABLE)
-        ret |= POLLOUT;
-    if (events & VIR_EVENT_HANDLE_ERROR)
-        ret |= POLLERR;
-    if (events & VIR_EVENT_HANDLE_HANGUP)
-        ret |= POLLHUP;
-    return ret;
-}
-
-int
-virEventPollFromNativeEvents(int events)
-{
-    int ret = 0;
-    if (events & POLLIN)
-        ret |= VIR_EVENT_HANDLE_READABLE;
-    if (events & POLLOUT)
-        ret |= VIR_EVENT_HANDLE_WRITABLE;
-    if (events & POLLERR)
-        ret |= VIR_EVENT_HANDLE_ERROR;
-    if (events & POLLNVAL) /* Treat NVAL as error, since libvirt doesn't distinguish */
-        ret |= VIR_EVENT_HANDLE_ERROR;
-    if (events & POLLHUP)
-        ret |= VIR_EVENT_HANDLE_HANGUP;
-    return ret;
-}
diff --git a/src/util/vireventpollinternal.h b/src/util/vireventpollinternal.h
new file mode 100644
index 0000000..19d2fd9
--- /dev/null
+++ b/src/util/vireventpollinternal.h
@@ -0,0 +1,91 @@
+/*
+ * vireventpoll.h: Poll based event loop for monitoring file handles
+ *
+ * Copyright (C) 2007 Daniel P. Berrange
+ * Copyright (C) 2007 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Daniel P. Berrange <berrange at redhat.com>
+ */
+
+#ifndef __VIR_EVENT_POLL_INTERNAL_H__
+# define __VIR_EVENT_POLL_INTERNAL_H__
+
+# include "internal.h"
+#include "virthread.h"
+
+int virEventPollInterruptLocked(void);
+
+/* State for a single file handle being monitored */
+struct virEventPollHandle {
+    int watch;
+    int fd;
+    int events;
+    virEventHandleCallback cb;
+    virFreeCallback ff;
+    void *opaque;
+    int deleted;
+};
+
+/* State for a single timer being generated */
+struct virEventPollTimeout {
+    int timer;
+    int frequency;
+    unsigned long long expiresAt;
+    virEventTimeoutCallback cb;
+    virFreeCallback ff;
+    void *opaque;
+    int deleted;
+};
+
+/* Allocate extra slots for virEventPollHandle/virEventPollTimeout
+   records in this multiple */
+#define EVENT_ALLOC_EXTENT 10
+
+/* State for the main event loop */
+struct virEventPollLoop {
+    virMutex lock;
+    int running;
+    virThread leader;
+    int wakeupfd[2];
+    size_t handlesCount;
+    size_t handlesAlloc;
+    struct virEventPollHandle *handles;
+    size_t timeoutsCount;
+    size_t timeoutsAlloc;
+    struct virEventPollTimeout *timeouts;
+};
+
+/* Only have one event loop */
+extern struct virEventPollLoop eventLoop;
+
+/* Unique ID for the next FD watch to be registered */
+extern int nextWatch;
+
+/* Unique ID for the next timer to be registered */
+extern int nextTimer;
+
+int virEventPollAddHandleInternal(int watch, int fd, int nativeevents);
+int virEventPollUpdateHandleInternal(int watch, int fd, int nativeevents);
+int virEventPollRemoveHandleInternal(int watch, int fd);
+int virEventPollInitInternal(void);
+void virEventPollDeinitInternal(void);
+int virEventPollWait(int timeout, void **opaque);
+void virEventPollOpaqueFree(void *opaque);
+int VirWokenFD(void *opaque, size_t n);
+int VirWokenEvents(void *opaque, size_t n);
+
+#endif /* __VIR_EVENT_POLL_INTERNAL_H__ */
-- 
1.9.5.msysgit.0




More information about the libvir-list mailing list