[libvirt PATCH 04/10] interface_backend_netcf: Use automatic mutex management

Friday, 25 March 2022

Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt;
---
 src/interface/interface_backend_netcf.c | 192 +++++++++++-------------
 1 file changed, 86 insertions(+), 106 deletions(-)

diff --git a/src/interface/interface_backend_netcf.c
b/src/interface/interface_backend_netcf.c
index 92698ee769..d625964421 100644
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -155,31 +155,26 @@ netcfStateCleanup(void)
 static int
 netcfStateReload(void)
 {
-    int ret = -1;
-
     if (!driver)
         return 0;
 
-    virObjectLock(driver);
-    ncf_close(driver->netcf);
-    if (ncf_init(&driver->netcf, NULL) != 0) {
-        /* this isn't a good situation, because we can't shut down the
-         * driver as there may still be connections to it. If we set
-         * the netcf handle to NULL, any subsequent calls to netcf
-         * will just fail rather than causing a crash. Not ideal, but
-         * livable (since this should never happen).
-         */
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("failed to re-init netcf"));
-        driver->netcf = NULL;
-        goto cleanup;
+    VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+        ncf_close(driver->netcf);
+        if (ncf_init(&driver->netcf, NULL) != 0) {
+            /* this isn't a good situation, because we can't shut down the
+             * driver as there may still be connections to it. If we set
+             * the netcf handle to NULL, any subsequent calls to netcf
+             * will just fail rather than causing a crash. Not ideal, but
+             * livable (since this should never happen).
+             */
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("failed to re-init netcf"));
+            driver->netcf = NULL;
+            return -1;
+        }
     }
 
-    ret = 0;
- cleanup:
-    virObjectUnlock(driver);
-
-    return ret;
+    return 0;
 }
 
 
@@ -528,66 +523,68 @@ static int netcfConnectListInterfacesImpl(virConnectPtr conn,
 
 static int netcfConnectNumOfInterfaces(virConnectPtr conn)
 {
-    int count;
+    int count = -1;
 
     if (virConnectNumOfInterfacesEnsureACL(conn) < 0)
         return -1;
 
-    virObjectLock(driver);
-    count = netcfConnectNumOfInterfacesImpl(conn,
-                                            NETCF_IFACE_ACTIVE,
-                                            virConnectNumOfInterfacesCheckACL);
-    virObjectUnlock(driver);
+    VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+        count = netcfConnectNumOfInterfacesImpl(conn,
+                                                NETCF_IFACE_ACTIVE,
+                                                virConnectNumOfInterfacesCheckACL);
+    }
+
     return count;
 }
 
 static int netcfConnectListInterfaces(virConnectPtr conn, char **const names, int
nnames)
 {
-    int count;
+    int count = -1;
 
     if (virConnectListInterfacesEnsureACL(conn) < 0)
         return -1;
 
-    virObjectLock(driver);
-    count = netcfConnectListInterfacesImpl(conn,
-                                           NETCF_IFACE_ACTIVE,
-                                           names, nnames,
-                                           virConnectListInterfacesCheckACL);
-    virObjectUnlock(driver);
-    return count;
+    VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+        count = netcfConnectListInterfacesImpl(conn,
+                                               NETCF_IFACE_ACTIVE,
+                                               names, nnames,
+                                               virConnectListInterfacesCheckACL);
+    }
 
+    return count;
 }
 
 static int netcfConnectNumOfDefinedInterfaces(virConnectPtr conn)
 {
-    int count;
+    int count = -1;
 
     if (virConnectNumOfDefinedInterfacesEnsureACL(conn) < 0)
         return -1;
 
-    virObjectLock(driver);
-    count = netcfConnectNumOfInterfacesImpl(conn,
-                                            NETCF_IFACE_INACTIVE,
-                                            virConnectNumOfDefinedInterfacesCheckACL);
-    virObjectUnlock(driver);
+    VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+        count = netcfConnectNumOfInterfacesImpl(conn,
+                                                NETCF_IFACE_INACTIVE,
+                                               
virConnectNumOfDefinedInterfacesCheckACL);
+    }
+
     return count;
 }
 
 static int netcfConnectListDefinedInterfaces(virConnectPtr conn, char **const names, int
nnames)
 {
-    int count;
+    int count = -1;
 
     if (virConnectListDefinedInterfacesEnsureACL(conn) < 0)
         return -1;
 
-    virObjectLock(driver);
-    count = netcfConnectListInterfacesImpl(conn,
-                                           NETCF_IFACE_INACTIVE,
-                                           names, nnames,
-                                           virConnectListDefinedInterfacesCheckACL);
-    virObjectUnlock(driver);
-    return count;
+    VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+        count = netcfConnectListInterfacesImpl(conn,
+                                               NETCF_IFACE_INACTIVE,
+                                               names, nnames,
+                                               virConnectListDefinedInterfacesCheckACL);
+    }
 
