Re: [libvirt] PATCH: 10/28: Thread safety for LXC driver

Sunday, 30 November 2008

This patch makes the LXC driver thread safe following the same pattern
as the Test/QEMU drivers

 lxc_conf.h   |    2 
 lxc_driver.c |  187 +++++++++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 151 insertions(+), 38 deletions(-)

Daniel

diff --git a/src/lxc_conf.h b/src/lxc_conf.h
--- a/src/lxc_conf.h
+++ b/src/lxc_conf.h
@@ -36,6 +36,8 @@
 
 typedef struct __lxc_driver lxc_driver_t;
 struct __lxc_driver {
+    PTHREAD_MUTEX_T(lock);
+
     virCapsPtr caps;
 
     virDomainObjList domains;
diff --git a/src/lxc_driver.c b/src/lxc_driver.c
--- a/src/lxc_driver.c
+++ b/src/lxc_driver.c
@@ -54,6 +54,16 @@ static lxc_driver_t *lxc_driver = NULL;
 static lxc_driver_t *lxc_driver = NULL;
 
 /* Functions */
+
+static void lxcDriverLock(lxc_driver_t *driver)
+{
+    pthread_mutex_lock(&driver->lock);
+}
+static void lxcDriverUnlock(lxc_driver_t *driver)
+{
+    pthread_mutex_unlock(&driver->lock);
+}
+
 
 static int lxcProbe(void)
 {
@@ -107,7 +117,10 @@ static virDomainPtr lxcDomainLookupByID(
     virDomainObjPtr vm;
     virDomainPtr dom = NULL;
 
+    lxcDriverLock(driver);
     vm = virDomainFindByID(&driver->domains, id);
+    lxcDriverUnlock(driver);
+
     if (!vm) {
         lxcError(conn, NULL, VIR_ERR_NO_DOMAIN, NULL);
         goto cleanup;
@@ -118,6 +131,8 @@ static virDomainPtr lxcDomainLookupByID(
         dom->id = vm->def->id;
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return dom;
 }
 
@@ -128,7 +143,10 @@ static virDomainPtr lxcDomainLookupByUUI
     virDomainObjPtr vm;
     virDomainPtr dom = NULL;
 
+    lxcDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, uuid);
+    lxcDriverUnlock(driver);
+
     if (!vm) {
         lxcError(conn, NULL, VIR_ERR_NO_DOMAIN, NULL);
         goto cleanup;
@@ -139,6 +157,8 @@ static virDomainPtr lxcDomainLookupByUUI
         dom->id = vm->def->id;
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return dom;
 }
 
@@ -149,7 +169,9 @@ static virDomainPtr lxcDomainLookupByNam
     virDomainObjPtr vm;
     virDomainPtr dom = NULL;
 
+    lxcDriverLock(driver);
     vm = virDomainFindByName(&driver->domains, name);
+    lxcDriverUnlock(driver);
     if (!vm) {
         lxcError(conn, NULL, VIR_ERR_NO_DOMAIN, NULL);
         goto cleanup;
@@ -160,6 +182,8 @@ static virDomainPtr lxcDomainLookupByNam
         dom->id = vm->def->id;
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return dom;
 }
 
@@ -167,9 +191,14 @@ static int lxcListDomains(virConnectPtr 
     lxc_driver_t *driver = conn->privateData;
     int got = 0, i;
 
-    for (i = 0 ; i < driver->domains.count && got < nids ; i++)
+    lxcDriverLock(driver);
+    for (i = 0 ; i < driver->domains.count && got < nids ; i++) {
+        virDomainObjLock(driver->domains.objs[i]);
         if (virDomainIsActive(driver->domains.objs[i]))
             ids[got++] = driver->domains.objs[i]->def->id;
+        virDomainObjUnlock(driver->domains.objs[i]);
+    }
+    lxcDriverUnlock(driver);
 
     return got;
 }
@@ -178,9 +207,14 @@ static int lxcNumDomains(virConnectPtr c
     lxc_driver_t *driver = conn->privateData;
     int n = 0, i;
 
-    for (i = 0 ; i < driver->domains.count ; i++)
+    lxcDriverLock(driver);
+    for (i = 0 ; i < driver->domains.count ; i++) {
+        virDomainObjLock(driver->domains.objs[i]);
         if (virDomainIsActive(driver->domains.objs[i]))
             n++;
+        virDomainObjUnlock(driver->domains.objs[i]);
+    }
+    lxcDriverUnlock(driver);
 
     return n;
 }
@@ -190,21 +224,27 @@ static int lxcListDefinedDomains(virConn
     lxc_driver_t *driver = conn->privateData;
     int got = 0, i;
 
+    lxcDriverLock(driver);
     for (i = 0 ; i < driver->domains.count && got < nnames ; i++) {
+        virDomainObjLock(driver->domains.objs[i]);
         if (!virDomainIsActive(driver->domains.objs[i])) {
             if (!(names[got++] = strdup(driver->domains.objs[i]->def->name))) {
                 lxcError(conn, NULL, VIR_ERR_NO_MEMORY,
                          "%s", _("failed to allocate space for VM name
string"));
+                virDomainObjUnlock(driver->domains.objs[i]);
                 goto cleanup;
             }
         }
+        virDomainObjUnlock(driver->domains.objs[i]);
     }
+    lxcDriverUnlock(driver);
 
     return got;
 
  cleanup:
     for (i = 0 ; i < got ; i++)
         VIR_FREE(names[i]);
+    lxcDriverUnlock(driver);
     return -1;
 }
 
@@ -213,9 +253,14 @@ static int lxcNumDefinedDomains(virConne
     lxc_driver_t *driver = conn->privateData;
     int n = 0, i;
 
-    for (i = 0 ; i < driver->domains.count ; i++)
+    lxcDriverLock(driver);
+    for (i = 0 ; i < driver->domains.count ; i++) {
+        virDomainObjLock(driver->domains.objs[i]);
         if (!virDomainIsActive(driver->domains.objs[i]))
             n++;
+        virDomainObjUnlock(driver->domains.objs[i]);
+    }
+    lxcDriverUnlock(driver);
 
     return n;
 }
@@ -226,9 +271,10 @@ static virDomainPtr lxcDomainDefine(virC
 {
     lxc_driver_t *driver = conn->privateData;
     virDomainDefPtr def = NULL;
-    virDomainObjPtr vm;
+    virDomainObjPtr vm = NULL;
     virDomainPtr dom = NULL;
 
+    lxcDriverLock(driver);
     if (!(def = virDomainDefParseString(conn, driver->caps, xml)))
         goto cleanup;
 
@@ -247,6 +293,7 @@ static virDomainPtr lxcDomainDefine(virC
                             driver->configDir,
                             vm->newDef ? vm->newDef : vm->def) < 0) {
         virDomainRemoveInactive(&driver->domains, vm);
+        vm = NULL;
         goto cleanup;
     }
 
@@ -256,6 +303,9 @@ static virDomainPtr lxcDomainDefine(virC
 
 cleanup:
     virDomainDefFree(def);
+    if (vm)
+        virDomainObjUnlock(vm);
+    lxcDriverUnlock(driver);
     return dom;
 }
 
@@ -265,6 +315,7 @@ static int lxcDomainUndefine(virDomainPt
     virDomainObjPtr vm;
     int ret = -1;
 
+    lxcDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
     if (!vm) {
         lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN,
@@ -291,9 +342,13 @@ static int lxcDomainUndefine(virDomainPt
         goto cleanup;
 
     virDomainRemoveInactive(&driver->domains, vm);
+    vm = NULL;
     ret = 0;
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
+    lxcDriverUnlock(driver);
     return ret;
 }
 
@@ -304,7 +359,10 @@ static int lxcDomainGetInfo(virDomainPtr
     virDomainObjPtr vm;
     int ret = -1;
 
+    lxcDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    lxcDriverUnlock(driver);
+
     if (!vm) {
         lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN,
                  "%s", _("no domain with matching uuid"));
@@ -325,6 +383,8 @@ static int lxcDomainGetInfo(virDomainPtr
     ret = 0;
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return ret;
 }
 
@@ -334,7 +394,10 @@ static char *lxcGetOSType(virDomainPtr d
     virDomainObjPtr vm;
     char *ret = NULL;
 
+    lxcDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    lxcDriverUnlock(driver);
+
     if (!vm) {
         lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN,
                  "%s", _("no domain with matching uuid"));
@@ -344,6 +407,8 @@ static char *lxcGetOSType(virDomainPtr d
     ret = strdup(vm->def->os.type);
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return ret;
 }
 
@@ -354,7 +419,10 @@ static char *lxcDomainDumpXML(virDomainP
     virDomainObjPtr vm;
     char *ret = NULL;
 
+    lxcDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    lxcDriverUnlock(driver);
+
     if (!vm) {
         lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN,
                  "%s", _("no domain with matching uuid"));
@@ -367,6 +435,8 @@ static char *lxcDomainDumpXML(virDomainP
                              flags);
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return ret;
 }
 
@@ -880,6 +950,7 @@ static int lxcDomainStart(virDomainPtr d
     virDomainObjPtr vm;
     int ret = -1;
 
+    lxcDriverLock(driver);
     vm = virDomainFindByName(&driver->domains, dom->name);
     if (!vm) {
         lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN,
@@ -896,6 +967,9 @@ static int lxcDomainStart(virDomainPtr d
     ret = lxcVmStart(dom->conn, driver, vm);
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
+    lxcDriverUnlock(driver);
     return ret;
 }
 
@@ -914,10 +988,11 @@ lxcDomainCreateAndStart(virConnectPtr co
                         const char *xml,
                         unsigned int flags ATTRIBUTE_UNUSED) {
     lxc_driver_t *driver = conn->privateData;
-    virDomainObjPtr vm;
+    virDomainObjPtr vm = NULL;
     virDomainDefPtr def;
     virDomainPtr dom = NULL;
 
+    lxcDriverLock(driver);
     if (!(def = virDomainDefParseString(conn, driver->caps, xml)))
         goto cleanup;
 
@@ -934,6 +1009,7 @@ lxcDomainCreateAndStart(virConnectPtr co
 
     if (lxcVmStart(conn, driver, vm) < 0) {
         virDomainRemoveInactive(&driver->domains, vm);
+        vm = NULL;
         goto cleanup;
     }
 
@@ -943,6 +1019,9 @@ lxcDomainCreateAndStart(virConnectPtr co
 
 cleanup:
     virDomainDefFree(def);
+    if (vm)
+        virDomainObjUnlock(vm);
+    lxcDriverUnlock(driver);
     return dom;
 }
 
@@ -960,6 +1039,7 @@ static int lxcDomainShutdown(virDomainPt
     virDomainObjPtr vm;
     int ret = -1;
 
+    lxcDriverLock(driver);
     vm = virDomainFindByID(&driver->domains, dom->id);
     if (!vm) {
         lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN,
@@ -968,8 +1048,15 @@ static int lxcDomainShutdown(virDomainPt
     }
 
     ret = lxcVmTerminate(dom->conn, driver, vm, 0);
+    if (!vm->persistent) {
+        virDomainRemoveInactive(&driver->domains, vm);
+        vm = NULL;
+    }
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
+    lxcDriverUnlock(driver);
     return ret;
 }
 
@@ -988,6 +1075,7 @@ static int lxcDomainDestroy(virDomainPtr
     virDomainObjPtr vm;
     int ret = -1;
 
+    lxcDriverLock(driver);
     vm = virDomainFindByID(&driver->domains, dom->id);
     if (!vm) {
         lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN,
@@ -996,8 +1084,15 @@ static int lxcDomainDestroy(virDomainPtr
     }
 
     ret = lxcVmTerminate(dom->conn, driver, vm, SIGKILL);
+    if (!vm->persistent) {
+        virDomainRemoveInactive(&driver->domains, vm);
+        vm = NULL;
+    }
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
+    lxcDriverUnlock(driver);
     return ret;
 }
 
@@ -1029,33 +1124,29 @@ static int lxcStartup(void)
     if (VIR_ALLOC(lxc_driver) < 0) {
         return -1;
     }
+    pthread_mutex_init(&lxc_driver->lock, NULL);
+    lxcDriverLock(lxc_driver);
 
     /* Check that this is a container enabled kernel */
     if(lxcContainerAvailable(0) < 0)
-        return -1;
+        goto cleanup;
 
     lxc_driver->have_netns = lxcCheckNetNsSupport();
 
     /* Call function to load lxc driver configuration information */
-    if (lxcLoadDriverConfig(lxc_driver) < 0) {
-        lxcShutdown();
-        return -1;
-    }
+    if (lxcLoadDriverConfig(lxc_driver) < 0)
+        goto cleanup;
 
-    if ((lxc_driver->caps = lxcCapsInit()) == NULL) {
-        lxcShutdown();
-        return -1;
-    }
+    if ((lxc_driver->caps = lxcCapsInit()) == NULL)
+        goto cleanup;
 
     if (virDomainLoadAllConfigs(NULL,
                                 lxc_driver->caps,
                                 &lxc_driver->domains,
                                 lxc_driver->configDir,
                                 lxc_driver->autostartDir,
-                                NULL, NULL) < 0) {
-        lxcShutdown();
-        return -1;
-    }
+                                NULL, NULL) < 0)
+        goto cleanup;
 
     for (i = 0 ; i < lxc_driver->domains.count ; i++) {
         virDomainObjPtr vm = lxc_driver->domains.objs[i];
@@ -1095,17 +1186,13 @@ static int lxcStartup(void)
         }
     }
 
+    lxcDriverUnlock(lxc_driver);
     return 0;
-}
 
-static void lxcFreeDriver(lxc_driver_t *driver)
-{
-    virCapabilitiesFree(driver->caps);
-    VIR_FREE(driver->configDir);
-    VIR_FREE(driver->autostartDir);
-    VIR_FREE(driver->stateDir);
-    VIR_FREE(driver->logDir);
-    VIR_FREE(driver);
+cleanup:
+    lxcDriverUnlock(lxc_driver);
+    lxcShutdown();
+    return -1;
 }
 
 static int lxcShutdown(void)
@@ -1113,8 +1200,16 @@ static int lxcShutdown(void)
     if (lxc_driver == NULL)
         return(-1);
 
+    lxcDriverLock(lxc_driver);
     virDomainObjListFree(&lxc_driver->domains);
-    lxcFreeDriver(lxc_driver);
+
+    virCapabilitiesFree(lxc_driver->caps);
+    VIR_FREE(lxc_driver->configDir);
+    VIR_FREE(lxc_driver->autostartDir);
+    VIR_FREE(lxc_driver->stateDir);
+    VIR_FREE(lxc_driver->logDir);
+    lxcDriverUnlock(lxc_driver);
+    VIR_FREE(lxc_driver);
     lxc_driver = NULL;
 
     return 0;
@@ -1130,16 +1225,21 @@ static int
 static int
 lxcActive(void) {
     unsigned int i;
+    int active = 0;
 
     if (lxc_driver == NULL)
         return(0);
 
-    for (i = 0 ; i < lxc_driver->domains.count ; i++)
+    lxcDriverLock(lxc_driver);
+    for (i = 0 ; i < lxc_driver->domains.count ; i++) {
+        virDomainObjLock(lxc_driver->domains.objs[i]);
         if (virDomainIsActive(lxc_driver->domains.objs[i]))
-            return 1;
+            active = 1;
+        virDomainObjUnlock(lxc_driver->domains.objs[i]);
+    }
+    lxcDriverUnlock(lxc_driver);
 
-    /* Otherwise we're happy to deal with a shutdown */
-    return 0;
+    return active;
 }
 
 static int lxcVersion(virConnectPtr conn, unsigned long *version)
@@ -1179,15 +1279,19 @@ static int lxcSetSchedulerParameters(vir
                                      virSchedParameterPtr params,
                                      int nparams)
 {
+    lxc_driver_t *driver = domain->conn->privateData;
     int i;
     virCgroupPtr group = NULL;
     virDomainObjPtr vm = NULL;
     int ret = -1;
 
     if (virCgroupHaveSupport() != 0)
-        goto cleanup;
+        return -1;
 
-    vm = virDomainFindByUUID(&lxc_driver->domains, domain->uuid);
+    lxcDriverLock(driver);
+    vm = virDomainFindByUUID(&driver->domains, domain->uuid);
+    lxcDriverUnlock(driver);
+
     if (vm == NULL) {
         lxcError(NULL, domain, VIR_ERR_INTERNAL_ERROR,
                  _("No such domain %s"), domain->uuid);
@@ -1213,7 +1317,8 @@ static int lxcSetSchedulerParameters(vir
 
 cleanup:
     virCgroupFree(&group);
-
+    if (vm)
+        virDomainObjUnlock(vm);
     return ret;
 }
 
@@ -1221,21 +1326,25 @@ static int lxcGetSchedulerParameters(vir
                                      virSchedParameterPtr params,
                                      int *nparams)
 {
+    lxc_driver_t *driver = domain->conn->privateData;
     virCgroupPtr group = NULL;
     virDomainObjPtr vm = NULL;
     unsigned long val;
     int ret = -1;
 
     if (virCgroupHaveSupport() != 0)
-        goto cleanup;
+        return -1;
 
     if ((*nparams) != 1) {
         lxcError(NULL, domain, VIR_ERR_INVALID_ARG,
                  "%s", _("Invalid parameter count"));
-        goto cleanup;
+        return -1;
     }
 
-    vm = virDomainFindByUUID(&lxc_driver->domains, domain->uuid);
+    lxcDriverLock(driver);
+    vm = virDomainFindByUUID(&driver->domains, domain->uuid);
+    lxcDriverUnlock(driver);
+
     if (vm == NULL) {
         lxcError(NULL, domain, VIR_ERR_INTERNAL_ERROR,
                  _("No such domain %s"), domain->uuid);
@@ -1255,6 +1364,8 @@ static int lxcGetSchedulerParameters(vir
 
 cleanup:
     virCgroupFree(&group);
+    if (vm)
+        virDomainObjUnlock(vm);
     return ret;
 }
 

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


    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

Re: [libvirt] PATCH: 10/28: Thread safety for LXC driver