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(¨_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, ¨_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, ¨_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(¨_driver->domains, - dom); } virDomainObjListFree(¨_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, µ) != 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 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list