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

Daniel P. Berrange berrange at redhat.com
Fri Oct 23 13:05:40 UTC 2009


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

* 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
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -384,7 +384,7 @@ qemudDispatchSignalEvent(int watch ATTRIBUTE_UNUSED,
     case SIGQUIT:
     case SIGTERM:
         VIR_WARN(_("Shutting down on signal %d"), siginfo.si_signo);
-        server->shutdown = 1;
+        server->quitEventThread = 1;
         break;
 
     default:
@@ -393,7 +393,7 @@ qemudDispatchSignalEvent(int watch ATTRIBUTE_UNUSED,
     }
 
     if (ret != 0)
-        server->shutdown = 1;
+        server->quitEventThread = 1;
 
     virMutexUnlock(&server->lock);
 }
@@ -579,16 +579,6 @@ static int qemudListenUnix(struct qemud_server *server,
         goto cleanup;
     }
 
-    if ((sock->watch = virEventAddHandleImpl(sock->fd,
-                                             VIR_EVENT_HANDLE_READABLE |
-                                             VIR_EVENT_HANDLE_ERROR |
-                                             VIR_EVENT_HANDLE_HANGUP,
-                                             qemudDispatchServerEvent,
-                                             server, NULL)) < 0) {
-        VIR_ERROR0(_("Failed to add server event callback"));
-        goto cleanup;
-    }
-
     sock->next = server->sockets;
     server->sockets = sock;
     server->nsockets++;
@@ -713,17 +703,6 @@ remoteListenTCP (struct qemud_server *server,
                       virStrerror (errno, ebuf, sizeof ebuf));
             goto cleanup;
         }
-
-        if ((sock->watch = virEventAddHandleImpl(sock->fd,
-                                                 VIR_EVENT_HANDLE_READABLE |
-                                                 VIR_EVENT_HANDLE_ERROR |
-                                                 VIR_EVENT_HANDLE_HANGUP,
-                                                 qemudDispatchServerEvent,
-                                                 server, NULL)) < 0) {
-            VIR_ERROR0(_("Failed to add server event callback"));
-            goto cleanup;
-        }
-
     }
 
     return 0;
@@ -1033,6 +1012,25 @@ static int qemudNetworkInit(struct qemud_server *server) {
     return -1;
 }
 
+static int qemudNetworkEnable(struct qemud_server *server) {
+    struct qemud_socket *sock;
+
+    sock = server->sockets;
+    while (sock) {
+        if ((sock->watch = virEventAddHandleImpl(sock->fd,
+                                                 VIR_EVENT_HANDLE_READABLE |
+                                                 VIR_EVENT_HANDLE_ERROR |
+                                                 VIR_EVENT_HANDLE_HANGUP,
+                                                 qemudDispatchServerEvent,
+                                                 server, NULL)) < 0) {
+            VIR_ERROR0(_("Failed to add server event callback"));
+            return -1;
+        }
+
+        sock = sock->next;
+    }
+    return 0;
+}
 
 static gnutls_session_t
 remoteInitializeTLSSession (void)
@@ -2178,7 +2176,7 @@ static void qemudInactiveTimer(int timerid, void *data) {
         virEventUpdateTimeoutImpl(timerid, -1);
     } else {
         DEBUG0("Timer expired and inactive, shutting down");
-        server->shutdown = 1;
+        server->quitEventThread = 1;
     }
 }
 
@@ -2208,9 +2206,10 @@ static void qemudFreeClient(struct qemud_client *client) {
     VIR_FREE(client);
 }
 
-static int qemudRunLoop(struct qemud_server *server) {
+static void *qemudRunLoop(void *opaque) {
+    struct qemud_server *server = opaque;
     int timerid = -1;
-    int ret = -1, i;
+    int i;
     int timerActive = 0;
 
     virMutexLock(&server->lock);
@@ -2220,7 +2219,7 @@ static int qemudRunLoop(struct qemud_server *server) {
                                           qemudInactiveTimer,
                                           server, NULL)) < 0) {
         VIR_ERROR0(_("Failed to register shutdown timeout"));
-        return -1;
+        return NULL;
     }
 
     if (min_workers > max_workers)