+    return count;
 }
 
 #define MATCH(FLAG) (flags & (FLAG))
@@ -731,8 +728,8 @@ static virInterfacePtr netcfInterfaceLookupByName(virConnectPtr conn,
     struct netcf_if *iface;
     virInterfacePtr ret = NULL;
     g_autoptr(virInterfaceDef) def = NULL;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
 
-    virObjectLock(driver);
     iface = ncf_lookup_by_name(driver->netcf, name);
     if (!iface) {
         const char *errmsg, *details;
@@ -759,7 +756,6 @@ static virInterfacePtr netcfInterfaceLookupByName(virConnectPtr conn,
 
  cleanup:
     ncf_if_free(iface);
-    virObjectUnlock(driver);
     return ret;
 }
 
@@ -770,8 +766,8 @@ static virInterfacePtr netcfInterfaceLookupByMACString(virConnectPtr
conn,
     int niface;
     virInterfacePtr ret = NULL;
     g_autoptr(virInterfaceDef) def = NULL;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
 
-    virObjectLock(driver);
     niface = ncf_lookup_by_mac_string(driver->netcf, macstr, 1, &iface);
 
     if (niface < 0) {
@@ -806,7 +802,6 @@ static virInterfacePtr netcfInterfaceLookupByMACString(virConnectPtr
conn,
 
  cleanup:
     ncf_if_free(iface);
-    virObjectUnlock(driver);
     return ret;
 }
 
@@ -818,11 +813,10 @@ static char *netcfInterfaceGetXMLDesc(virInterfacePtr ifinfo,
     g_autoptr(virInterfaceDef) ifacedef = NULL;
     char *ret = NULL;
     bool active;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
 
     virCheckFlags(VIR_INTERFACE_XML_INACTIVE, NULL);
 
-    virObjectLock(driver);
-
     iface = interfaceDriverGetNetcfIF(driver->netcf, ifinfo);
     if (!iface) {
         /* helper already reported error */
@@ -865,7 +859,6 @@ static char *netcfInterfaceGetXMLDesc(virInterfacePtr ifinfo,
  cleanup:
     ncf_if_free(iface);
     VIR_FREE(xmlstr);
-    virObjectUnlock(driver);
     return ret;
 }
 
@@ -877,11 +870,10 @@ static virInterfacePtr netcfInterfaceDefineXML(virConnectPtr conn,
     char *xmlstr = NULL;
     g_autoptr(virInterfaceDef) ifacedef = NULL;
     virInterfacePtr ret = NULL;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
 
     virCheckFlags(VIR_INTERFACE_DEFINE_VALIDATE, NULL);
 
-    virObjectLock(driver);
-
     ifacedef = virInterfaceDefParseString(xml, flags);
     if (!ifacedef) {
         /* error was already reported */
@@ -913,7 +905,6 @@ static virInterfacePtr netcfInterfaceDefineXML(virConnectPtr conn,
  cleanup:
     ncf_if_free(iface);
     VIR_FREE(xmlstr);
-    virObjectUnlock(driver);
     return ret;
 }
 
@@ -922,8 +913,7 @@ static int netcfInterfaceUndefine(virInterfacePtr ifinfo)
     struct netcf_if *iface = NULL;
     g_autoptr(virInterfaceDef) def = NULL;
     int ret = -1;
-
-    virObjectLock(driver);
+    VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
 
     iface = interfaceDriverGetNetcfIF(driver->netcf, ifinfo);
     if (!iface) {
@@ -951,7 +941,6 @@ static int netcfInterfaceUndefine(virInterfacePtr ifinfo)
 
  cleanup:
     ncf_if_free(iface);
-    virObjectUnlock(driver);
     return ret;
 }
 
@@ -962,11 +951,10 @@ static int netcfInterfaceCreate(virInterfacePtr ifinfo,
     g_autoptr(virInterfaceDef) def = NULL;
     int ret = -1;
     bool active;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
 
     virCheckFlags(0, -1);
 
-    virObjectLock(driver);
-
     iface = interfaceDriverGetNetcfIF(driver->netcf, ifinfo);
     if (!iface) {
         /* helper already reported error */
@@ -1002,7 +990,6 @@ static int netcfInterfaceCreate(virInterfacePtr ifinfo,
 
  cleanup:
     ncf_if_free(iface);
-    virObjectUnlock(driver);
     return ret;
 }
 
@@ -1013,11 +1000,10 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo,
     g_autoptr(virInterfaceDef) def = NULL;
     int ret = -1;
     bool active;
+    VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
 
     virCheckFlags(0, -1);
 
-    virObjectLock(driver);
-
     iface = interfaceDriverGetNetcfIF(driver->netcf, ifinfo);
     if (!iface) {
         /* helper already reported error */
@@ -1053,7 +1039,6 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo,
 
  cleanup:
     ncf_if_free(iface);
-    virObjectUnlock(driver);
     return ret;
 }
 
@@ -1063,8 +1048,7 @@ static int netcfInterfaceIsActive(virInterfacePtr ifinfo)
     g_autoptr(virInterfaceDef) def = NULL;
     int ret = -1;
     bool active;
-
-    virObjectLock(driver);
+    VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
 
     iface = interfaceDriverGetNetcfIF(driver->netcf, ifinfo);
     if (!iface) {
@@ -1085,82 +1069,78 @@ static int netcfInterfaceIsActive(virInterfacePtr ifinfo)
 
  cleanup:
     ncf_if_free(iface);
-    virObjectUnlock(driver);
     return ret;
 }
 
 static int netcfInterfaceChangeBegin(virConnectPtr conn, unsigned int flags)
 {
-    int ret;
+    int ret = -1;
 
     virCheckFlags(0, -1); /* currently flags must be 0 */
 
     if (virInterfaceChangeBeginEnsureACL(conn) < 0)
         return -1;
 
-    virObjectLock(driver);
-
-    ret = ncf_change_begin(driver->netcf, 0);
-    if (ret < 0) {
-        const char *errmsg, *details;
-        int errcode = ncf_error(driver->netcf, &errmsg, &details);
-        virReportError(netcf_to_vir_err(errcode),
-                       _("failed to begin transaction: %s%s%s"),
-                       errmsg, details ? " - " : "",
-                       NULLSTR_EMPTY(details));
+    VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+        ret = ncf_change_begin(driver->netcf, 0);
+        if (ret < 0) {
+            const char *errmsg, *details;
+            int errcode = ncf_error(driver->netcf, &errmsg, &details);
+            virReportError(netcf_to_vir_err(errcode),
+                           _("failed to begin transaction: %s%s%s"),
+                           errmsg, details ? " - " : "",
+                           NULLSTR_EMPTY(details));
+        }
     }
 
-    virObjectUnlock(driver);
     return ret;
 }
 
 static int netcfInterfaceChangeCommit(virConnectPtr conn, unsigned int flags)
 {
-    int ret;
+    int ret = -1;
 
     virCheckFlags(0, -1); /* currently flags must be 0 */
 
     if (virInterfaceChangeCommitEnsureACL(conn) < 0)
         return -1;
 
-    virObjectLock(driver);
-
-    ret = ncf_change_commit(driver->netcf, 0);
-    if (ret < 0) {
-        const char *errmsg, *details;
-        int errcode = ncf_error(driver->netcf, &errmsg, &details);
-        virReportError(netcf_to_vir_err(errcode),
-                       _("failed to commit transaction: %s%s%s"),
-                       errmsg, details ? " - " : "",
-                       NULLSTR_EMPTY(details));
+    VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+        ret = ncf_change_commit(driver->netcf, 0);
+        if (ret < 0) {
+            const char *errmsg, *details;
+            int errcode = ncf_error(driver->netcf, &errmsg, &details);
+            virReportError(netcf_to_vir_err(errcode),
+                           _("failed to commit transaction: %s%s%s"),
+                           errmsg, details ? " - " : "",
+                           NULLSTR_EMPTY(details));
+        }
     }
 
-    virObjectUnlock(driver);
     return ret;
 }
 
 static int netcfInterfaceChangeRollback(virConnectPtr conn, unsigned int flags)
 {
-    int ret;
+    int ret = -1;
 
     virCheckFlags(0, -1); /* currently flags must be 0 */
 
     if (virInterfaceChangeRollbackEnsureACL(conn) < 0)
         return -1;
 
-    virObjectLock(driver);
-
-    ret = ncf_change_rollback(driver->netcf, 0);
-    if (ret < 0) {
-        const char *errmsg, *details;
-        int errcode = ncf_error(driver->netcf, &errmsg, &details);
-        virReportError(netcf_to_vir_err(errcode),
-                       _("failed to rollback transaction: %s%s%s"),
-                       errmsg, details ? " - " : "",
-                       NULLSTR_EMPTY(details));
+    VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+        ret = ncf_change_rollback(driver->netcf, 0);
+        if (ret < 0) {
+            const char *errmsg, *details;
+            int errcode = ncf_error(driver->netcf, &errmsg, &details);
+            virReportError(netcf_to_vir_err(errcode),
+                           _("failed to rollback transaction: %s%s%s"),
+                           errmsg, details ? " - " : "",
+                           NULLSTR_EMPTY(details));
+        }
     }
 
-    virObjectUnlock(driver);
     return ret;
 }
 
-- 
2.31.1


    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

[libvirt PATCH 04/10] interface_backend_netcf: Use automatic mutex management