Re: [libvirt] PATCH: 21/28: Fill in locking stubs

This patch fills in the previous stub methods for locking/unlocking internal objects. With the following methods return a locked object virDomainFindByID virDomainFindByName virDomainFindByUUID virDomainAssignDef All the other methods accepting a virDomainObjPtr instance, require that the object first be locked. For virDomainDefPtr objects, if they are standalone, no locking is required (hence why the Xen driver isn't touched in any of these patches). If they are associated with a virDomainObjPtr though, this parent object must be locked. The same applies for virNetworkObjPtr, virStoragePoolObjPtr and the virNodeDeviceObjPtr objects & their methods. domain_conf.c | 40 ++++++++++++++++++++++++++++++++++++---- domain_conf.h | 2 ++ network_conf.c | 46 ++++++++++++++++++++++++++++++++++++++-------- network_conf.h | 2 ++ node_device_conf.c | 28 +++++++++++++++++++++++++++- node_device_conf.h | 2 ++ storage_conf.c | 35 ++++++++++++++++++++++++++++++++--- storage_conf.h | 2 ++ 8 files changed, 141 insertions(+), 16 deletions(-) Daniel diff --git a/src/domain_conf.c b/src/domain_conf.c --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -151,10 +151,13 @@ virDomainObjPtr virDomainFindByID(const { unsigned int i; - for (i = 0 ; i < doms->count ; i++) + for (i = 0 ; i < doms->count ; i++) { + virDomainObjLock(doms->objs[i]); if (virDomainIsActive(doms->objs[i]) && doms->objs[i]->def->id == id) return doms->objs[i]; + virDomainObjUnlock(doms->objs[i]); + } return NULL; } @@ -165,9 +168,12 @@ virDomainObjPtr virDomainFindByUUID(cons { unsigned int i; - for (i = 0 ; i < doms->count ; i++) + for (i = 0 ; i < doms->count ; i++) { + virDomainObjLock(doms->objs[i]); if (!memcmp(doms->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) return doms->objs[i]; + virDomainObjUnlock(doms->objs[i]); + } return NULL; } @@ -177,9 +183,12 @@ virDomainObjPtr virDomainFindByName(cons { unsigned int i; - for (i = 0 ; i < doms->count ; i++) + for (i = 0 ; i < doms->count ; i++) { + virDomainObjLock(doms->objs[i]); if (STREQ(doms->objs[i]->def->name, name)) return doms->objs[i]; + virDomainObjUnlock(doms->objs[i]); + } return NULL; } @@ -455,6 +464,8 @@ virDomainObjPtr virDomainAssignDef(virCo return NULL; } + pthread_mutex_init(&domain->lock, NULL); + virDomainObjLock(domain); domain->state = VIR_DOMAIN_SHUTOFF; domain->def = def; @@ -475,8 +486,12 @@ void virDomainRemoveInactive(virDomainOb { unsigned int i; + virDomainObjUnlock(dom); + for (i = 0 ; i < doms->count ; i++) { + virDomainObjLock(doms->objs[i]); if (doms->objs[i] == dom) { + virDomainObjUnlock(doms->objs[i]); virDomainObjFree(doms->objs[i]); if (i < (doms->count - 1)) @@ -490,6 +505,7 @@ void virDomainRemoveInactive(virDomainOb break; } + virDomainObjUnlock(doms->objs[i]); } } @@ -3337,8 +3353,10 @@ int virDomainLoadAllConfigs(virConnectPt entry->d_name, notify, opaque); - if (dom) + if (dom) { + virDomainObjUnlock(dom); dom->persistent = 1; + } } closedir(dir); @@ -3459,6 +3477,19 @@ const char *virDomainDefDefaultEmulator( } +#ifdef HAVE_PTHREAD_H + +void virDomainObjLock(virDomainObjPtr obj) +{ + pthread_mutex_lock(&obj->lock); +} + +void virDomainObjUnlock(virDomainObjPtr obj) +{ + pthread_mutex_unlock(&obj->lock); +} + +#else void virDomainObjLock(virDomainObjPtr obj ATTRIBUTE_UNUSED) { } @@ -3466,5 +3497,6 @@ void virDomainObjUnlock(virDomainObjPtr void virDomainObjUnlock(virDomainObjPtr obj ATTRIBUTE_UNUSED) { } +#endif #endif /* ! PROXY */ diff --git a/src/domain_conf.h b/src/domain_conf.h --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -454,6 +454,8 @@ typedef struct _virDomainObj virDomainOb typedef struct _virDomainObj virDomainObj; typedef virDomainObj *virDomainObjPtr; struct _virDomainObj { + PTHREAD_MUTEX_T(lock); + int stdin_fd; int stdout_fd; int stdout_watch; diff --git a/src/network_conf.c b/src/network_conf.c --- a/src/network_conf.c +++ b/src/network_conf.c @@ -58,9 +58,12 @@ virNetworkObjPtr virNetworkFindByUUID(co { unsigned int i; - for (i = 0 ; i < nets->count ; i++) + for (i = 0 ; i < nets->count ; i++) { + virNetworkObjLock(nets->objs[i]); if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) return nets->objs[i]; + virNetworkObjUnlock(nets->objs[i]); + } return NULL; } @@ -70,9 +73,12 @@ virNetworkObjPtr virNetworkFindByName(co { unsigned int i; - for (i = 0 ; i < nets->count ; i++) + for (i = 0 ; i < nets->count ; i++) { + virNetworkObjLock(nets->objs[i]); if (STREQ(nets->objs[i]->def->name, name)) return nets->objs[i]; + virNetworkObjUnlock(nets->objs[i]); + } return NULL; } @@ -157,7 +163,8 @@ virNetworkObjPtr virNetworkAssignDef(vir virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL); return NULL; } - + pthread_mutex_init(&network->lock, NULL); + virNetworkObjLock(network); network->def = def; if (VIR_REALLOC_N(nets->objs, nets->count + 1) < 0) { @@ -178,8 +185,11 @@ void virNetworkRemoveInactive(virNetwork { unsigned int i; + virNetworkObjUnlock(net); for (i = 0 ; i < nets->count ; i++) { + virNetworkObjLock(nets->objs[i]); if (nets->objs[i] == net) { + virNetworkObjUnlock(nets->objs[i]); virNetworkObjFree(nets->objs[i]); if (i < (nets->count - 1)) @@ -193,6 +203,7 @@ void virNetworkRemoveInactive(virNetwork break; } + virNetworkObjUnlock(nets->objs[i]); } } @@ -770,6 +781,8 @@ int virNetworkLoadAllConfigs(virConnectP } while ((entry = readdir(dir))) { + virNetworkObjPtr net; + if (entry->d_name[0] == '.') continue; @@ -778,11 +791,13 @@ int virNetworkLoadAllConfigs(virConnectP /* NB: ignoring errors, so one malformed config doesn't kill the whole process */ - virNetworkLoadConfig(conn, - nets, - configDir, - autostartDir, - entry->d_name); + net = virNetworkLoadConfig(conn, + nets, + configDir, + autostartDir, + entry->d_name); + if (net) + virNetworkObjUnlock(net); } closedir(dir); @@ -812,6 +827,19 @@ int virNetworkDeleteConfig(virConnectPtr return 0; } +#ifdef HAVE_PTHREAD_H + +void virNetworkObjLock(virNetworkObjPtr obj) +{ + pthread_mutex_lock(&obj->lock); +} + +void virNetworkObjUnlock(virNetworkObjPtr obj) +{ + pthread_mutex_unlock(&obj->lock); +} + +#else void virNetworkObjLock(virNetworkObjPtr obj ATTRIBUTE_UNUSED) { } @@ -819,3 +847,5 @@ void virNetworkObjUnlock(virNetworkObjPt void virNetworkObjUnlock(virNetworkObjPtr obj ATTRIBUTE_UNUSED) { } + +#endif diff --git a/src/network_conf.h b/src/network_conf.h --- a/src/network_conf.h +++ b/src/network_conf.h @@ -82,6 +82,8 @@ typedef struct _virNetworkObj virNetwork typedef struct _virNetworkObj virNetworkObj; typedef virNetworkObj *virNetworkObjPtr; struct _virNetworkObj { + PTHREAD_MUTEX_T(lock); + pid_t dnsmasqPid; unsigned int active : 1; unsigned int autostart : 1; diff --git a/src/node_device_conf.c b/src/node_device_conf.c --- a/src/node_device_conf.c +++ b/src/node_device_conf.c @@ -59,9 +59,12 @@ virNodeDeviceObjPtr virNodeDeviceFindByN { unsigned int i; - for (i = 0; i < devs->count; i++) + for (i = 0; i < devs->count; i++) { + virNodeDeviceObjLock(devs->objs[i]); if (STREQ(devs->objs[i]->def->name, name)) return devs->objs[i]; + virNodeDeviceObjUnlock(devs->objs[i]); + } return NULL; } @@ -125,6 +128,8 @@ virNodeDeviceObjPtr virNodeDeviceAssignD return NULL; } + pthread_mutex_init(&device->lock, NULL); + virNodeDeviceObjLock(device); device->def = def; if (VIR_REALLOC_N(devs->objs, devs->count+1) < 0) { @@ -144,8 +149,12 @@ void virNodeDeviceObjRemove(virNodeDevic { unsigned int i; + virNodeDeviceObjUnlock(dev); + for (i = 0; i < devs->count; i++) { + virNodeDeviceObjLock(dev); if (devs->objs[i] == dev) { + virNodeDeviceObjUnlock(dev); virNodeDeviceObjFree(devs->objs[i]); if (i < (devs->count - 1)) @@ -159,6 +168,7 @@ void virNodeDeviceObjRemove(virNodeDevic break; } + virNodeDeviceObjUnlock(dev); } } @@ -398,6 +408,20 @@ void virNodeDevCapsDefFree(virNodeDevCap } +#ifdef HAVE_PTHREAD_H + +void virNodeDeviceObjLock(virNodeDeviceObjPtr obj) +{ + pthread_mutex_lock(&obj->lock); +} + +void virNodeDeviceObjUnlock(virNodeDeviceObjPtr obj) +{ + pthread_mutex_unlock(&obj->lock); +} + +#else + void virNodeDeviceObjLock(virNodeDeviceObjPtr obj ATTRIBUTE_UNUSED) { } @@ -405,3 +429,5 @@ void virNodeDeviceObjUnlock(virNodeDevic void virNodeDeviceObjUnlock(virNodeDeviceObjPtr obj ATTRIBUTE_UNUSED) { } + +#endif diff --git a/src/node_device_conf.h b/src/node_device_conf.h --- a/src/node_device_conf.h +++ b/src/node_device_conf.h @@ -142,6 +142,8 @@ typedef struct _virNodeDeviceObj virNode typedef struct _virNodeDeviceObj virNodeDeviceObj; typedef virNodeDeviceObj *virNodeDeviceObjPtr; struct _virNodeDeviceObj { + PTHREAD_MUTEX_T(lock); + virNodeDeviceDefPtr def; /* device definition */ void *privateData; /* driver-specific private data */ void (*privateFree)(void *data); /* destructor for private data */ diff --git a/src/storage_conf.c b/src/storage_conf.c --- a/src/storage_conf.c +++ b/src/storage_conf.c @@ -305,8 +305,12 @@ virStoragePoolObjRemove(virStoragePoolOb { unsigned int i; + virStoragePoolObjUnlock(pool); + for (i = 0 ; i < pools->count ; i++) { + virStoragePoolObjLock(pools->objs[i]); if (pools->objs[i] == pool) { + virStoragePoolObjUnlock(pools->objs[i]); virStoragePoolObjFree(pools->objs[i]); if (i < (pools->count - 1)) @@ -320,6 +324,7 @@ virStoragePoolObjRemove(virStoragePoolOb break; } + virStoragePoolObjUnlock(pools->objs[i]); } } @@ -1143,9 +1148,12 @@ virStoragePoolObjFindByUUID(virStoragePo const unsigned char *uuid) { unsigned int i; - for (i = 0 ; i < pools->count ; i++) + for (i = 0 ; i < pools->count ; i++) { + virStoragePoolObjLock(pools->objs[i]); if (!memcmp(pools->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) return pools->objs[i]; + virStoragePoolObjUnlock(pools->objs[i]); + } return NULL; } @@ -1155,9 +1163,12 @@ virStoragePoolObjFindByName(virStoragePo const char *name) { unsigned int i; - for (i = 0 ; i < pools->count ; i++) + for (i = 0 ; i < pools->count ; i++) { + virStoragePoolObjLock(pools->objs[i]); if (STREQ(pools->objs[i]->def->name, name)) return pools->objs[i]; + virStoragePoolObjUnlock(pools->objs[i]); + } return NULL; } @@ -1233,6 +1244,8 @@ virStoragePoolObjAssignDef(virConnectPtr return NULL; } + pthread_mutex_init(&pool->lock, NULL); + virStoragePoolObjLock(pool); pool->active = 0; pool->def = def; @@ -1317,6 +1330,7 @@ virStoragePoolLoadAllConfigs(virConnectP char *xml = NULL; char path[PATH_MAX]; char autostartLink[PATH_MAX]; + virStoragePoolObjPtr pool; if (entry->d_name[0] == '.') continue; @@ -1341,7 +1355,9 @@ virStoragePoolLoadAllConfigs(virConnectP if (virFileReadAll(path, 8192, &xml) < 0) continue; - virStoragePoolObjLoad(conn, pools, entry->d_name, path, xml, autostartLink); + pool = virStoragePoolObjLoad(conn, pools, entry->d_name, path, xml, autostartLink); + if (pool) + virStoragePoolObjUnlock(pool); VIR_FREE(xml); } @@ -1499,6 +1515,18 @@ char *virStoragePoolSourceListFormat(vir } +#ifdef HAVE_PTHREAD_H + +void virStoragePoolObjLock(virStoragePoolObjPtr obj) +{ + pthread_mutex_lock(&obj->lock); +} + +void virStoragePoolObjUnlock(virStoragePoolObjPtr obj) +{ + pthread_mutex_unlock(&obj->lock); +} +#else void virStoragePoolObjLock(virStoragePoolObjPtr obj ATTRIBUTE_UNUSED) { } @@ -1506,3 +1534,4 @@ void virStoragePoolObjUnlock(virStorageP void virStoragePoolObjUnlock(virStoragePoolObjPtr obj ATTRIBUTE_UNUSED) { } +#endif diff --git a/src/storage_conf.h b/src/storage_conf.h --- a/src/storage_conf.h +++ b/src/storage_conf.h @@ -223,6 +223,8 @@ typedef virStoragePoolObj *virStoragePoo typedef virStoragePoolObj *virStoragePoolObjPtr; struct _virStoragePoolObj { + PTHREAD_MUTEX_T(lock); + char *configFile; char *autostartLink; int active; -- |: 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 :|