Re: [libvirt] PATCH: 5/5: Locking for the virNetworkObjPtr instances

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

This implements locking in the virNetworkObjPtr objects. This follows 
exactly the same model as the  virDomainObjPtr locking.

There still needs to be code in the network driver itself to make use
of this locking, so this patch is a work in progress

Daniel

diff --git a/src/network_conf.c b/src/network_conf.c
--- a/src/network_conf.c
+++ b/src/network_conf.c
@@ -52,26 +52,46 @@ VIR_ENUM_IMPL(virNetworkForward,
         __virReportErrorHelper(conn, VIR_FROM_NETWORK, code, __FILE__,       \
                                __FUNCTION__, __LINE__, fmt)
 
+/**
+ * @virNetworkFindByUUID:
+ *
+ * Caller must hold exclusive lock over 'nets'
+ *
+ * Returns a virNetworkObjPtr locked for exclusive access, or NULL
+ */
 virNetworkObjPtr virNetworkFindByUUID(const virNetworkObjListPtr nets,
                                       const unsigned char *uuid)
 {
     unsigned int i;
 
-    for (i = 0 ; i < nets->count ; i++)
+    for (i = 0 ; i < nets->count ; i++) {
+        virNetworkLock(nets->objs[i]);
         if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN))
             return nets->objs[i];
+        virNetworkUnlock(nets->objs[i]);
+    }
 
     return NULL;
 }
 
+/**
+ * @virNetworkFindByName:
+ *
+ * Caller must hold exclusive lock over 'nets'
+ *
+ * Returns a virNetworkObjPtr locked for exclusive access, or NULL
+ */
 virNetworkObjPtr virNetworkFindByName(const virNetworkObjListPtr nets,
                                       const char *name)
 {
     unsigned int i;
 
-    for (i = 0 ; i < nets->count ; i++)
+    for (i = 0 ; i < nets->count ; i++) {
+        virNetworkLock(nets->objs[i]);
         if (STREQ(nets->objs[i]->def->name, name))
             return nets->objs[i];
+        virNetworkUnlock(nets->objs[i]);
+    }
 
     return NULL;
 }
@@ -108,6 +128,13 @@ void virNetworkDefFree(virNetworkDefPtr 
     VIR_FREE(def);
 }
 
+
+/**
+ * @virDomainObjFree
+ *
+ * Caller should not hold lock on 'net', but must
+ * also ensure no other thread can have an active lock
+ */
 void virNetworkObjFree(virNetworkObjPtr net)
 {
     if (!net)
@@ -122,16 +149,35 @@ void virNetworkObjFree(virNetworkObjPtr 
     VIR_FREE(net);
 }
 
+/**
+ * @virNetworkObjListFree
+ *
+ * Caller must hold exclusive lock over 'nets'
+ */
 void virNetworkObjListFree(virNetworkObjListPtr nets)
 {
     unsigned int i;
 
-    for (i = 0 ; i < nets->count ; i++)
+    for (i = 0 ; i < nets->count ; i++) {
+        /* Lock+unlock ensures no one is still using this dom */
+        virNetworkLock(nets->objs[i]);
+        virNetworkUnlock(nets->objs[i]);
+
         virNetworkObjFree(nets->objs[i]);
+    }
 
     VIR_FREE(nets->objs);
     nets->count = 0;
 }
+
+
+/**
+ * @virNetworkAssignDef
+ *
+ * Caller must hold exclusive lock over 'nets'.
+ *
+ * The return network object will be locked
+ */
 
 virNetworkObjPtr virNetworkAssignDef(virConnectPtr conn,
                                      virNetworkObjListPtr nets,
@@ -158,6 +204,7 @@ virNetworkObjPtr virNetworkAssignDef(vir
     }
 
     network->def = def;
+    pthread_mutex_init(&network->lock, NULL);
 
     if (VIR_REALLOC_N(nets->objs, nets->count + 1) < 0) {
         virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL);
@@ -168,14 +215,27 @@ virNetworkObjPtr virNetworkAssignDef(vir
     nets->objs[nets->count] = network;
     nets->count++;
 
+    virNetworkLock(network);
+
     return network;
 
 }
 
+
+/**
+ * @virNetworkRemoveInactive:
+ *
+ * Caller must hold exclusive lock over 'nets', but
+ * 'net' must be unlocked
+ */
 void virNetworkRemoveInactive(virNetworkObjListPtr nets,
                               const virNetworkObjPtr net)
 {
     unsigned int i;
+
+    /* Ensure no other thread holds a lock on dom */
+    virNetworkLock(net);
+    virNetworkUnlock(net);
 
     for (i = 0 ; i < nets->count ; i++) {
         if (nets->objs[i] == net) {
@@ -769,6 +829,8 @@ int virNetworkLoadAllConfigs(virConnectP
     }
 
     while ((entry = readdir(dir))) {
+        virNetworkObjPtr net;
+
         if (entry->d_name[0] == '.')
             continue;
 
@@ -777,11 +839,15 @@ 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) {
+            net->persistent = 1;
+            virNetworkUnlock(net);
+        }
     }
 
     closedir(dir);
diff --git a/src/network_conf.h b/src/network_conf.h
--- a/src/network_conf.h
+++ b/src/network_conf.h
@@ -78,6 +78,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;
@@ -103,6 +105,19 @@ virNetworkIsActive(const virNetworkObjPt
     return net->active;
 }
 
+static inline int
+virNetworkLock(virNetworkObjPtr net)
+{
+    return pthread_mutex_lock(&net->lock);
+}
+
+
+static inline int
+virNetworkUnlock(virNetworkObjPtr net)
+{
+    return pthread_mutex_unlock(&net->lock);
+}
+
 
 virNetworkObjPtr virNetworkFindByUUID(const virNetworkObjListPtr nets,
                                       const unsigned char *uuid);


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