From: Daniel P. Berrange <berrange@redhat.com>
Date: Mon, 2 Nov 2009 23:18:19 +0000 (-0500)
Subject: Fix return value in virStateInitialize impl for LXC
X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=979218cdd9887b132eb0f29fe2048f89e90beae1;p=libvirt.git

Fix return value in virStateInitialize impl for LXC

The LXC driver was mistakenly returning -1 for lxcStartup()
in scenarios that are not an error. This caused the libvirtd
to quit for unprivileged users. This fixes the return code
of LXC driver, and also adds a "name" field to the virStateDriver
struct and logging to make it easier to find these problems
in the future

* 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
---

diff --git a/src/driver.h b/src/driver.h
index 0c8f9232b8..d4a8b960a1 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 5ddf27a189..b0b67c2409 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 47e59f65d4..9a5c43a1c7 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 32331016cd..76caa7e5e0 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1509,6 +1509,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 a6c79419f0..d2ffa1d931 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 18be5ed936..fe8d116691 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 cffd626cde..19a335c305 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 437a1b4c79..4eb4b68a74 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7154,6 +7154,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 a1989999c9..ee7a046909 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 d61c24a96b..1d5b4f7991 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 4f8949b34c..80e4543f99 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 fac3c4ca37..5c6dbff9a7 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 f2744b095d..41f340ced7 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,
 };