External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-08-27 02:51:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove use of virConnectPtr from all remaining nwfilter code

Browse Source 
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>
This commit is contained in:
Daniel P. Berrange 2013-10-03 12:51:48 +01:00
parent ebca369e3f
commit 999d72fbd5
6 changed files with 43 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
src/conf/nwfilter_conf.c
View File

@ -2744,8 +2744,7 @@ cleanup:
static int static int
_virNWFilterDefLoopDetect(virConnectPtr conn, _virNWFilterDefLoopDetect(virNWFilterObjListPtr nwfilters,
virNWFilterObjListPtr nwfilters,
virNWFilterDefPtr def, virNWFilterDefPtr def,
const char *filtername) const char *filtername)
{ {
@ -2769,7 +2768,7 @@ _virNWFilterDefLoopDetect(virConnectPtr conn,
obj = virNWFilterObjFindByName(nwfilters, obj = virNWFilterObjFindByName(nwfilters,
entry->include->filterref); entry->include->filterref);
if (obj) { if (obj) {
rc = _virNWFilterDefLoopDetect(conn, nwfilters, rc = _virNWFilterDefLoopDetect(nwfilters,
obj->def, filtername); obj->def, filtername);
virNWFilterObjUnlock(obj); virNWFilterObjUnlock(obj);
@ -2785,7 +2784,6 @@ _virNWFilterDefLoopDetect(virConnectPtr conn,
/* /*
* virNWFilterDefLoopDetect: * virNWFilterDefLoopDetect:
* @conn: pointer to virConnect object
* @nwfilters : the nwfilters to search * @nwfilters : the nwfilters to search
* @def : the filter definition that may add a loop and is to be tested * @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. * Returns 0 in case no loop was detected, -1 otherwise.
*/ */
static int static int
virNWFilterDefLoopDetect(virConnectPtr conn, virNWFilterDefLoopDetect(virNWFilterObjListPtr nwfilters,
virNWFilterObjListPtr nwfilters,
virNWFilterDefPtr def) virNWFilterDefPtr def)
{ {
return _virNWFilterDefLoopDetect(conn, nwfilters, def, def->name); return _virNWFilterDefLoopDetect(nwfilters, def, def->name);
} }
int nCallbackDriver; int nCallbackDriver;
@ -2858,7 +2855,7 @@ static void *virNWFilterDomainFWUpdateOpaque;
* error. This should be called upon reloading of the driver. * error. This should be called upon reloading of the driver.
*/ */
int int
virNWFilterInstFiltersOnAllVMs(virConnectPtr conn) virNWFilterInstFiltersOnAllVMs(void)
{ {
size_t i; size_t i;
struct domUpdateCBStruct cb = { struct domUpdateCBStruct cb = {
@ -2868,15 +2865,14 @@ virNWFilterInstFiltersOnAllVMs(virConnectPtr conn)
}; };
for (i = 0; i < nCallbackDriver; i++) for (i = 0; i < nCallbackDriver; i++)
callbackDrvArray[i]->vmFilterRebuild(conn, callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB,
virNWFilterDomainFWUpdateCB,
&cb); &cb);
return 0; return 0;
} }
static int static int
virNWFilterTriggerVMFilterRebuild(virConnectPtr conn) virNWFilterTriggerVMFilterRebuild(void)
{ {
size_t i; size_t i;
int ret = 0; int ret = 0;
@ -2890,8 +2886,7 @@ virNWFilterTriggerVMFilterRebuild(virConnectPtr conn)
return -1; return -1;
for (i = 0; i < nCallbackDriver; i++) { for (i = 0; i < nCallbackDriver; i++) {
if (callbackDrvArray[i]->vmFilterRebuild(conn, if (callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB,
virNWFilterDomainFWUpdateCB,
&cb) < 0) &cb) < 0)
ret = -1; ret = -1;
} }
@ -2900,15 +2895,13 @@ virNWFilterTriggerVMFilterRebuild(virConnectPtr conn)
cb.step = STEP_TEAR_NEW; /* rollback */ cb.step = STEP_TEAR_NEW; /* rollback */
for (i = 0; i < nCallbackDriver; i++) for (i = 0; i < nCallbackDriver; i++)
callbackDrvArray[i]->vmFilterRebuild(conn, callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB,
virNWFilterDomainFWUpdateCB,
&cb); &cb);
} else { } else {
cb.step = STEP_TEAR_OLD; /* switch over */ cb.step = STEP_TEAR_OLD; /* switch over */
for (i = 0; i < nCallbackDriver; i++) for (i = 0; i < nCallbackDriver; i++)
callbackDrvArray[i]->vmFilterRebuild(conn, callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB,
virNWFilterDomainFWUpdateCB,
&cb); &cb);
} }
@ -2919,14 +2912,13 @@ virNWFilterTriggerVMFilterRebuild(virConnectPtr conn)
int int
virNWFilterTestUnassignDef(virConnectPtr conn, virNWFilterTestUnassignDef(virNWFilterObjPtr nwfilter)
virNWFilterObjPtr nwfilter)
{ {
int rc = 0; int rc = 0;
nwfilter->wantRemoved = 1; nwfilter->wantRemoved = 1;
/* trigger the update on VMs referencing the filter */ /* trigger the update on VMs referencing the filter */
if (virNWFilterTriggerVMFilterRebuild(conn)) if (virNWFilterTriggerVMFilterRebuild())
rc = -1; rc = -1;
nwfilter->wantRemoved = 0; nwfilter->wantRemoved = 0;
@ -2965,8 +2957,7 @@ cleanup:
} }
virNWFilterObjPtr virNWFilterObjPtr
virNWFilterObjAssignDef(virConnectPtr conn, virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
virNWFilterObjListPtr nwfilters,
virNWFilterDefPtr def) virNWFilterDefPtr def)
{ {
virNWFilterObjPtr nwfilter; virNWFilterObjPtr nwfilter;
@ -2985,7 +2976,7 @@ virNWFilterObjAssignDef(virConnectPtr conn,
virNWFilterObjUnlock(nwfilter); virNWFilterObjUnlock(nwfilter);
} }
if (virNWFilterDefLoopDetect(conn, nwfilters, def) < 0) { if (virNWFilterDefLoopDetect(nwfilters, def) < 0) {
virReportError(VIR_ERR_OPERATION_FAILED, virReportError(VIR_ERR_OPERATION_FAILED,
"%s", _("filter would introduce a loop")); "%s", _("filter would introduce a loop"));
return NULL; return NULL;
@ -3004,7 +2995,7 @@ virNWFilterObjAssignDef(virConnectPtr conn,
nwfilter->newDef = def; nwfilter->newDef = def;
/* trigger the update on VMs referencing the filter */ /* trigger the update on VMs referencing the filter */
if (virNWFilterTriggerVMFilterRebuild(conn)) { if (virNWFilterTriggerVMFilterRebuild()) {
nwfilter->newDef = NULL; nwfilter->newDef = NULL;
virNWFilterUnlockFilterUpdates(); virNWFilterUnlockFilterUpdates();
virNWFilterObjUnlock(nwfilter); virNWFilterObjUnlock(nwfilter);
@ -3046,8 +3037,7 @@ virNWFilterObjAssignDef(virConnectPtr conn,
static virNWFilterObjPtr static virNWFilterObjPtr
virNWFilterObjLoad(virConnectPtr conn, virNWFilterObjLoad(virNWFilterObjListPtr nwfilters,
virNWFilterObjListPtr nwfilters,
const char *file, const char *file,
const char *path) const char *path)
{ {
@ -3066,7 +3056,7 @@ virNWFilterObjLoad(virConnectPtr conn,
return NULL; return NULL;
} }
if (!(nwfilter = virNWFilterObjAssignDef(conn, nwfilters, def))) { if (!(nwfilter = virNWFilterObjAssignDef(nwfilters, def))) {
virNWFilterDefFree(def); virNWFilterDefFree(def);
return NULL; return NULL;
} }
@ -3082,8 +3072,7 @@ virNWFilterObjLoad(virConnectPtr conn,
int int
virNWFilterLoadAllConfigs(virConnectPtr conn, virNWFilterLoadAllConfigs(virNWFilterObjListPtr nwfilters,
virNWFilterObjListPtr nwfilters,
const char *configDir) const char *configDir)
{ {
DIR *dir; DIR *dir;
@ -3111,7 +3100,7 @@ virNWFilterLoadAllConfigs(virConnectPtr conn,
if (!(path = virFileBuildPath(configDir, entry->d_name, NULL))) if (!(path = virFileBuildPath(configDir, entry->d_name, NULL)))
continue; continue;
nwfilter = virNWFilterObjLoad(conn, nwfilters, entry->d_name, path); nwfilter = virNWFilterObjLoad(nwfilters, entry->d_name, path);
if (nwfilter) if (nwfilter)
virNWFilterObjUnlock(nwfilter); virNWFilterObjUnlock(nwfilter);

14
src/conf/nwfilter_conf.h
View File

@ -687,12 +687,10 @@ int virNWFilterObjSaveDef(virNWFilterDriverStatePtr driver,
int virNWFilterObjDeleteDef(virNWFilterObjPtr nwfilter); int virNWFilterObjDeleteDef(virNWFilterObjPtr nwfilter);
virNWFilterObjPtr virNWFilterObjAssignDef(virConnectPtr conn, virNWFilterObjPtr virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
virNWFilterObjListPtr nwfilters,
virNWFilterDefPtr def); virNWFilterDefPtr def);
int virNWFilterTestUnassignDef(virConnectPtr conn, int virNWFilterTestUnassignDef(virNWFilterObjPtr nwfilter);
virNWFilterObjPtr nwfilter);
virNWFilterDefPtr virNWFilterDefParseNode(xmlDocPtr xml, virNWFilterDefPtr virNWFilterDefParseNode(xmlDocPtr xml,
xmlNodePtr root); xmlNodePtr root);
@ -706,8 +704,7 @@ int virNWFilterSaveXML(const char *configDir,
int virNWFilterSaveConfig(const char *configDir, int virNWFilterSaveConfig(const char *configDir,
virNWFilterDefPtr def); virNWFilterDefPtr def);
int virNWFilterLoadAllConfigs(virConnectPtr conn, int virNWFilterLoadAllConfigs(virNWFilterObjListPtr nwfilters,
virNWFilterObjListPtr nwfilters,
const char *configDir); const char *configDir);
char *virNWFilterConfigFile(const char *dir, char *virNWFilterConfigFile(const char *dir,
@ -725,11 +722,10 @@ void virNWFilterUnlockFilterUpdates(void);
int virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB, void *opaque); int virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB, void *opaque);
void virNWFilterConfLayerShutdown(void); void virNWFilterConfLayerShutdown(void);
int virNWFilterInstFiltersOnAllVMs(virConnectPtr conn); int virNWFilterInstFiltersOnAllVMs(void);
typedef int (*virNWFilterRebuild)(virConnectPtr conn, typedef int (*virNWFilterRebuild)(virDomainObjListIterator domUpdateCB,
virDomainObjListIterator domUpdateCB,
void *data); void *data);
typedef void (*virNWFilterVoidCall)(void); typedef void (*virNWFilterVoidCall)(void);

3
src/lxc/lxc_driver.c
View File

@ -84,8 +84,7 @@ virLXCDriverPtr lxc_driver = NULL;
/* callbacks for nwfilter */ /* callbacks for nwfilter */
static int static int
lxcVMFilterRebuild(virConnectPtr conn ATTRIBUTE_UNUSED, lxcVMFilterRebuild(virDomainObjListIterator iter, void *data)
virDomainObjListIterator iter, void *data)
{ {
return virDomainObjListForEach(lxc_driver->domains, iter, data); return virDomainObjListForEach(lxc_driver->domains, iter, data);
} }

26
src/nwfilter/nwfilter_driver.c
View File

@ -235,8 +235,7 @@ nwfilterStateInitialize(bool privileged,
VIR_FREE(base); VIR_FREE(base);
if (virNWFilterLoadAllConfigs(NULL, if (virNWFilterLoadAllConfigs(&driverState->nwfilters,
&driverState->nwfilters,
driverState->configDir) < 0) driverState->configDir) < 0)
goto error; goto error;
@ -272,19 +271,14 @@ err_free_driverstate:
* files and update its state * files and update its state
*/ */
static int static int
nwfilterStateReload(void) { nwfilterStateReload(void)
virConnectPtr conn; {
if (!driverState)
if (!driverState) {
return -1; return -1;
}
if (!driverState->privileged) if (!driverState->privileged)
return 0; return 0;
conn = virConnectOpen("qemu:///system");
if (conn) {
virNWFilterDHCPSnoopEnd(NULL); virNWFilterDHCPSnoopEnd(NULL);
/* shut down all threads -- they will be restarted if necessary */ /* shut down all threads -- they will be restarted if necessary */
virNWFilterLearnThreadsTerminate(true); virNWFilterLearnThreadsTerminate(true);
@ -292,17 +286,13 @@ nwfilterStateReload(void) {
nwfilterDriverLock(driverState); nwfilterDriverLock(driverState);
virNWFilterCallbackDriversLock(); virNWFilterCallbackDriversLock();
virNWFilterLoadAllConfigs(conn, virNWFilterLoadAllConfigs(&driverState->nwfilters,
&driverState->nwfilters,
driverState->configDir); driverState->configDir);
virNWFilterCallbackDriversUnlock(); virNWFilterCallbackDriversUnlock();
nwfilterDriverUnlock(driverState); nwfilterDriverUnlock(driverState);
virNWFilterInstFiltersOnAllVMs(conn); virNWFilterInstFiltersOnAllVMs();
virConnectClose(conn);
}
return 0; return 0;
} }
@ -573,7 +563,7 @@ nwfilterDefineXML(virConnectPtr conn,
if (virNWFilterDefineXMLEnsureACL(conn, def) < 0) if (virNWFilterDefineXMLEnsureACL(conn, def) < 0)
goto cleanup; goto cleanup;
if (!(nwfilter = virNWFilterObjAssignDef(conn, &driver->nwfilters, def))) if (!(nwfilter = virNWFilterObjAssignDef(&driver->nwfilters, def)))
goto cleanup; goto cleanup;
if (virNWFilterObjSaveDef(driver, nwfilter, def) < 0) { if (virNWFilterObjSaveDef(driver, nwfilter, def) < 0) {
@ -617,7 +607,7 @@ nwfilterUndefine(virNWFilterPtr obj) {
if (virNWFilterUndefineEnsureACL(obj->conn, nwfilter->def) < 0) if (virNWFilterUndefineEnsureACL(obj->conn, nwfilter->def) < 0)
goto cleanup; goto cleanup;
if (virNWFilterTestUnassignDef(obj->conn, nwfilter) < 0) { if (virNWFilterTestUnassignDef(nwfilter) < 0) {
virReportError(VIR_ERR_OPERATION_INVALID, virReportError(VIR_ERR_OPERATION_INVALID,
"%s", "%s",
_("nwfilter is in use")); _("nwfilter is in use"));

3
src/qemu/qemu_driver.c
View File

@ -177,8 +177,7 @@ static void
qemuVMDriverUnlock(void) {} qemuVMDriverUnlock(void) {}
static int static int
qemuVMFilterRebuild(virConnectPtr conn ATTRIBUTE_UNUSED, qemuVMFilterRebuild(virDomainObjListIterator iter, void *data)
virDomainObjListIterator iter, void *data)
{ {
return virDomainObjListForEach(qemu_driver->domains, iter, data); return virDomainObjListForEach(qemu_driver->domains, iter, data);
} }

3
src/uml/uml_driver.c
View File

@ -148,8 +148,7 @@ static int umlMonitorCommand(const struct uml_driver *driver,
static struct uml_driver *uml_driver = NULL; static struct uml_driver *uml_driver = NULL;
static int static int
umlVMFilterRebuild(virConnectPtr conn ATTRIBUTE_UNUSED, umlVMFilterRebuild(virDomainObjListIterator iter, void *data)
virDomainObjListIterator iter, void *data)
{ {
return virDomainObjListForEach(uml_driver->domains, iter, data); return virDomainObjListForEach(uml_driver->domains, iter, data);
} }