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

[libvirt] nwfilter: grab driver lock earlier during init (bz96649)



This patch is in relation to Bug 966449:

https://bugzilla.redhat.com/show_bug.cgi?id=966449

Below is a possible patch addressing the coredump.

Thread 1 must be calling  nwfilterDriverRemoveDBusMatches(). It does so
with nwfilterDriverLock held. In the patch below I am now moving the
nwfilterDriverLock(driverState) further up so that the initialization,
which seems to either take a long time or is entirely stuck, occurs with
the lock held and the shutdown cannot occur at the same time. 

To avoid having to make the nwfilterDriverLock lockable multiple times /
recursive I changed the virNWFilterDriverIsWatchingFirewallD to take as
an argument whether it has to grab that lock. There's only a single
caller at the moment that calls this function during initialization. We
could remove this lock entirely and maybe append to the name of the
function NoLock (?).

---
 src/nwfilter/nwfilter_driver.c            |   18 +++++++++++++-----
 src/nwfilter/nwfilter_driver.h            |    2 +-
 src/nwfilter/nwfilter_ebiptables_driver.c |    7 ++++++-
 3 files changed, 20 insertions(+), 7 deletions(-)

Index: libvirt/src/nwfilter/nwfilter_driver.c
===================================================================
--- libvirt.orig/src/nwfilter/nwfilter_driver.c
+++ libvirt/src/nwfilter/nwfilter_driver.c
@@ -191,6 +191,8 @@ nwfilterStateInitialize(bool privileged,
     if (!privileged)
         return 0;
 
+    nwfilterDriverLock(driverState);
+
     if (virNWFilterIPAddrMapInit() < 0)
         goto err_free_driverstate;
     if (virNWFilterLearnInit() < 0)
@@ -203,8 +205,6 @@ nwfilterStateInitialize(bool privileged,
     if (virNWFilterConfLayerInit(virNWFilterDomainFWUpdateCB) < 0)
         goto err_techdrivers_shutdown;
 
-    nwfilterDriverLock(driverState);
-
     /*
      * startup the DBus late so we don't get a reload signal while
      * initializing
@@ -309,21 +309,29 @@ nwfilterStateReload(void) {
 /**
  * virNWFilterIsWatchingFirewallD:
  *
+ * @needDriverLock: Provide 'true' if this function needs to grab
+ *                  the nwfilter driver lock, 'false' otherwise,
+ *                  which may be the case during initialization
+ *
  * Checks if the nwfilter has the DBus watches for FirewallD installed.
  *
  * Returns true if it is watching firewalld, false otherwise
  */
 bool
-virNWFilterDriverIsWatchingFirewallD(void)
+virNWFilterDriverIsWatchingFirewallD(bool needDriverLock)
 {
     bool ret;
 
     if (!driverState)
         return false;
 
-    nwfilterDriverLock(driverState);
+    if (needDriverLock)
+        nwfilterDriverLock(driverState);
+
     ret = driverState->watchingFirewallD;
-    nwfilterDriverUnlock(driverState);
+
+    if (needDriverLock)
+        nwfilterDriverUnlock(driverState);
 
     return ret;
 }
Index: libvirt/src/nwfilter/nwfilter_driver.h
===================================================================
--- libvirt.orig/src/nwfilter/nwfilter_driver.h
+++ libvirt/src/nwfilter/nwfilter_driver.h
@@ -33,6 +33,6 @@
 
 int nwfilterRegister(void);
 
-bool virNWFilterDriverIsWatchingFirewallD(void);
+bool virNWFilterDriverIsWatchingFirewallD(bool needDriverLock);
 
 #endif /* __VIR_NWFILTER_DRIVER_H__ */
Index: libvirt/src/nwfilter/nwfilter_ebiptables_driver.c
===================================================================
--- libvirt.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -4191,7 +4191,12 @@ ebiptablesDriverInitWithFirewallD(void)
     int status;
     int ret = -1;
 
-    if (!virNWFilterDriverIsWatchingFirewallD())
+    /*
+     * check whether we are watching firewalld
+     * Since we call this function during initialization we won't need
+     * to have it get the lock, so we pass 'false'.
+     */
+    if (!virNWFilterDriverIsWatchingFirewallD(false))
         return -1;
 
     firewall_cmd_path = virFindFileInPath("firewall-cmd");


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