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 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list