[libvirt] [PATCH 11/21] Move libvirtd event loop into background thread

[Daniel Veillard]

On Fri, Oct 23, 2009 at 02:05:40PM +0100, Daniel P. Berrange wrote:
> The virStateInitialize() call for starting up stateful drivers
> may require that the event loop is running already. This it is
> neccessary to start the event loop before this call. At the
> same time, network clients must not be processed until afte
> virStateInitialize has completed.
> 
> The qemudListenUnix() and remoteListenTCP() methods must
> therefore not register file handle watches, merely open the
> network sockets & listen() on them. This means clients can
> connected and are queued, pending completion of initialization
> 
> The qemudRunLoop() method is moved into a background thread
> that is started early to allow access to the event loop during
> driver initialization. The main process thread leader pretty
> much does nothing once the daemon is running, merely waits
> for the event loop thread to quit

  I guess this calls for a extra page of documentation about the
threading startup of the server, i.e. what starts when. threaded
C code is really hard to follow and if you don't have the big
picture (a single picture might be sufficient !) it rather hard
to get things just from reading code.

> * daemon/libvirtd.c, daemon/libvirtd.h: Move event loop into
>   a background thread
> * src/driver.h: Add a 'name' field to state driver to allow
>   easy identification during failures
> * src/libvirt.c: Log name of failed driver for virStateInit
>   failures
> * src/lxc/lxc_driver.c: Don't return a failure code for
>   lxcStartup() if LXC is not available on this host, simply
>   disable the driver.
> * src/network/bridge_driver.c, src/node_device/node_device_devkit.c,
>   src/node_device/node_device_hal.c, src/opennebula/one_driver.c,
>   src/qemu/qemu_driver.c, src/remote/remote_driver.c,
>   src/secret/secret_driver.c, src/storage/storage_driver.c,
>   src/uml/uml_driver.c, src/xen/xen_driver.c: Fill in name
>   field in virStateDriver struct
> ---
>  daemon/libvirtd.c                    |  115 +++++++++++++++++++++++-----------
>  daemon/libvirtd.h                    |    4 +-
>  src/driver.h                         |    1 +
>  src/libvirt.c                        |    5 +-
>  src/lxc/lxc_driver.c                 |   24 ++++---
>  src/network/bridge_driver.c          |    1 +
>  src/node_device/node_device_devkit.c |    1 +
>  src/node_device/node_device_hal.c    |    1 +
>  src/opennebula/one_driver.c          |    1 +
>  src/qemu/qemu_driver.c               |    1 +
>  src/remote/remote_driver.c           |    1 +
>  src/secret/secret_driver.c           |    1 +
>  src/storage/storage_driver.c         |    1 +
>  src/uml/uml_driver.c                 |    1 +
>  src/xen/xen_driver.c                 |    1 +
>  15 files changed, 111 insertions(+), 48 deletions(-)
> 
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index b118da8..a20bbb8 100644

>  
> -    for (;;) {
> +    for (;!server->quitEventThread;) {

  while (!server->quitEventThread) { 

is a bit more readable and less error prone :-)

>          /* A shutdown timeout is specified, so check
>           * if any drivers have active state, if not
>           * shutdown after timeout seconds
> @@ -2310,11 +2309,6 @@ static int qemudRunLoop(struct qemud_server *server) {
>                  server->nactiveworkers--;
>              }
>          }
> +
> +static int qemudStartEventLoop(struct qemud_server *server) {
> +    pthread_attr_t attr;
> +    pthread_attr_init(&attr);
> +    /* We want to join the eventloop, so don't detach it */
> +    /*pthread_attr_setdetachstate(&attr, 1);*/

  should we really keep that comment ? it's a bit confusing no ?

> +    if (pthread_create(&server->eventThread,
> +                       &attr,
> +                       qemudRunLoop,
> +                       server) != 0)
> +        return -1;
> +
> +    server->hasEventThread = 1;
> +
> +    return 0;
> +}
[...]
>  
> +shutdown:
> +    /* In a non-0 shutdown scenario we need to tell event loop
> +     * to quit immediately. Otherwise in normal case we just
> +     * sit in the thread join forever. Sure this means the
> +     * main thread doesn't do anything useful ever, but that's
> +     * not too much of drain on resources
> +     */
> +    if (ret != 0) {
> +        virMutexLock(&server->lock);
> +        if (server->hasEventThread)
> +            /* This SIGQUIT triggers the shutdown process */
> +            kill(getpid(), SIGQUIT);
> +        virMutexUnlock(&server->lock);
> +    }
> +    pthread_join(server->eventThread, NULL);

  humpf .... okay ...


  ACK, there is some cleanups merged in that patch too,

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/