[libvirt] PATCH: 25/28: Thread safety for libvirtd event loop

[Daniel Veillard]

On Mon, Dec 01, 2008 at 12:18:24AM +0000, Daniel P. Berrange wrote:
> This patch makes the event loop in the libvirtd daemon thread safe, and
> re-entrant. This allows one thread to add/remove/update timers/file handle
> watches while other thread is doing the poll. This sometimes requires that
> we wakeup the main thread to make it see changes to the poll FD list. We
> use the traditional self-pipe trick for this task.
[...]
> -static int virEventDispatchHandles(struct pollfd *fds) {
> +static int virEventDispatchHandles(int nfds, struct pollfd *fds) {
>      int i;
> -    virEventHandleType hEvents;
> -    /* Save this now - it may be changed during dispatch */
> -    int nhandles = eventLoop.handlesCount;
>  
> -    for (i = 0 ; i < nhandles ; i++) {
> +    for (i = 0 ; i < nfds ; i++) {
>          if (eventLoop.handles[i].deleted) {
>              EVENT_DEBUG("Skip deleted %d", eventLoop.handles[i].fd);
>              continue;
>          }
>  
>          if (fds[i].revents) {
> -            hEvents = virPollEventToEventHandleType(fds[i].revents);
> -            EVENT_DEBUG("Dispatch %d %d %d %p",
> -                        eventLoop.handles[i].watch,
> -                        fds[i].fd, fds[i].revents,
> -                        eventLoop.handles[i].opaque);
> -            (eventLoop.handles[i].cb)(eventLoop.handles[i].watch,
> -                                      fds[i].fd,
> -                                      hEvents,
> -                                      eventLoop.handles[i].opaque);
> +            virEventHandleCallback cb = eventLoop.handles[i].cb;
> +            void *opaque = eventLoop.handles[i].opaque;
> +            int hEvents = virPollEventToEventHandleType(fds[i].revents);
> +            EVENT_DEBUG("Dispatch %d %d %p", fds[i].fd,
> +                        fds[i].revents, eventLoop.handles[i].opaque);
> +            virEventUnlock();
> +            (cb)(eventLoop.handles[i].watch,
> +                 fds[i].fd, hEvents, opaque);
> +            virEventLock();
>          }
>      }

  Hum, if this routine assume we enter it with the event lock and keep
it though its lifetime except when dispatching, then I guess it's worth
a comment, right ? Is there only one thread which can access or
unregister fds, or do we have another lock in place ?

  Looks okay, but it's far from trivial,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/