This patch fixes handling of deleted file handles. First of all it reverts the patch http://www.redhat.com/archives/libvir-list/2009-March/msg00246.html As an alternative solution to that original problem, the first thing that virEventRunOnce() does, is to purge all timers/watches which were marked as deleted. This deals with deletes that happen between invocations of the virEventRunOnce() method. It also ensures that when going into poll(), all the registered timers/watches are live. This guarentees that array indexes match up between the poll FD array and our list of watches. Then during the dispatch of FDs, we have a simpler check to skip invocation of file handles / timers marked as deleted. Daniel diff -r 63d6824fe2a3 qemud/event.c --- a/qemud/event.c Fri May 08 16:06:40 2009 +0100 +++ b/qemud/event.c Fri May 08 16:07:04 2009 +0100 @@ -313,7 +313,7 @@ static int virEventCalculateTimeout(int EVENT_DEBUG("Calculate expiry of %d timers", eventLoop.timeoutsCount); /* Figure out if we need a timeout */ for (i = 0 ; i < eventLoop.timeoutsCount ; i++) { - if (eventLoop.timeouts[i].deleted || eventLoop.timeouts[i].frequency < 0) + if (eventLoop.timeouts[i].frequency < 0) continue; EVENT_DEBUG("Got a timeout scheduled for %llu", eventLoop.timeouts[i].expiresAt); @@ -350,32 +350,26 @@ static int virEventCalculateTimeout(int * file handles. The caller must free the returned data struct * returns: the pollfd array, or NULL on error */ -static int virEventMakePollFDs(struct pollfd **retfds) { +static struct pollfd *virEventMakePollFDs(void) { struct pollfd *fds; - int i, nfds = 0; + int i; + + /* Setup the poll file handle data structs */ + if (VIR_ALLOC_N(fds, eventLoop.handlesCount) < 0) + return NULL; for (i = 0 ; i < eventLoop.handlesCount ; i++) { - if (eventLoop.handles[i].deleted) - continue; - nfds++; - } - *retfds = NULL; - /* Setup the poll file handle data structs */ - if (VIR_ALLOC_N(fds, nfds) < 0) - return -1; - - for (i = 0, nfds = 0 ; i < eventLoop.handlesCount ; i++) { - if (eventLoop.handles[i].deleted) - continue; - fds[nfds].fd = eventLoop.handles[i].fd; - fds[nfds].events = eventLoop.handles[i].events; - fds[nfds].revents = 0; + EVENT_DEBUG("Prepare n=%d w=%d, f=%d e=%d", i, + eventLoop.handles[i].watch, + eventLoop.handles[i].fd, + eventLoop.handles[i].events); + fds[i].fd = eventLoop.handles[i].fd; + fds[i].events = eventLoop.handles[i].events; + fds[i].revents = 0; //EVENT_DEBUG("Wait for %d %d", eventLoop.handles[i].fd, eventLoop.handles[i].events); - nfds++; } - *retfds = fds; - return nfds; + return fds; } @@ -435,26 +429,30 @@ static int virEventDispatchTimeouts(void * Returns 0 upon success, -1 if an error occurred */ static int virEventDispatchHandles(int nfds, struct pollfd *fds) { - int i, n; + int i; - for (i = 0, n = 0 ; i < eventLoop.handlesCount && n < nfds ; i++) { + /* 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 ; i < nfds ; i++) { if (eventLoop.handles[i].deleted) { - EVENT_DEBUG("Skip deleted %d", eventLoop.handles[i].fd); + EVENT_DEBUG("Skip deleted n=%d w=%d f=%d", i, + eventLoop.handles[i].watch, eventLoop.handles[i].fd); continue; } - if (fds[n].revents) { + if (fds[i].revents) { virEventHandleCallback cb = eventLoop.handles[i].cb; void *opaque = eventLoop.handles[i].opaque; - int hEvents = virPollEventToEventHandleType(fds[n].revents); - EVENT_DEBUG("Dispatch %d %d %p", fds[n].fd, - fds[n].revents, eventLoop.handles[i].opaque); + int hEvents = virPollEventToEventHandleType(fds[i].revents); + EVENT_DEBUG("Dispatch n=%d f=%d w=%d e=%d %p", i, + fds[i].fd, eventLoop.handles[i].watch, + fds[i].revents, eventLoop.handles[i].opaque); virEventUnlock(); (cb)(eventLoop.handles[i].watch, - fds[n].fd, hEvents, opaque); + fds[i].fd, hEvents, opaque); virEventLock(); } - n++; } return 0; @@ -545,22 +543,21 @@ static int virEventCleanupHandles(void) * at least one file handle has an event, or a timer expires */ int virEventRunOnce(void) { - struct pollfd *fds; + struct pollfd *fds = NULL; int ret, timeout, nfds; virEventLock(); eventLoop.running = 1; eventLoop.leader = pthread_self(); - if ((nfds = virEventMakePollFDs(&fds)) < 0) { - virEventUnlock(); - return -1; - } - if (virEventCalculateTimeout(&timeout) < 0) { - VIR_FREE(fds); - virEventUnlock(); - return -1; - } + if (virEventCleanupTimeouts() < 0 || + virEventCleanupHandles() < 0) + goto error; + + if (!(fds = virEventMakePollFDs()) || + virEventCalculateTimeout(&timeout) < 0) + goto error; + nfds = eventLoop.handlesCount; virEventUnlock(); @@ -572,38 +569,31 @@ int virEventRunOnce(void) { if (errno == EINTR) { goto retry; } - VIR_FREE(fds); - return -1; + goto error_unlocked; } virEventLock(); - if (virEventDispatchTimeouts() < 0) { - VIR_FREE(fds); - virEventUnlock(); - return -1; - } + if (virEventDispatchTimeouts() < 0) + goto error; if (ret > 0 && - virEventDispatchHandles(nfds, fds) < 0) { - VIR_FREE(fds); - virEventUnlock(); - return -1; - } - VIR_FREE(fds); + virEventDispatchHandles(nfds, fds) < 0) + goto error; - if (virEventCleanupTimeouts() < 0) { - virEventUnlock(); - return -1; - } - - if (virEventCleanupHandles() < 0) { - virEventUnlock(); - return -1; - } + if (virEventCleanupTimeouts() < 0 || + virEventCleanupHandles() < 0) + goto error; eventLoop.running = 0; virEventUnlock(); + VIR_FREE(fds); return 0; + +error: + virEventUnlock(); +error_unlocked: + VIR_FREE(fds); + return -1; } static void virEventHandleWakeup(int watch ATTRIBUTE_UNUSED, -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list