43618df1d4
Fix nwfilter crash during firewalld install (bz #1014762) Fix crash with nographics (bz #1014088)
383 lines
14 KiB
Diff
383 lines
14 KiB
Diff
From 1766db28533e2b5a96792aa0811e5364e0bb54d4 Mon Sep 17 00:00:00 2001
|
|
From: "Daniel P. Berrange" <berrange@redhat.com>
|
|
Date: Thu, 3 Oct 2013 14:07:00 +0100
|
|
Subject: [PATCH] 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>
|
|
---
|
|
src/conf/nwfilter_conf.c | 49 ++++++++++++++++--------------------------
|
|
src/conf/nwfilter_conf.h | 14 +++++-------
|
|
src/lxc/lxc_driver.c | 3 +--
|
|
src/nwfilter/nwfilter_driver.c | 42 ++++++++++++++----------------------
|
|
src/qemu/qemu_driver.c | 3 +--
|
|
src/uml/uml_driver.c | 3 +--
|
|
6 files changed, 43 insertions(+), 71 deletions(-)
|
|
|
|
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
|
|
index 9927f7e..7152aae 100644
|
|
--- a/src/conf/nwfilter_conf.c
|
|
+++ b/src/conf/nwfilter_conf.c
|
|
@@ -2744,8 +2744,7 @@ cleanup:
|
|
|
|
|
|
static int
|
|
-_virNWFilterDefLoopDetect(virConnectPtr conn,
|
|
- virNWFilterObjListPtr nwfilters,
|
|
+_virNWFilterDefLoopDetect(virNWFilterObjListPtr nwfilters,
|
|
virNWFilterDefPtr def,
|
|
const char *filtername)
|
|
{
|
|
@@ -2769,7 +2768,7 @@ _virNWFilterDefLoopDetect(virConnectPtr conn,
|
|
obj = virNWFilterObjFindByName(nwfilters,
|
|
entry->include->filterref);
|
|
if (obj) {
|
|
- rc = _virNWFilterDefLoopDetect(conn, nwfilters,
|
|
+ rc = _virNWFilterDefLoopDetect(nwfilters,
|
|
obj->def, filtername);
|
|
|
|
virNWFilterObjUnlock(obj);
|
|
@@ -2785,7 +2784,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
|
|
*
|
|
@@ -2795,11 +2793,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;
|
|
@@ -2858,7 +2855,7 @@ static void *virNWFilterDomainFWUpdateOpaque;
|
|
* error. This should be called upon reloading of the driver.
|
|
*/
|
|
int
|
|
-virNWFilterInstFiltersOnAllVMs(virConnectPtr conn)
|
|
+virNWFilterInstFiltersOnAllVMs(void)
|
|
{
|
|
size_t i;
|
|
struct domUpdateCBStruct cb = {
|
|
@@ -2868,15 +2865,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)
|
|
{
|
|
size_t i;
|
|
int ret = 0;
|
|
@@ -2890,8 +2886,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;
|
|
}
|
|
@@ -2900,15 +2895,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);
|
|
}
|
|
|
|
@@ -2919,14 +2912,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;
|
|
@@ -2965,8 +2957,7 @@ cleanup:
|
|
}
|
|
|
|
virNWFilterObjPtr
|
|
-virNWFilterObjAssignDef(virConnectPtr conn,
|
|
- virNWFilterObjListPtr nwfilters,
|
|
+virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
|
|
virNWFilterDefPtr def)
|
|
{
|
|
virNWFilterObjPtr nwfilter;
|
|
@@ -2985,7 +2976,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;
|
|
@@ -3004,7 +2995,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);
|
|
@@ -3046,8 +3037,7 @@ virNWFilterObjAssignDef(virConnectPtr conn,
|
|
|
|
|
|
static virNWFilterObjPtr
|
|
-virNWFilterObjLoad(virConnectPtr conn,
|
|
- virNWFilterObjListPtr nwfilters,
|
|
+virNWFilterObjLoad(virNWFilterObjListPtr nwfilters,
|
|
const char *file,
|
|
const char *path)
|
|
{
|
|
@@ -3066,7 +3056,7 @@ virNWFilterObjLoad(virConnectPtr conn,
|
|
return NULL;
|
|
}
|
|
|
|
- if (!(nwfilter = virNWFilterObjAssignDef(conn, nwfilters, def))) {
|
|
+ if (!(nwfilter = virNWFilterObjAssignDef(nwfilters, def))) {
|
|
virNWFilterDefFree(def);
|
|
return NULL;
|
|
}
|
|
@@ -3082,8 +3072,7 @@ virNWFilterObjLoad(virConnectPtr conn,
|
|
|
|
|
|
int
|
|
-virNWFilterLoadAllConfigs(virConnectPtr conn,
|
|
- virNWFilterObjListPtr nwfilters,
|
|
+virNWFilterLoadAllConfigs(virNWFilterObjListPtr nwfilters,
|
|
const char *configDir)
|
|
{
|
|
DIR *dir;
|
|
@@ -3111,7 +3100,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 e470615..29906f1 100644
|
|
--- a/src/conf/nwfilter_conf.h
|
|
+++ b/src/conf/nwfilter_conf.h
|
|
@@ -687,12 +687,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);
|
|
@@ -706,8 +704,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,
|
|
@@ -725,11 +722,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 8b13f84..e3a34d6 100644
|
|
--- a/src/lxc/lxc_driver.c
|
|
+++ b/src/lxc/lxc_driver.c
|
|
@@ -84,8 +84,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 6e20e03..d25c6f2 100644
|
|
--- a/src/nwfilter/nwfilter_driver.c
|
|
+++ b/src/nwfilter/nwfilter_driver.c
|
|
@@ -235,8 +235,7 @@ nwfilterStateInitialize(bool privileged,
|
|
|
|
VIR_FREE(base);
|
|
|
|
- if (virNWFilterLoadAllConfigs(NULL,
|
|
- &driverState->nwfilters,
|
|
+ if (virNWFilterLoadAllConfigs(&driverState->nwfilters,
|
|
driverState->configDir) < 0)
|
|
goto error;
|
|
|
|
@@ -272,37 +271,28 @@ err_free_driverstate:
|
|
* files and update its state
|
|
*/
|
|
static int
|
|
-nwfilterStateReload(void) {
|
|
- virConnectPtr conn;
|
|
-
|
|
- if (!driverState) {
|
|
+nwfilterStateReload(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;
|
|
}
|
|
@@ -573,7 +563,7 @@ nwfilterDefineXML(virConnectPtr conn,
|
|
if (virNWFilterDefineXMLEnsureACL(conn, def) < 0)
|
|
goto cleanup;
|
|
|
|
- if (!(nwfilter = virNWFilterObjAssignDef(conn, &driver->nwfilters, def)))
|
|
+ if (!(nwfilter = virNWFilterObjAssignDef(&driver->nwfilters, def)))
|
|
goto cleanup;
|
|
|
|
if (virNWFilterObjSaveDef(driver, nwfilter, def) < 0) {
|
|
@@ -617,7 +607,7 @@ nwfilterUndefine(virNWFilterPtr obj) {
|
|
if (virNWFilterUndefineEnsureACL(obj->conn, nwfilter->def) < 0)
|
|
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 e8bc04d..068d29f 100644
|
|
--- a/src/qemu/qemu_driver.c
|
|
+++ b/src/qemu/qemu_driver.c
|
|
@@ -177,8 +177,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 9ca352f..eb02542 100644
|
|
--- 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);
|
|
}
|