Re: [libvirt] PATCH: 14/28: Thread safety for network driver

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

 



This patch makes the network driver thread safe, following the pattern of
the earlier patches

 network_driver.c |  136 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 125 insertions(+), 11 deletions(-)

Daniel

diff --git a/src/network_driver.c b/src/network_driver.c
--- a/src/network_driver.c
+++ b/src/network_driver.c
@@ -59,6 +59,8 @@
 
 /* Main driver state */
 struct network_driver {
+    PTHREAD_MUTEX_T(lock);
+
     virNetworkObjList networks;
 
     iptablesContext *iptables;
@@ -67,6 +69,16 @@ struct network_driver {
     char *networkAutostartDir;
     char *logDir;
 };
+
+
+static void networkDriverLock(struct network_driver *driver)
+{
+    pthread_mutex_lock(&driver->lock);
+}
+static void networkDriverUnlock(struct network_driver *driver)
+{
+    pthread_mutex_unlock(&driver->lock);
+}
 
 static int networkShutdown(void);
 
@@ -95,6 +107,7 @@ networkAutostartConfigs(struct network_d
     unsigned int i;
 
     for (i = 0 ; i < driver->networks.count ; i++) {
+        virNetworkObjLock(driver->networks.objs[i]);
         if (driver->networks.objs[i]->autostart &&
             !virNetworkIsActive(driver->networks.objs[i]) &&
             networkStartNetworkDaemon(NULL, driver, driver->networks.objs[i]) < 0) {
@@ -103,6 +116,7 @@ networkAutostartConfigs(struct network_d
                        driver->networks.objs[i]->def->name,
                        err ? err->message : NULL);
         }
+        virNetworkObjUnlock(driver->networks.objs[i]);
     }
 }
 
@@ -119,6 +133,9 @@ networkStartup(void) {
 
     if (VIR_ALLOC(driverState) < 0)
         goto error;
+
+    pthread_mutex_init(&driverState->lock, NULL);
+    networkDriverLock(driverState);
 
     if (!uid) {
         if (asprintf(&driverState->logDir,
@@ -165,6 +182,8 @@ networkStartup(void) {
 
     networkAutostartConfigs(driverState);
 
+    networkDriverUnlock(driverState);
+
     return 0;
 
 out_of_memory:
@@ -172,6 +191,9 @@ out_of_memory:
               "%s", _("networkStartup: out of memory\n"));
 
 error:
+    if (driverState)
+        networkDriverUnlock(driverState);
+
     VIR_FREE(base);
     networkShutdown();
     return -1;
@@ -188,6 +210,7 @@ networkReload(void) {
     if (!driverState)
         return 0;
 
+    networkDriverLock(driverState);
     virNetworkLoadAllConfigs(NULL,
                              &driverState->networks,
                              driverState->networkConfigDir,
@@ -200,7 +223,7 @@ networkReload(void) {
     }
 
     networkAutostartConfigs(driverState);
-
+    networkDriverUnlock(driverState);
     return 0;
 }
 
@@ -220,12 +243,15 @@ networkActive(void) {
     if (!driverState)
         return 0;
 
+    networkDriverLock(driverState);
     for (i = 0 ; i < driverState->networks.count ; i++) {
         virNetworkObjPtr net = driverState->networks.objs[i];
+        virNetworkObjLock(net);
         if (virNetworkIsActive(net))
             active = 1;
+        virNetworkObjUnlock(net);
     }
-
+    networkDriverUnlock(driverState);
     return active;
 }
 
@@ -241,12 +267,16 @@ networkShutdown(void) {
     if (!driverState)
         return -1;
 
+    networkDriverLock(driverState);
+
     /* shutdown active networks */
     for (i = 0 ; i < driverState->networks.count ; i++) {
         virNetworkObjPtr net = driverState->networks.objs[i];
+        virNetworkObjLock(net);
         if (virNetworkIsActive(net))
             networkShutdownNetworkDaemon(NULL, driverState,
                                          driverState->networks.objs[i]);
+        virNetworkObjUnlock(net);
     }
 
     /* free inactive networks */
@@ -260,6 +290,8 @@ networkShutdown(void) {
         brShutdown(driverState->brctl);
     if (driverState->iptables)
         iptablesContextFree(driverState->iptables);
+
+    networkDriverUnlock(driverState);
 
     VIR_FREE(driverState);
 
@@ -801,10 +833,6 @@ static int networkShutdownNetworkDaemon(
         network->newDef = NULL;
     }
 
-    if (!network->configFile)
-        virNetworkRemoveInactive(&driver->networks,
-                                 network);
-
     return 0;
 }
 
@@ -815,7 +843,9 @@ static virNetworkPtr networkLookupByUUID
     virNetworkObjPtr network;
     virNetworkPtr ret = NULL;
 
+    networkDriverLock(driver);
     network = virNetworkFindByUUID(&driver->networks, uuid);
+    networkDriverUnlock(driver);
     if (!network) {
         networkReportError(conn, NULL, NULL, VIR_ERR_NO_NETWORK,
                          "%s", _("no network with matching uuid"));
@@ -825,6 +855,8 @@ static virNetworkPtr networkLookupByUUID
     ret = virGetNetwork(conn, network->def->name, network->def->uuid);
 
 cleanup:
+    if (network)
+        virNetworkObjUnlock(network);
     return ret;
 }
 
@@ -834,7 +866,9 @@ static virNetworkPtr networkLookupByName
     virNetworkObjPtr network;
     virNetworkPtr ret = NULL;
 
+    networkDriverLock(driver);
     network = virNetworkFindByName(&driver->networks, name);
+    networkDriverUnlock(driver);
     if (!network) {
         networkReportError(conn, NULL, NULL, VIR_ERR_NO_NETWORK,
                          "%s", _("no network with matching name"));
@@ -844,6 +878,8 @@ static virNetworkPtr networkLookupByName
     ret = virGetNetwork(conn, network->def->name, network->def->uuid);
 
 cleanup:
+    if (network)
+        virNetworkObjUnlock(network);
     return ret;
 }
 
@@ -866,9 +902,14 @@ static int networkNumNetworks(virConnect
     int nactive = 0, i;
     struct network_driver *driver = conn->networkPrivateData;
 
-    for (i = 0 ; i < driver->networks.count ; i++)
+    networkDriverLock(driver);
+    for (i = 0 ; i < driver->networks.count ; i++) {
+        virNetworkObjLock(driver->networks.objs[i]);
         if (virNetworkIsActive(driver->networks.objs[i]))
             nactive++;
+        virNetworkObjUnlock(driver->networks.objs[i]);
+    }
+    networkDriverUnlock(driver);
 
     return nactive;
 }
@@ -877,19 +918,26 @@ static int networkListNetworks(virConnec
     struct network_driver *driver = conn->networkPrivateData;
     int got = 0, i;
 
+    networkDriverLock(driver);
     for (i = 0 ; i < driver->networks.count && got < nnames ; i++) {
+        virNetworkObjLock(driver->networks.objs[i]);
         if (virNetworkIsActive(driver->networks.objs[i])) {
             if (!(names[got] = strdup(driver->networks.objs[i]->def->name))) {
+                virNetworkObjUnlock(driver->networks.objs[i]);
                 networkReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY,
                                    "%s", _("failed to allocate space for VM name string"));
                 goto cleanup;
             }
             got++;
         }
+        virNetworkObjUnlock(driver->networks.objs[i]);
     }
+    networkDriverUnlock(driver);
+
     return got;
 
  cleanup:
+    networkDriverUnlock(driver);
     for (i = 0 ; i < got ; i++)
         VIR_FREE(names[i]);
     return -1;
@@ -899,9 +947,14 @@ static int networkNumDefinedNetworks(vir
     int ninactive = 0, i;
     struct network_driver *driver = conn->networkPrivateData;
 
-    for (i = 0 ; i < driver->networks.count ; i++)
+    networkDriverLock(driver);
+    for (i = 0 ; i < driver->networks.count ; i++) {
+        virNetworkObjLock(driver->networks.objs[i]);
         if (!virNetworkIsActive(driver->networks.objs[i]))
             ninactive++;
+        virNetworkObjUnlock(driver->networks.objs[i]);
+    }
+    networkDriverUnlock(driver);
 
     return ninactive;
 }
@@ -910,19 +963,25 @@ static int networkListDefinedNetworks(vi
     struct network_driver *driver = conn->networkPrivateData;
     int got = 0, i;
 
+    networkDriverLock(driver);
     for (i = 0 ; i < driver->networks.count && got < nnames ; i++) {
+        virNetworkObjLock(driver->networks.objs[i]);
         if (!virNetworkIsActive(driver->networks.objs[i])) {
             if (!(names[got] = strdup(driver->networks.objs[i]->def->name))) {
+                virNetworkObjUnlock(driver->networks.objs[i]);
                 networkReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY,
                                    "%s", _("failed to allocate space for VM name string"));
                 goto cleanup;
             }
             got++;
         }
+        virNetworkObjUnlock(driver->networks.objs[i]);
     }
+    networkDriverUnlock(driver);
     return got;
 
  cleanup:
+    networkDriverUnlock(driver);
     for (i = 0 ; i < got ; i++)
         VIR_FREE(names[i]);
     return -1;
@@ -931,8 +990,10 @@ static virNetworkPtr networkCreate(virCo
 static virNetworkPtr networkCreate(virConnectPtr conn, const char *xml) {
     struct network_driver *driver = conn->networkPrivateData;
     virNetworkDefPtr def;
-    virNetworkObjPtr network;
+    virNetworkObjPtr network = NULL;
     virNetworkPtr ret = NULL;
+
+    networkDriverLock(driver);
 
     if (!(def = virNetworkDefParseString(conn, xml)))
         goto cleanup;
@@ -944,8 +1005,10 @@ static virNetworkPtr networkCreate(virCo
     def = NULL;
 
     if (networkStartNetworkDaemon(conn, driver, network) < 0) {
+        virNetworkObjUnlock(network);
         virNetworkRemoveInactive(&driver->networks,
                                  network);
+        network = NULL;
         goto cleanup;
     }
 
@@ -953,14 +1016,19 @@ static virNetworkPtr networkCreate(virCo
 
 cleanup:
     virNetworkDefFree(def);
+    if (network)
+        virNetworkObjUnlock(network);
+    networkDriverUnlock(driver);
     return ret;
 }
 
 static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) {
     struct network_driver *driver = conn->networkPrivateData;
     virNetworkDefPtr def;
-    virNetworkObjPtr network;
+    virNetworkObjPtr network = NULL;
     virNetworkPtr ret = NULL;
+
+    networkDriverLock(driver);
 
     if (!(def = virNetworkDefParseString(conn, xml)))
         goto cleanup;
@@ -975,8 +1043,10 @@ static virNetworkPtr networkDefine(virCo
                              driver->networkConfigDir,
                              driver->networkAutostartDir,
                              network) < 0) {
+        virNetworkObjUnlock(network);
         virNetworkRemoveInactive(&driver->networks,
                                  network);
+        network = NULL;
         goto cleanup;
     }
 
@@ -984,13 +1054,18 @@ static virNetworkPtr networkDefine(virCo
 
 cleanup:
     virNetworkDefFree(def);
+    if (network)
+        virNetworkObjUnlock(network);
+    networkDriverUnlock(driver);
     return ret;
 }
 
 static int networkUndefine(virNetworkPtr net) {
     struct network_driver *driver = net->conn->networkPrivateData;
-    virNetworkObjPtr network;
+    virNetworkObjPtr network = NULL;
     int ret = -1;
+
+    networkDriverLock(driver);
 
     network = virNetworkFindByUUID(&driver->networks, net->uuid);
     if (!network) {
@@ -1008,11 +1083,16 @@ static int networkUndefine(virNetworkPtr
     if (virNetworkDeleteConfig(net->conn, network) < 0)
         goto cleanup;
 
+    virNetworkObjUnlock(network);
     virNetworkRemoveInactive(&driver->networks,
                              network);
+    network = NULL;
     ret = 0;
 
 cleanup:
+    if (network)
+        virNetworkObjUnlock(network);
+    networkDriverUnlock(driver);
     return ret;
 }
 
@@ -1021,7 +1101,10 @@ static int networkStart(virNetworkPtr ne
     virNetworkObjPtr network;
     int ret = -1;
 
+    networkDriverLock(driver);
     network = virNetworkFindByUUID(&driver->networks, net->uuid);
+    networkDriverUnlock(driver);
+
     if (!network) {
         networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK,
                            "%s", _("no network with matching uuid"));
@@ -1031,6 +1114,8 @@ static int networkStart(virNetworkPtr ne
     ret = networkStartNetworkDaemon(net->conn, driver, network);
 
 cleanup:
+    if (network)
+        virNetworkObjUnlock(network);
     return ret;
 }
 
@@ -1039,7 +1124,10 @@ static int networkDestroy(virNetworkPtr 
     virNetworkObjPtr network;
     int ret = -1;
 
+    networkDriverLock(driver);
     network = virNetworkFindByUUID(&driver->networks, net->uuid);
+    networkDriverUnlock(driver);
+
     if (!network) {
         networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK,
                            "%s", _("no network with matching uuid"));
@@ -1047,8 +1135,15 @@ static int networkDestroy(virNetworkPtr 
     }
 
     ret = networkShutdownNetworkDaemon(net->conn, driver, network);
+    if (!network->configFile) {
+        virNetworkRemoveInactive(&driver->networks,
+                                 network);
+        network = NULL;
+    }
 
 cleanup:
+    if (network)
+        virNetworkObjUnlock(network);
     return ret;
 }
 
@@ -1057,7 +1152,10 @@ static char *networkDumpXML(virNetworkPt
     virNetworkObjPtr network;
     char *ret = NULL;
 
+    networkDriverLock(driver);
     network = virNetworkFindByUUID(&driver->networks, net->uuid);
+    networkDriverUnlock(driver);
+
     if (!network) {
         networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK,
                            "%s", _("no network with matching uuid"));
@@ -1067,6 +1165,8 @@ static char *networkDumpXML(virNetworkPt
     ret = virNetworkDefFormat(net->conn, network->def);
 
 cleanup:
+    if (network)
+        virNetworkObjUnlock(network);
     return ret;
 }
 
@@ -1075,7 +1175,10 @@ static char *networkGetBridgeName(virNet
     virNetworkObjPtr network;
     char *bridge = NULL;
 
+    networkDriverLock(driver);
     network = virNetworkFindByUUID(&driver->networks, net->uuid);
+    networkDriverUnlock(driver);
+
     if (!network) {
         networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK,
                            "%s", _("no network with matching id"));
@@ -1088,6 +1191,8 @@ static char *networkGetBridgeName(virNet
                            "%s", _("failed to allocate space for network bridge string"));
 
 cleanup:
+    if (network)
+        virNetworkObjUnlock(network);
     return bridge;
 }
 
@@ -1097,7 +1202,9 @@ static int networkGetAutostart(virNetwor
     virNetworkObjPtr network;
     int ret = -1;
 
+    networkDriverLock(driver);
     network = virNetworkFindByUUID(&driver->networks, net->uuid);
+    networkDriverUnlock(driver);
     if (!network) {
         networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK,
                          "%s", _("no network with matching uuid"));
@@ -1108,6 +1215,8 @@ static int networkGetAutostart(virNetwor
     ret = 0;
 
 cleanup:
+    if (network)
+        virNetworkObjUnlock(network);
     return ret;
 }
 
@@ -1117,7 +1226,10 @@ static int networkSetAutostart(virNetwor
     virNetworkObjPtr network;
     int ret = -1;
 
+    networkDriverLock(driver);
     network = virNetworkFindByUUID(&driver->networks, net->uuid);
+    networkDriverUnlock(driver);
+
     if (!network) {
         networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK,
                          "%s", _("no network with matching uuid"));
@@ -1157,6 +1269,8 @@ static int networkSetAutostart(virNetwor
     ret = 0;
 
 cleanup:
+    if (network)
+        virNetworkObjUnlock(network);
     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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]