[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]