Re: [libvirt] PATCH: 12/28: Thread safety for UML driver

Sunday, 30 November 2008

This patch makes the UML driver thread safe

 uml_conf.h   |    2 
 uml_driver.c |  209 +++++++++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 172 insertions(+), 39 deletions(-)

Daniel

diff --git a/src/uml_conf.h b/src/uml_conf.h
--- a/src/uml_conf.h
+++ b/src/uml_conf.h
@@ -39,6 +39,8 @@
 
 /* Main driver state */
 struct uml_driver {
+    PTHREAD_MUTEX_T(lock);
+
     unsigned int umlVersion;
     int nextvmid;
 
diff --git a/src/uml_driver.c b/src/uml_driver.c
--- a/src/uml_driver.c
+++ b/src/uml_driver.c
@@ -73,6 +73,15 @@ static int umlShutdown(void);
 #define DEBUG0(msg) VIR_DEBUG(__FILE__, "%s", msg)
 
 #define umlLog(level, msg...) fprintf(stderr, msg)
+
+static void umlDriverLock(struct uml_driver *driver)
+{
+    pthread_mutex_lock(&driver->lock);
+}
+static void umlDriverUnlock(struct uml_driver *driver)
+{
+    pthread_mutex_unlock(&driver->lock);
+}
 
 
 static int umlOpenMonitor(virConnectPtr conn,
@@ -288,13 +297,16 @@ umlStartup(void) {
     if (VIR_ALLOC(uml_driver) < 0)
         return -1;
 
+    pthread_mutex_init(&uml_driver->lock, NULL);
+    umlDriverLock(uml_driver);
+
     /* Don't have a dom0 so start from 1 */
     uml_driver->nextvmid = 1;
 
     if (!(pw = getpwuid(uid))) {
         umlLog(UML_ERR, _("Failed to find user record for uid '%d':
%s\n"),
                uid, strerror(errno));
-        goto out_nouid;
+        goto error;
     }
 
     if (!uid) {
@@ -338,49 +350,45 @@ umlStartup(void) {
 
     if ((uml_driver->inotifyFD = inotify_init()) < 0) {
         umlLog(UML_ERR, "%s", _("cannot initialize inotify"));
-        goto out_nouid;
+        goto error;
     }
 
     if (virFileMakePath(uml_driver->monitorDir) < 0) {
         umlLog(UML_ERR, _("Failed to create monitor directory %s: %s"),
                uml_driver->monitorDir, strerror(errno));
-        umlShutdown();
-        return -1;
+        goto error;
     }
 
     if ((uml_driver->inotifyWatch =
          inotify_add_watch(uml_driver->inotifyFD,
                            uml_driver->monitorDir,
-                           IN_CREATE | IN_MODIFY | IN_DELETE)) < 0) {
-        umlShutdown();
-        return -1;
-    }
+                           IN_CREATE | IN_MODIFY | IN_DELETE)) < 0)
+        goto error;
 
     if (virEventAddHandle(uml_driver->inotifyFD, POLLIN,
-                          umlInotifyEvent, uml_driver, NULL) < 0) {
-        umlShutdown();
-        return -1;
-    }
+                          umlInotifyEvent, uml_driver, NULL) < 0)
+        goto error;
 
     if (virDomainLoadAllConfigs(NULL,
                                 uml_driver->caps,
                                 &uml_driver->domains,
                                 uml_driver->configDir,
                                 uml_driver->autostartDir,
-                                NULL, NULL) < 0) {
-        umlShutdown();
-        return -1;
-    }
+                                NULL, NULL) < 0)
+        goto error;
+
     umlAutostartConfigs(uml_driver);
 
+    umlDriverUnlock(uml_driver);
     return 0;
 
- out_of_memory:
+out_of_memory:
     umlLog (UML_ERR,
               "%s", _("umlStartup: out of memory\n"));
- out_nouid:
+error:
     VIR_FREE(base);