@@ -2229,7 +2228,7 @@ static int qemudRunLoop(struct qemud_server *server) {
     server->nworkers = max_workers;
     if (VIR_ALLOC_N(server->workers, server->nworkers) < 0) {
         VIR_ERROR0(_("Failed to allocate workers"));
-        return -1;
+        return NULL;
     }
 
     for (i = 0 ; i < min_workers ; i++) {
@@ -2238,7 +2237,7 @@ static int qemudRunLoop(struct qemud_server *server) {
         server->nactiveworkers++;
     }
 
-    for (;;) {
+    for (;!server->quitEventThread;) {
         /* 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--;
             }
         }
-
-        if (server->shutdown) {
-            ret = 0;
-            break;
-        }
     }
 
 cleanup:
@@ -2333,9 +2327,28 @@ cleanup:
     VIR_FREE(server->workers);
 
     virMutexUnlock(&server->lock);
-    return ret;
+    return NULL;
 }
 
+
+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);*/
+
+    if (pthread_create(&server->eventThread,
+                       &attr,
+                       qemudRunLoop,
+                       server) != 0)
+        return -1;
+
+    server->hasEventThread = 1;
+
+    return 0;
+}
+
+
 static void qemudCleanup(struct qemud_server *server) {
     struct qemud_socket *sock;
 
@@ -3110,6 +3123,13 @@ int main(int argc, char **argv) {
         statuswrite = -1;
     }
 
+    /* Start the event loop in a background thread, since
+     * state initialization needs events to be being processed */
+    if (qemudStartEventLoop(server) < 0) {
+        VIR_ERROR0("Event thread startup failed");
+        goto error;
+    }
+
     /* Start the stateful HV drivers
      * This is delibrately done after telling the parent process
      * we're ready, since it can take a long time and this will
@@ -3119,9 +3139,32 @@ int main(int argc, char **argv) {
         goto error;
     }
 
-    qemudRunLoop(server);
+    /* Start accepting new clients from network */
+    virMutexLock(&server->lock);
+    if (qemudNetworkEnable(server) < 0) {
+        VIR_ERROR0("Network event loop enablement failed");
+        goto shutdown;
+    }
+    virMutexUnlock(&server->lock);
+
     ret = 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);
+
 error:
     if (statuswrite != -1) {
         if (ret != 0) {
diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h
index c0784d8..e3624c6 100644
--- a/daemon/libvirtd.h
+++ b/daemon/libvirtd.h
@@ -269,7 +269,9 @@ struct qemud_server {
     int sigread;
     int sigwrite;
     char *logDir;
-    unsigned int shutdown : 1;
+    pthread_t eventThread;
+    unsigned int hasEventThread :1;
+    unsigned int quitEventThread :1;
 #ifdef HAVE_AVAHI
     struct libvirtd_mdns *mdns;
 #endif
diff --git a/src/driver.h b/src/driver.h
index 0c8f923..d4a8b96 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -731,6 +731,7 @@ typedef struct _virStateDriver virStateDriver;
 typedef virStateDriver *virStateDriverPtr;
 
 struct _virStateDriver {
+    const char *name;
     virDrvStateInitialize  initialize;
     virDrvStateCleanup     cleanup;
     virDrvStateReload      reload;
diff --git a/src/libvirt.c b/src/libvirt.c
index 9e87900..ea51490 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -827,8 +827,11 @@ int virStateInitialize(int privileged) {
 
     for (i = 0 ; i < virStateDriverTabCount ; i++) {
         if (virStateDriverTab[i]->initialize &&
-            virStateDriverTab[i]->initialize(privileged) < 0)
+            virStateDriverTab[i]->initialize(privileged) < 0) {
+            VIR_ERROR("Initialization of %s state driver failed",
+                      virStateDriverTab[i]->name);
             ret = -1;
+        }
     }
     return ret;
 }
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 116f3ae..c76d6f4 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1641,12 +1641,21 @@ static int lxcStartup(int privileged)
      * XXX remove this when valgrind is fixed
      */
     ld = getenv("LD_PRELOAD");
-    if (ld && strstr(ld, "vgpreload"))
-        return -1;
+    if (ld && strstr(ld, "vgpreload")) {
+        VIR_INFO0("Running under valgrind, disabling driver");
+        return 0;
+    }
 
-    /* Check that the user is root */
+    /* Check that the user is root, silently disable if not */
     if (!privileged) {
-        return -1;
+        VIR_INFO0("Not running privileged, disabling driver");
+        return 0;
+    }
+
+    /* Check that this is a container enabled kernel */
+    if (lxcContainerAvailable(0) < 0) {
+        VIR_INFO0("LXC support not available in this kernel, disabling driver");
+        return 0;
     }
 
     if (VIR_ALLOC(lxc_driver) < 0) {
@@ -1658,12 +1667,6 @@ static int lxcStartup(int privileged)
     }
     lxcDriverLock(lxc_driver);
 
-    /* Check that this is a container enabled kernel */
-    if (lxcContainerAvailable(0) < 0) {
-        VIR_INFO0("LXC support not available in this kernel, disabling driver");
-        goto cleanup;
-    }
-
     if (virDomainObjListInit(&lxc_driver->domains) < 0)
         goto cleanup;
 
@@ -2322,6 +2325,7 @@ static virDriver lxcDriver = {
 };
 
 static virStateDriver lxcStateDriver = {
+    .name = "LXC",
     .initialize = lxcStartup,
     .cleanup = lxcShutdown,
     .active = lxcActive,
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 2a6a7ab..d49cef6 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1504,6 +1504,7 @@ static virNetworkDriver networkDriver = {
 };
 
 static virStateDriver networkStateDriver = {
+    "Network",
     networkStartup,
     networkShutdown,
     networkReload,
diff --git a/src/node_device/node_device_devkit.c b/src/node_device/node_device_devkit.c
index a6c7941..d2ffa1d 100644
--- a/src/node_device/node_device_devkit.c
+++ b/src/node_device/node_device_devkit.c
@@ -431,6 +431,7 @@ static virDeviceMonitor devkitDeviceMonitor = {
 
 
 static virStateDriver devkitStateDriver = {
+    .name = "DeviceKit",
     .initialize = devkitDeviceMonitorStartup,
     .cleanup = devkitDeviceMonitorShutdown,
     .reload = devkitDeviceMonitorReload,
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c
index 18be5ed..fe8d116 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -873,6 +873,7 @@ static virDeviceMonitor halDeviceMonitor = {
 
 
 static virStateDriver halStateDriver = {
+    .name = "HAL",
     .initialize = halDeviceMonitorStartup,
     .cleanup = halDeviceMonitorShutdown,
     .reload = halDeviceMonitorReload,
diff --git a/src/opennebula/one_driver.c b/src/opennebula/one_driver.c
index beb48ce..f95d45e 100644
--- a/src/opennebula/one_driver.c
+++ b/src/opennebula/one_driver.c
@@ -761,6 +761,7 @@ static virDriver oneDriver = {
 };
 
 static virStateDriver oneStateDriver = {
+    .name = "OpenNebula",
     .initialize = oneStartup,
     .cleanup    = oneShutdown,
     .active     = oneActive,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1651071..1bb82eb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7150,6 +7150,7 @@ static virDriver qemuDriver = {
 
 
 static virStateDriver qemuStateDriver = {
+    .name = "QEMU",
     .initialize = qemudStartup,
     .cleanup = qemudShutdown,
     .reload = qemudReload,
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index bf001eb..2c0406b 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8560,6 +8560,7 @@ static virDeviceMonitor dev_monitor = {
 
 #ifdef WITH_LIBVIRTD
 static virStateDriver state_driver = {
+    .name = "Remote",
     .initialize = remoteStartup,
 };
 #endif
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index d61c24a..1d5b4f7 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -1074,6 +1074,7 @@ static virSecretDriver secretDriver = {
 };
 
 static virStateDriver stateDriver = {
+    .name = "Secret",
     .initialize = secretDriverStartup,
     .cleanup = secretDriverCleanup,
     .reload = secretDriverReload,
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 4f8949b..80e4543 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1744,6 +1744,7 @@ static virStorageDriver storageDriver = {
 
 
 static virStateDriver stateDriver = {
+    .name = "Storage",
     .initialize = storageDriverStartup,
     .cleanup = storageDriverShutdown,
     .reload = storageDriverReload,
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 4fb04d9..98c0e14 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -1855,6 +1855,7 @@ static virDriver umlDriver = {
 
 
 static virStateDriver umlStateDriver = {
+    .name = "UML",
     .initialize = umlStartup,
     .cleanup = umlShutdown,
     .reload = umlReload,
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 5273a11..859f109 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -182,6 +182,7 @@ xenInitialize (int privileged ATTRIBUTE_UNUSED)
 }
 
 static virStateDriver state_driver = {
+    .name = "Xen",
     .initialize = xenInitialize,
 };
 
-- 
1.6.2.5




More information about the libvir-list mailing list