Remove use of virConnectPtr from all remaining nwfilter code

The virConnectPtr is passed around loads of nwfilter code in
order to provide it as a parameter to the callback registered
by the virt drivers. None of the virt drivers use this param
though, so it serves no purpose.

Avoiding the need to pass a virConnectPtr means that the
nwfilterStateReload method no longer needs to open a bogus
QEMU driver connection. This addresses a race condition that
can lead to a crash on startup.

The nwfilter driver starts before the QEMU driver and registers
some callbacks with DBus to detect firewalld reload. If the
firewalld reload happens while the QEMU driver is still starting
up though, the nwfilterStateReload method will open a connection
to the partially initialized QEMU driver and cause a crash.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 999d72fbd59ea712128ae294b69b6a54039d757b)

Conflicts:
  src/nwfilter/nwfilter_driver.c
    - *EnsureACL*() was added after this branch was created, and
       caused two small conflicts in the context around a hunk.
    - nwfilterDriverReload() was renamed to nwfilterStateReload()
      upstream.

diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index aafa56f7ade39b3d0d7115abad4b063b09e13968..f5b6bcc3267fad5d8a08503722507a762be61376 100644 (file)
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2759,8 +2759,7 @@ cleanup:
 
 
 static int
-_virNWFilterDefLoopDetect(virConnectPtr conn,
-                          virNWFilterObjListPtr nwfilters,
+_virNWFilterDefLoopDetect(virNWFilterObjListPtr nwfilters,
                           virNWFilterDefPtr def,
                           const char *filtername)
 {
@@ -2784,7 +2783,7 @@ _virNWFilterDefLoopDetect(virConnectPtr conn,
             obj = virNWFilterObjFindByName(nwfilters,
                                            entry->include->filterref);
             if (obj) {
-                rc = _virNWFilterDefLoopDetect(conn, nwfilters,
+                rc = _virNWFilterDefLoopDetect(nwfilters,
                                                obj->def, filtername);
 
                 virNWFilterObjUnlock(obj);
@@ -2800,7 +2799,6 @@ _virNWFilterDefLoopDetect(virConnectPtr conn,
 
 /*
  * virNWFilterDefLoopDetect:
- * @conn: pointer to virConnect object
  * @nwfilters : the nwfilters to search
  * @def : the filter definition that may add a loop and is to be tested
  *
@@ -2810,11 +2808,10 @@ _virNWFilterDefLoopDetect(virConnectPtr conn,
  * Returns 0 in case no loop was detected, -1 otherwise.
  */
 static int
-virNWFilterDefLoopDetect(virConnectPtr conn,
-                         virNWFilterObjListPtr nwfilters,
+virNWFilterDefLoopDetect(virNWFilterObjListPtr nwfilters,
                          virNWFilterDefPtr def)
 {
-    return _virNWFilterDefLoopDetect(conn, nwfilters, def, def->name);
+    return _virNWFilterDefLoopDetect(nwfilters, def, def->name);
 }
 
 int nCallbackDriver;
@@ -2873,7 +2870,7 @@ static void *virNWFilterDomainFWUpdateOpaque;
  * error. This should be called upon reloading of the driver.
  */
 int
-virNWFilterInstFiltersOnAllVMs(virConnectPtr conn)
+virNWFilterInstFiltersOnAllVMs(void)
 {
     int i;
     struct domUpdateCBStruct cb = {
@@ -2883,15 +2880,14 @@ virNWFilterInstFiltersOnAllVMs(virConnectPtr conn)
     };
 
     for (i = 0; i < nCallbackDriver; i++)
-        callbackDrvArray[i]->vmFilterRebuild(conn,
-                                             virNWFilterDomainFWUpdateCB,
+        callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB,
                                              &cb);
 
     return 0;
 }
 
 static int
-virNWFilterTriggerVMFilterRebuild(virConnectPtr conn)
+virNWFilterTriggerVMFilterRebuild(void)
 {
     int i;
     int ret = 0;
@@ -2905,8 +2901,7 @@ virNWFilterTriggerVMFilterRebuild(virConnectPtr conn)
         return -1;
 
     for (i = 0; i < nCallbackDriver; i++) {
-        if (callbackDrvArray[i]->vmFilterRebuild(conn,
-                                                 virNWFilterDomainFWUpdateCB,
+        if (callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB,
                                                  &cb) < 0)
             ret = -1;
     }
@@ -2915,15 +2910,13 @@ virNWFilterTriggerVMFilterRebuild(virConnectPtr conn)
         cb.step = STEP_TEAR_NEW; /* rollback */
 
         for (i = 0; i < nCallbackDriver; i++)
-            callbackDrvArray[i]->vmFilterRebuild(conn,
-                                                 virNWFilterDomainFWUpdateCB,
+            callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB,
                                                  &cb);
     } else {
         cb.step = STEP_TEAR_OLD; /* switch over */
 
         for (i = 0; i < nCallbackDriver; i++)
-            callbackDrvArray[i]->vmFilterRebuild(conn,
-                                                 virNWFilterDomainFWUpdateCB,
+            callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB,
                                                  &cb);
     }
 
@@ -2934,14 +2927,13 @@ virNWFilterTriggerVMFilterRebuild(virConnectPtr conn)
 
 
 int
-virNWFilterTestUnassignDef(virConnectPtr conn,
-                           virNWFilterObjPtr nwfilter)
+virNWFilterTestUnassignDef(virNWFilterObjPtr nwfilter)
 {
     int rc = 0;
 
     nwfilter->wantRemoved = 1;
     /* trigger the update on VMs referencing the filter */
-    if (virNWFilterTriggerVMFilterRebuild(conn))
+    if (virNWFilterTriggerVMFilterRebuild())
         rc = -1;
 
     nwfilter->wantRemoved = 0;
@@ -2980,8 +2972,7 @@ cleanup:
 }
 
 virNWFilterObjPtr
-virNWFilterObjAssignDef(virConnectPtr conn,
-                        virNWFilterObjListPtr nwfilters,
+virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
                         virNWFilterDefPtr def)
 {
     virNWFilterObjPtr nwfilter;
@@ -3000,7 +2991,7 @@ virNWFilterObjAssignDef(virConnectPtr conn,
         virNWFilterObjUnlock(nwfilter);
     }
 
-    if (virNWFilterDefLoopDetect(conn, nwfilters, def) < 0) {
+    if (virNWFilterDefLoopDetect(nwfilters, def) < 0) {
         virReportError(VIR_ERR_OPERATION_FAILED,
                        "%s", _("filter would introduce a loop"));
         return NULL;
@@ -3019,7 +3010,7 @@ virNWFilterObjAssignDef(virConnectPtr conn,
 
         nwfilter->newDef = def;
         /* trigger the update on VMs referencing the filter */
-        if (virNWFilterTriggerVMFilterRebuild(conn)) {
+        if (virNWFilterTriggerVMFilterRebuild()) {
             nwfilter->newDef = NULL;
             virNWFilterUnlockFilterUpdates();
             virNWFilterObjUnlock(nwfilter);
@@ -3064,8 +3055,7 @@ virNWFilterObjAssignDef(virConnectPtr conn,
 
 
 static virNWFilterObjPtr
-virNWFilterObjLoad(virConnectPtr conn,
-                   virNWFilterObjListPtr nwfilters,
+virNWFilterObjLoad(virNWFilterObjListPtr nwfilters,
                    const char *file,
                    const char *path)
 {
@@ -3084,7 +3074,7 @@ virNWFilterObjLoad(virConnectPtr conn,
         return NULL;
     }
 
-    if (!(nwfilter = virNWFilterObjAssignDef(conn, nwfilters, def))) {
+    if (!(nwfilter = virNWFilterObjAssignDef(nwfilters, def))) {
         virNWFilterDefFree(def);
         return NULL;
     }
@@ -3102,8 +3092,7 @@ virNWFilterObjLoad(virConnectPtr conn,
 
 
 int
-virNWFilterLoadAllConfigs(virConnectPtr conn,
-                          virNWFilterObjListPtr nwfilters,
+virNWFilterLoadAllConfigs(virNWFilterObjListPtr nwfilters,
                           const char *configDir)
 {
     DIR *dir;
@@ -3131,7 +3120,7 @@ virNWFilterLoadAllConfigs(virConnectPtr conn,
         if (!(path = virFileBuildPath(configDir, entry->d_name, NULL)))
             continue;
 
-        nwfilter = virNWFilterObjLoad(conn, nwfilters, entry->d_name, path);
+        nwfilter = virNWFilterObjLoad(nwfilters, entry->d_name, path);
         if (nwfilter)
             virNWFilterObjUnlock(nwfilter);
 

diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h
index ea90fbd425dbab46ca238cfa3a355561492d4287..1b72ba8cba4074d071c5a7c26045da695cd340c2 100644 (file)
--- a/src/conf/nwfilter_conf.h
+++ b/src/conf/nwfilter_conf.h
@@ -688,12 +688,10 @@ int virNWFilterObjSaveDef(virNWFilterDriverStatePtr driver,
 
 int virNWFilterObjDeleteDef(virNWFilterObjPtr nwfilter);
 
-virNWFilterObjPtr virNWFilterObjAssignDef(virConnectPtr conn,
-                                          virNWFilterObjListPtr nwfilters,
+virNWFilterObjPtr virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
                                           virNWFilterDefPtr def);
 
-int virNWFilterTestUnassignDef(virConnectPtr conn,
-                               virNWFilterObjPtr nwfilter);
+int virNWFilterTestUnassignDef(virNWFilterObjPtr nwfilter);
 
 virNWFilterDefPtr virNWFilterDefParseNode(xmlDocPtr xml,
                                           xmlNodePtr root);
@@ -707,8 +705,7 @@ int virNWFilterSaveXML(const char *configDir,
 int virNWFilterSaveConfig(const char *configDir,
                           virNWFilterDefPtr def);
 
-int virNWFilterLoadAllConfigs(virConnectPtr conn,
-                              virNWFilterObjListPtr nwfilters,
+int virNWFilterLoadAllConfigs(virNWFilterObjListPtr nwfilters,
                               const char *configDir);
 
 char *virNWFilterConfigFile(const char *dir,
@@ -726,11 +723,10 @@ void virNWFilterUnlockFilterUpdates(void);
 int virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB, void *opaque);
 void virNWFilterConfLayerShutdown(void);
 
-int virNWFilterInstFiltersOnAllVMs(virConnectPtr conn);
+int virNWFilterInstFiltersOnAllVMs(void);
 
 
-typedef int (*virNWFilterRebuild)(virConnectPtr conn,
-                                  virDomainObjListIterator domUpdateCB,
+typedef int (*virNWFilterRebuild)(virDomainObjListIterator domUpdateCB,
                                   void *data);
 typedef void (*virNWFilterVoidCall)(void);
 

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 338b8eb84f471c96062a8efba48a8878b00216e7..17126d4b0ed88362a29c1869c22e4d86f32afccc 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -82,8 +82,7 @@ virLXCDriverPtr lxc_driver = NULL;
 
 /* callbacks for nwfilter */
 static int
-lxcVMFilterRebuild(virConnectPtr conn ATTRIBUTE_UNUSED,
-                   virDomainObjListIterator iter, void *data)
+lxcVMFilterRebuild(virDomainObjListIterator iter, void *data)
 {
     return virDomainObjListForEach(lxc_driver->domains, iter, data);
 }

diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 22423dd5f73b74aaf966710d88ce12a3955ba12b..743947b36bbeb48f8d388a829dc68658b565a8d9 100644 (file)
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -2,7 +2,7 @@
  * nwfilter_driver.c: core driver for network filter APIs
  *                    (based on storage_driver.c)
  *
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2011, 2014 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  * Copyright (C) 2010 IBM Corporation
  * Copyright (C) 2010 Stefan Berger
@@ -230,8 +230,7 @@ nwfilterDriverStartup(bool privileged,
 
     VIR_FREE(base);
 
-    if (virNWFilterLoadAllConfigs(NULL,
-                                  &driverState->nwfilters,
+    if (virNWFilterLoadAllConfigs(&driverState->nwfilters,
                                   driverState->configDir) < 0)
         goto error;
 
@@ -270,37 +269,28 @@ err_free_driverstate:
  * files and update its state
  */
 static int
-nwfilterDriverReload(void) {
-    virConnectPtr conn;
-
-    if (!driverState) {
+nwfilterDriverReload(void)
+{
+    if (!driverState)
         return -1;
-    }
 
     if (!driverState->privileged)
         return 0;
 
-    conn = virConnectOpen("qemu:///system");
-
-    if (conn) {
-        virNWFilterDHCPSnoopEnd(NULL);
-        /* shut down all threads -- they will be restarted if necessary */
-        virNWFilterLearnThreadsTerminate(true);
-
-        nwfilterDriverLock(driverState);
-        virNWFilterCallbackDriversLock();
+    virNWFilterDHCPSnoopEnd(NULL);
+    /* shut down all threads -- they will be restarted if necessary */
+    virNWFilterLearnThreadsTerminate(true);
 
-        virNWFilterLoadAllConfigs(conn,
-                                  &driverState->nwfilters,
-                                  driverState->configDir);
+    nwfilterDriverLock(driverState);
+    virNWFilterCallbackDriversLock();
 
-        virNWFilterCallbackDriversUnlock();
-        nwfilterDriverUnlock(driverState);
+    virNWFilterLoadAllConfigs(&driverState->nwfilters,
+                              driverState->configDir);
 
-        virNWFilterInstFiltersOnAllVMs(conn);
+    virNWFilterCallbackDriversUnlock();
+    nwfilterDriverUnlock(driverState);
 
-        virConnectClose(conn);
-    }
+    virNWFilterInstFiltersOnAllVMs();
 
     return 0;
 }
@@ -544,7 +534,7 @@ nwfilterDefine(virConnectPtr conn,
     if (!(def = virNWFilterDefParseString(xml)))
         goto cleanup;
 
-    if (!(nwfilter = virNWFilterObjAssignDef(conn, &driver->nwfilters, def)))
+    if (!(nwfilter = virNWFilterObjAssignDef(&driver->nwfilters, def)))
         goto cleanup;
 
     if (virNWFilterObjSaveDef(driver, nwfilter, def) < 0) {
@@ -585,7 +575,7 @@ nwfilterUndefine(virNWFilterPtr obj) {
         goto cleanup;
     }
 
-    if (virNWFilterTestUnassignDef(obj->conn, nwfilter) < 0) {
+    if (virNWFilterTestUnassignDef(nwfilter) < 0) {
         virReportError(VIR_ERR_OPERATION_INVALID,
                        "%s",
                        _("nwfilter is in use"));

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d4c90efd39059a87c57841f31268edf8840e0164..8a53a8bbd4fd23b3c6f3ea3365546bf2bf6aab6e 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -158,8 +158,7 @@ static void
 qemuVMDriverUnlock(void) {}
 
 static int
-qemuVMFilterRebuild(virConnectPtr conn ATTRIBUTE_UNUSED,
-                    virDomainObjListIterator iter, void *data)
+qemuVMFilterRebuild(virDomainObjListIterator iter, void *data)
 {
     return virDomainObjListForEach(qemu_driver->domains, iter, data);
 }

diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 63a807a9a6fa2f0536a1dc92225ac04898da4c4f..062dee3e4ba5cedb620e5907fb0ada56a3ec524a 100644 (file)
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -148,8 +148,7 @@ static int umlMonitorCommand(const struct uml_driver *driver,
 static struct uml_driver *uml_driver = NULL;
 
 static int
-umlVMFilterRebuild(virConnectPtr conn ATTRIBUTE_UNUSED,
-                   virDomainObjListIterator iter, void *data)
+umlVMFilterRebuild(virDomainObjListIterator iter, void *data)
 {
     return virDomainObjListForEach(uml_driver->domains, iter, data);
 }