-    VIR_FREE(uml_driver);
+    umlDriverUnlock(uml_driver);
+    umlShutdown();
     return -1;
 }
 
@@ -395,6 +403,7 @@ umlReload(void) {
     if (!uml_driver)
         return 0;
 
+    umlDriverLock(uml_driver);
     virDomainLoadAllConfigs(NULL,
                             uml_driver->caps,
                             &uml_driver->domains,
@@ -403,6 +412,7 @@ umlReload(void) {
                             NULL, NULL);
 
     umlAutostartConfigs(uml_driver);
+    umlDriverUnlock(uml_driver);
 
     return 0;
 }
@@ -418,16 +428,21 @@ static int
 static int
 umlActive(void) {
     unsigned int i;
+    int active = 0;
 
     if (!uml_driver)
         return 0;
 
-    for (i = 0 ; i < uml_driver->domains.count ; i++)
+    umlDriverLock(uml_driver);
+    for (i = 0 ; i < uml_driver->domains.count ; i++) {
+        virDomainObjLock(uml_driver->domains.objs[i]);
         if (virDomainIsActive(uml_driver->domains.objs[i]))
-            return 1;
+            active = 1;
+        virDomainObjUnlock(uml_driver->domains.objs[i]);
+    }
+    umlDriverUnlock(uml_driver);
 
-    /* Otherwise we're happy to deal with a shutdown */
-    return 0;
+    return active;
 }
 
 /**
@@ -442,6 +457,7 @@ umlShutdown(void) {
     if (!uml_driver)
         return -1;
 
+    umlDriverLock(uml_driver);
     virEventRemoveHandle(uml_driver->inotifyWatch);
     close(uml_driver->inotifyFD);
     virCapabilitiesFree(uml_driver->caps);
@@ -451,9 +467,6 @@ umlShutdown(void) {
         virDomainObjPtr dom = uml_driver->domains.objs[i];
         if (virDomainIsActive(dom))
             umlShutdownVMDaemon(NULL, uml_driver, dom);
-        if (!dom->persistent)
-            virDomainRemoveInactive(&uml_driver->domains,
-                                    dom);
     }
 
     virDomainObjListFree(&uml_driver->domains);
@@ -466,6 +479,7 @@ umlShutdown(void) {
     if (uml_driver->brctl)
         brShutdown(uml_driver->brctl);
 
+    umlDriverUnlock(uml_driver);
     VIR_FREE(uml_driver);
 
     return 0;
@@ -904,9 +918,11 @@ static char *umlGetCapabilities(virConne
     struct uml_driver *driver = (struct uml_driver *)conn->privateData;
     char *xml;
 
+    umlDriverLock(driver);
     if ((xml = virCapabilitiesFormatXML(driver->caps)) == NULL)
         umlReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY,
                  "%s", _("failed to allocate space for capabilities
support"));
+    umlDriverUnlock(driver);
 
     return xml;
 }
@@ -1018,7 +1034,10 @@ static virDomainPtr umlDomainLookupByID(
     virDomainObjPtr vm;
     virDomainPtr dom = NULL;
 
+    umlDriverLock(driver);
     vm = virDomainFindByID(&driver->domains, id);
+    umlDriverUnlock(driver);
+
     if (!vm) {
         umlReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
         goto cleanup;
@@ -1028,6 +1047,8 @@ static virDomainPtr umlDomainLookupByID(
     if (dom) dom->id = vm->def->id;
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return dom;
 }
 
@@ -1037,7 +1058,10 @@ static virDomainPtr umlDomainLookupByUUI
     virDomainObjPtr vm;
     virDomainPtr dom = NULL;
 
+    umlDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, uuid);
+    umlDriverUnlock(driver);
+
     if (!vm) {
         umlReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
         goto cleanup;
@@ -1047,6 +1071,8 @@ static virDomainPtr umlDomainLookupByUUI
     if (dom) dom->id = vm->def->id;
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return dom;
 }
 
@@ -1056,7 +1082,10 @@ static virDomainPtr umlDomainLookupByNam
     virDomainObjPtr vm;
     virDomainPtr dom = NULL;
 
+    umlDriverLock(driver);
     vm = virDomainFindByName(&driver->domains, name);
+    umlDriverUnlock(driver);
+
     if (!vm) {
         umlReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
         goto cleanup;
@@ -1066,24 +1095,33 @@ static virDomainPtr umlDomainLookupByNam
     if (dom) dom->id = vm->def->id;
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return dom;
 }
 
 static int umlGetVersion(virConnectPtr conn, unsigned long *version) {
+    struct uml_driver *driver = conn->privateData;
     struct utsname ut;
     int major, minor, micro;
+    int ret = -1;
 
     uname(&ut);
 
+    umlDriverLock(driver);
     if (sscanf(ut.release, "%u.%u.%u",
                &major, &minor, &micro) != 3) {
         umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
                        _("cannot parse version %s"), ut.release);
-        return -1;
+        goto cleanup;
     }
 
-    *version = uml_driver->umlVersion;
-    return 0;
+    *version = driver->umlVersion;
+    ret = 0;
+
+cleanup:
+    umlDriverUnlock(driver);
+    return ret;
 }
 
 static char *
@@ -1112,9 +1150,14 @@ static int umlListDomains(virConnectPtr 
     struct uml_driver *driver = conn->privateData;
     int got = 0, i;
 
-    for (i = 0 ; i < driver->domains.count && got < nids ; i++)
+    umlDriverLock(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]);
+    }
+    umlDriverUnlock(driver);
 
     return got;
 }
@@ -1122,9 +1165,14 @@ static int umlNumDomains(virConnectPtr c
     struct uml_driver *driver = conn->privateData;
     int n = 0, i;
 
-    for (i = 0 ; i < driver->domains.count ; i++)
+    umlDriverLock(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]);
+    }
+    umlDriverUnlock(driver);
 
     return n;
 }
@@ -1132,9 +1180,10 @@ static virDomainPtr umlDomainCreate(virC
                                       unsigned int flags ATTRIBUTE_UNUSED) {
     struct uml_driver *driver = conn->privateData;
     virDomainDefPtr def;
-    virDomainObjPtr vm;
+    virDomainObjPtr vm = NULL;
     virDomainPtr dom = NULL;
 
+    umlDriverLock(driver);
     if (!(def = virDomainDefParseString(conn, driver->caps, xml)))
         goto cleanup;
 
@@ -1165,6 +1214,7 @@ static virDomainPtr umlDomainCreate(virC
     if (umlStartVMDaemon(conn, driver, vm) < 0) {
         virDomainRemoveInactive(&driver->domains,
                                 vm);
+        vm = NULL;
         goto cleanup;
     }
 
@@ -1173,7 +1223,9 @@ static virDomainPtr umlDomainCreate(virC
 
 cleanup:
     virDomainDefFree(def);
-
+    if (vm)
+        virDomainObjUnlock(vm);
+    umlDriverUnlock(driver);
     return dom;
 }
 
@@ -1184,7 +1236,9 @@ static int umlDomainShutdown(virDomainPt
     char *info;
     int ret = -1;
 
+    umlDriverLock(driver);
     vm = virDomainFindByID(&driver->domains, dom->id);
+    umlDriverUnlock(driver);
     if (!vm) {
         umlReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
                          _("no domain with matching id %d"), dom->id);
@@ -1202,8 +1256,9 @@ static int umlDomainShutdown(virDomainPt
 
 cleanup:
     VIR_FREE(info);
+    if (vm)
+        virDomainObjUnlock(vm);
     return ret;
-
 }
 
 
@@ -1212,6 +1267,7 @@ static int umlDomainDestroy(virDomainPtr
     virDomainObjPtr vm;
     int ret = -1;
 
+    umlDriverLock(driver);
     vm = virDomainFindByID(&driver->domains, dom->id);
     if (!vm) {
         umlReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
@@ -1220,12 +1276,17 @@ static int umlDomainDestroy(virDomainPtr
     }
 
     umlShutdownVMDaemon(dom->conn, driver, vm);
-    if (!vm->persistent)
+    if (!vm->persistent) {
         virDomainRemoveInactive(&driver->domains,
                                 vm);
+        vm = NULL;
+    }
     ret = 0;
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
+    umlDriverUnlock(driver);
     return ret;
 }
 
@@ -1235,7 +1296,9 @@ static char *umlDomainGetOSType(virDomai
     virDomainObjPtr vm;
     char *type = NULL;
 
+    umlDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    umlDriverUnlock(driver);
     if (!vm) {
         umlReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
                          "%s", _("no domain with matching uuid"));
@@ -1247,6 +1310,8 @@ static char *umlDomainGetOSType(virDomai
                          "%s", _("failed to allocate space for
ostype"));
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return type;
 }
 
@@ -1256,7 +1321,10 @@ static unsigned long umlDomainGetMaxMemo
     virDomainObjPtr vm;
     unsigned long ret = 0;
 
+    umlDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    umlDriverUnlock(driver);
+
     if (!vm) {
         char uuidstr[VIR_UUID_STRING_BUFLEN];
 
@@ -1268,6 +1336,8 @@ static unsigned long umlDomainGetMaxMemo
     ret = vm->def->maxmem;
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return ret;
 }
 
@@ -1276,7 +1346,10 @@ static int umlDomainSetMaxMemory(virDoma
     virDomainObjPtr vm;
     int ret = -1;
 
+    umlDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    umlDriverUnlock(driver);
+
     if (!vm) {
         char uuidstr[VIR_UUID_STRING_BUFLEN];
 
@@ -1295,6 +1368,8 @@ static int umlDomainSetMaxMemory(virDoma
     ret = 0;
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return ret;
 }
 
@@ -1303,7 +1378,10 @@ static int umlDomainSetMemory(virDomainP
     virDomainObjPtr vm;
     int ret = -1;
 
+    umlDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    umlDriverUnlock(driver);
+
     if (!vm) {
         char uuidstr[VIR_UUID_STRING_BUFLEN];
 
@@ -1329,6 +1407,8 @@ static int umlDomainSetMemory(virDomainP
     ret = 0;
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return ret;
 }
 
@@ -1338,7 +1418,10 @@ static int umlDomainGetInfo(virDomainPtr
     virDomainObjPtr vm;
     int ret = -1;
 
+    umlDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    umlDriverUnlock(driver);
+
     if (!vm) {
         umlReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
                          "%s", _("no domain with matching uuid"));
@@ -1363,6 +1446,8 @@ static int umlDomainGetInfo(virDomainPtr
     ret = 0;
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return ret;
 }
 
@@ -1373,7 +1458,10 @@ static char *umlDomainDumpXML(virDomainP
     virDomainObjPtr vm;
     char *ret = NULL;
 
+    umlDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    umlDriverUnlock(driver);
+
     if (!vm) {
         umlReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
                          "%s", _("no domain with matching uuid"));
@@ -1386,6 +1474,8 @@ static char *umlDomainDumpXML(virDomainP
                              flags);
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return ret;
 }
 
@@ -1395,21 +1485,27 @@ static int umlListDefinedDomains(virConn
     struct uml_driver *driver = conn->privateData;
     int got = 0, i;
 
+    umlDriverLock(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))) {
                 umlReportError(conn, NULL, 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]);
     }
+    umlDriverUnlock(driver);
 
     return got;
 
  cleanup:
     for (i = 0 ; i < got ; i++)
         VIR_FREE(names[i]);
+    umlDriverUnlock(driver);
     return -1;
 }
 
@@ -1417,9 +1513,14 @@ static int umlNumDefinedDomains(virConne
     struct uml_driver *driver = conn->privateData;
     int n = 0, i;
 
-    for (i = 0 ; i < driver->domains.count ; i++)
+    umlDriverLock(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]);
+    }
+    umlDriverUnlock(driver);
 
     return n;
 }
@@ -1430,6 +1531,7 @@ static int umlDomainStart(virDomainPtr d
     virDomainObjPtr vm;
     int ret = -1;
 
+    umlDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
     if (!vm) {
         umlReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
@@ -1440,6 +1542,9 @@ static int umlDomainStart(virDomainPtr d
     ret = umlStartVMDaemon(dom->conn, driver, vm);
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
+    umlDriverUnlock(driver);
     return ret;
 }
 
@@ -1447,9 +1552,10 @@ static virDomainPtr umlDomainDefine(virC
 static virDomainPtr umlDomainDefine(virConnectPtr conn, const char *xml) {
     struct uml_driver *driver = conn->privateData;
     virDomainDefPtr def;
-    virDomainObjPtr vm;
+    virDomainObjPtr vm = NULL;
     virDomainPtr dom = NULL;
 
+    umlDriverLock(driver);
     if (!(def = virDomainDefParseString(conn, driver->caps, xml)))
         goto cleanup;
 
@@ -1465,6 +1571,7 @@ static virDomainPtr umlDomainDefine(virC
                             vm->newDef ? vm->newDef : vm->def) < 0) {
         virDomainRemoveInactive(&driver->domains,
                                 vm);
+        vm = NULL;
         goto cleanup;
     }
 
@@ -1473,6 +1580,9 @@ static virDomainPtr umlDomainDefine(virC
 
 cleanup:
     virDomainDefFree(def);
+    if (vm)
+        virDomainObjUnlock(vm);
+    umlDriverUnlock(driver);
     return dom;
 }
 
@@ -1481,7 +1591,9 @@ static int umlDomainUndefine(virDomainPt
     virDomainObjPtr vm;
     int ret = -1;
 
+    umlDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+
     if (!vm) {
         umlReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
                          "%s", _("no domain with matching uuid"));
@@ -1505,9 +1617,13 @@ static int umlDomainUndefine(virDomainPt
 
     virDomainRemoveInactive(&driver->domains,
                             vm);
+    vm = NULL;
     ret = 0;
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
+    umlDriverUnlock(driver);
     return ret;
 }
 
@@ -1519,7 +1635,10 @@ static int umlDomainGetAutostart(virDoma
     virDomainObjPtr vm;
     int ret = -1;
 
+    umlDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    umlDriverUnlock(driver);
+
     if (!vm) {
         umlReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
                          "%s", _("no domain with matching uuid"));
@@ -1530,15 +1649,21 @@ static int umlDomainGetAutostart(virDoma
     ret = 0;
 
 cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
     return ret;
 }
 
 static int umlDomainSetAutostart(virDomainPtr dom,
                                    int autostart) {
     struct uml_driver *driver = dom->conn->privateData;
-    virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    virDomainObjPtr vm;
     char *configFile = NULL, *autostartLink = NULL;
     int ret = -1;
+
+    umlDriverLock(driver);
+    vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    umlDriverUnlock(driver);
 
     if (!vm) {
         umlReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
@@ -1592,7 +1717,8 @@ cleanup:
 cleanup:
     VIR_FREE(configFile);
     VIR_FREE(autostartLink);
-
+    if (vm)
+        virDomainObjUnlock(vm);
     return ret;
 }
 
@@ -1608,7 +1734,10 @@ umlDomainBlockPeek (virDomainPtr dom,
     virDomainObjPtr vm;
     int fd = -1, ret = -1, i;
 
+    umlDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    umlDriverUnlock(driver);
+
     if (!vm) {
         umlReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
                           _("no domain with matching uuid"));
@@ -1659,6 +1788,8 @@ umlDomainBlockPeek (virDomainPtr dom,
 
 cleanup:
     if (fd >= 0) close (fd);
+    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: 12/28: Thread safety for UML driver