[libvirt] [PATCH v4 0/6] nwfilter: refactor the driver to make it independent of virt drivers
v1: https://www.redhat.com/archives/libvir-list/2018-April/msg02616.html v2: https://www.redhat.com/archives/libvir-list/2018-May/msg01145.html v3: https://www.redhat.com/archives/libvir-list/2018-June/msg01121.html Today the nwfilter driver is entangled with the virt drivers in both directions. At various times when rebuilding filters nwfilter will call out to the virt driver to iterate over running guest's NICs. This has caused very complicated lock ordering rules to be required. If we are to split the virt drivers out into separate daemons we need to get rid of this coupling since we don't want the separate daemons calling each other, as that risks deadlock if all of the RPC workers are busy. The obvious way to solve this is to have the nwfilter driver remember all the filters it has active, avoiding the need to iterate over running guests. Changed in v4: - Merged already reviewed patches - Correctly handle nwfilter recreate on daemon startup - Added virsh docs Changed in v3: - Updated API version numbers - Use accessors for virNWFilterBindingObjPtr struct - Other fixes John mentioned Changed in v2: - The virNWFilterBindingPtr was renamed virNWFilterBindingDefPtr - New virNWFilterBindingObjPtr & virNWFilterBindingObjListPtr structs added to track the objects in the driver - New virNWFilterBindingPtr public API type was added - New public APIs for listing filter bindings, querying XML, and creating/deleting them - Convert the virt drivers to use the public API for creating and deleting bindings - Persistent active bindings out to disk so they're preserved across restarts - Added RNG schema and XML-2-XML test - New virsh commands for listing/querying XML/creating/deleting bindings Daniel P. Berrangé (6): virsh: add manpage docs for nwfilter-binding commands. nwfilter: keep track of active filter bindings nwfilter: remove virt driver callback layer for rebuilding filters nwfilter: wire up new APIs for listing and querying filter bindings nwfilter: wire up new APIs for creating and deleting nwfilter bindings nwfilter: convert virt drivers to use public API for nwfilter bindings src/conf/domain_nwfilter.c | 135 ++++++++++++-- src/conf/domain_nwfilter.h | 16 +- src/conf/nwfilter_conf.c | 136 ++------------ src/conf/nwfilter_conf.h | 51 +----- src/conf/virnwfilterobj.c | 4 +- src/conf/virnwfilterobj.h | 4 + src/libvirt_private.syms | 8 +- src/lxc/lxc_driver.c | 28 --- src/lxc/lxc_process.c | 2 +- src/nwfilter/nwfilter_driver.c | 236 ++++++++++++++++++++----- src/nwfilter/nwfilter_gentech_driver.c | 187 ++++++++++---------- src/nwfilter/nwfilter_gentech_driver.h | 8 +- src/qemu/qemu_driver.c | 25 --- src/qemu/qemu_hotplug.c | 4 +- src/qemu/qemu_interface.c | 4 +- src/qemu/qemu_process.c | 6 +- src/remote/remote_daemon.c | 1 + src/uml/uml_conf.c | 2 +- src/uml/uml_driver.c | 29 --- tools/virsh.pod | 35 ++++ 20 files changed, 478 insertions(+), 443 deletions(-) -- 2.17.1
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tools/virsh.pod | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tools/virsh.pod b/tools/virsh.pod index c9ef4f137c..47985ebf78 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -4807,6 +4807,41 @@ variables, and defaults to C<vi>. =back +=head1 NWFILTER BINDING COMMANDS + +The following commands manipulate network filter bindings. Network filter +bindings track the association between a network port and a network +filter. Generally the bindings are managed automatically by the hypervisor +drivers when adding/removing NICs on a guest. + +If an admin is creating/deleting TAP devices for non-guest usage, +however, the network filter binding commands provide a way to make use +of the network filters directly. + +=over 4 + +=item B<nwfilter-binding-create> I<xmlfile> + +Associate a network port with a network filter. The network filter backend +will immediately attempt to instantiate the filter rules on the port. + +=item B<nwfilter-binding-undefine> I<port-name> + +Disassociate a network port from a network filter. The network filter +backend will immediately tear down the filter rules that exist on the +port. + +=item B<nwfilter-binding-list> + +List all of the network ports which have filters associated with them + +=item B<nwfilter-binding-dumpxml> I<port-name> + +Output the network filter binding XML for the network device called +C<port-name> + +=back + =head1 HYPERVISOR-SPECIFIC COMMANDS NOTE: Use of the following commands is B<strongly> discouraged. They -- 2.17.1
On 06/26/2018 06:23 AM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tools/virsh.pod | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
Just need to add periods to the end of a couple of sentences below Reviewed-by: John Ferlan <jferlan@redhat.com> John
diff --git a/tools/virsh.pod b/tools/virsh.pod index c9ef4f137c..47985ebf78 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -4807,6 +4807,41 @@ variables, and defaults to C<vi>.
=back
+=head1 NWFILTER BINDING COMMANDS + +The following commands manipulate network filter bindings. Network filter +bindings track the association between a network port and a network +filter. Generally the bindings are managed automatically by the hypervisor +drivers when adding/removing NICs on a guest. + +If an admin is creating/deleting TAP devices for non-guest usage, +however, the network filter binding commands provide a way to make use +of the network filters directly. + +=over 4 + +=item B<nwfilter-binding-create> I<xmlfile> + +Associate a network port with a network filter. The network filter backend +will immediately attempt to instantiate the filter rules on the port. + +=item B<nwfilter-binding-undefine> I<port-name> + +Disassociate a network port from a network filter. The network filter +backend will immediately tear down the filter rules that exist on the +port. + +=item B<nwfilter-binding-list> + +List all of the network ports which have filters associated with them
s/them/them.
+ +=item B<nwfilter-binding-dumpxml> I<port-name> + +Output the network filter binding XML for the network device called +C<port-name>
s/>/>.
+ +=back + =head1 HYPERVISOR-SPECIFIC COMMANDS
NOTE: Use of the following commands is B<strongly> discouraged. They
Currently the nwfilter driver does not keep any record of what filter bindings it has active. This means that when it needs to recreate filters, it has to rely on triggering callbacks provided by the virt drivers. This introduces a hash table recording the virNWFilterBinding objects so the driver has a record of all active filters. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/virnwfilterobj.h | 4 ++ src/nwfilter/nwfilter_driver.c | 84 ++++++++++++++++++++++++---------- 2 files changed, 63 insertions(+), 25 deletions(-) diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index 433b0402d0..4a54dd50da 100644 --- a/src/conf/virnwfilterobj.h +++ b/src/conf/virnwfilterobj.h @@ -22,6 +22,7 @@ # include "internal.h" # include "nwfilter_conf.h" +# include "virnwfilterbindingobjlist.h" typedef struct _virNWFilterObj virNWFilterObj; typedef virNWFilterObj *virNWFilterObjPtr; @@ -37,7 +38,10 @@ struct _virNWFilterDriverState { virNWFilterObjListPtr nwfilters; + virNWFilterBindingObjListPtr bindings; + char *configDir; + char *bindingDir; }; virNWFilterDefPtr diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 7202691646..1449b67c72 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -38,7 +38,6 @@ #include "domain_conf.h" #include "domain_nwfilter.h" #include "nwfilter_driver.h" -#include "virnwfilterbindingdef.h" #include "nwfilter_gentech_driver.h" #include "configmake.h" #include "virfile.h" @@ -174,7 +173,6 @@ nwfilterStateInitialize(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { - char *base = NULL; DBusConnection *sysbus = NULL; if (virDBusHasSystemBus() && @@ -191,6 +189,9 @@ nwfilterStateInitialize(bool privileged, if (!(driver->nwfilters = virNWFilterObjListNew())) goto error; + if (!(driver->bindings = virNWFilterBindingObjListNew())) + goto error; + if (!privileged) return 0; @@ -230,30 +231,35 @@ nwfilterStateInitialize(bool privileged, goto error; } - if (VIR_STRDUP(base, SYSCONFDIR "/libvirt") < 0) + if (VIR_STRDUP(driver->configDir, SYSCONFDIR "/libvirt/nwfilter") < 0) goto error; - if (virAsprintf(&driver->configDir, - "%s/nwfilter", base) == -1) + if (virFileMakePathWithMode(driver->configDir, S_IRWXU) < 0) { + virReportSystemError(errno, _("cannot create config directory '%s'"), + driver->configDir); goto error; + } - VIR_FREE(base); + if (VIR_STRDUP(driver->bindingDir, LOCALSTATEDIR "/run/libvirt/nwfilter-binding") < 0) + goto error; - if (virFileMakePathWithMode(driver->configDir, S_IRWXU) < 0) { + if (virFileMakePathWithMode(driver->bindingDir, S_IRWXU) < 0) { virReportSystemError(errno, _("cannot create config directory '%s'"), - driver->configDir); + driver->bindingDir); goto error; } if (virNWFilterObjListLoadAllConfigs(driver->nwfilters, driver->configDir) < 0) goto error; + if (virNWFilterBindingObjListLoadAllConfigs(driver->bindings, driver->bindingDir) < 0) + goto error; + nwfilterDriverUnlock(); return 0; error: - VIR_FREE(base); nwfilterDriverUnlock(); nwfilterStateCleanup(); @@ -333,9 +339,12 @@ nwfilterStateCleanup(void) nwfilterDriverRemoveDBusMatches(); VIR_FREE(driver->configDir); + VIR_FREE(driver->bindingDir); nwfilterDriverUnlock(); } + virObjectUnref(driver->bindings); + /* free inactive nwfilters */ virNWFilterObjListFree(driver->nwfilters); @@ -647,13 +656,35 @@ nwfilterInstantiateFilter(const char *vmname, const unsigned char *vmuuid, virDomainNetDefPtr net) { - virNWFilterBindingDefPtr binding; + virNWFilterBindingObjPtr obj; + virNWFilterBindingDefPtr def; int ret; - if (!(binding = virNWFilterBindingDefForNet(vmname, vmuuid, net))) + obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, net->ifname); + if (obj) { + virNWFilterBindingObjEndAPI(&obj); + return 0; + } + + if (!(def = virNWFilterBindingDefForNet(vmname, vmuuid, net))) + return -1; + + obj = virNWFilterBindingObjListAdd(driver->bindings, + def); + if (!obj) { + virNWFilterBindingDefFree(def); return -1; - ret = virNWFilterInstantiateFilter(driver, binding); - virNWFilterBindingDefFree(binding); + } + + ret = virNWFilterInstantiateFilter(driver, def); + + if (ret >= 0) + virNWFilterBindingObjSave(obj, driver->bindingDir); + else + virNWFilterBindingObjListRemove(driver->bindings, obj); + + virNWFilterBindingObjEndAPI(&obj); + return ret; } @@ -661,18 +692,21 @@ nwfilterInstantiateFilter(const char *vmname, static void nwfilterTeardownFilter(virDomainNetDefPtr net) { - virNWFilterBindingDef binding = { - .portdevname = net->ifname, - .linkdevname = (net->type == VIR_DOMAIN_NET_TYPE_DIRECT ? - net->data.direct.linkdev : NULL), - .mac = net->mac, - .filter = net->filter, - .filterparams = net->filterparams, - .ownername = NULL, - .owneruuid = {0}, - }; - if ((net->ifname) && (net->filter)) - virNWFilterTeardownFilter(&binding); + virNWFilterBindingObjPtr obj; + virNWFilterBindingDefPtr def; + if (!net->ifname) + return; + + obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, net->ifname); + if (!obj) + return; + + def = virNWFilterBindingObjGetDef(obj); + virNWFilterTeardownFilter(def); + virNWFilterBindingObjDelete(obj, driver->bindingDir); + + virNWFilterBindingObjListRemove(driver->bindings, obj); + virNWFilterBindingObjEndAPI(&obj); } -- 2.17.1
On 06/26/2018 06:23 AM, Daniel P. Berrangé wrote:
Currently the nwfilter driver does not keep any record of what filter bindings it has active. This means that when it needs to recreate filters, it has to rely on triggering callbacks provided by the virt drivers. This introduces a hash table recording the virNWFilterBinding objects so the driver has a record of all active filters.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/virnwfilterobj.h | 4 ++ src/nwfilter/nwfilter_driver.c | 84 ++++++++++++++++++++++++---------- 2 files changed, 63 insertions(+), 25 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John BTW: I'm just running the avocado test again for final patch 6/6 checks.
Now that the nwfilter driver keeps a list of bindings that it has created, there is no need for the complex virt driver callbacks. It is possible to simply iterate of the list of recorded filter bindings. This means that rebuilding filters no longer has to acquire any locks on the virDomainObj objects, as they're never touched. Reviewed-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/nwfilter_conf.c | 136 +++----------------- src/conf/nwfilter_conf.h | 51 +------- src/conf/virnwfilterobj.c | 4 +- src/libvirt_private.syms | 7 +- src/lxc/lxc_driver.c | 28 ----- src/nwfilter/nwfilter_driver.c | 22 ++-- src/nwfilter/nwfilter_gentech_driver.c | 167 ++++++++++++++++--------- src/nwfilter/nwfilter_gentech_driver.h | 4 +- src/qemu/qemu_driver.c | 25 ---- src/uml/uml_driver.c | 29 ----- 10 files changed, 144 insertions(+), 329 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index de26a6d034..706e803a25 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2819,121 +2819,6 @@ virNWFilterSaveConfig(const char *configDir, } -int nCallbackDriver; -#define MAX_CALLBACK_DRIVER 10 -static virNWFilterCallbackDriverPtr callbackDrvArray[MAX_CALLBACK_DRIVER]; - -void -virNWFilterRegisterCallbackDriver(virNWFilterCallbackDriverPtr cbd) -{ - if (nCallbackDriver < MAX_CALLBACK_DRIVER) - callbackDrvArray[nCallbackDriver++] = cbd; -} - - -void -virNWFilterUnRegisterCallbackDriver(virNWFilterCallbackDriverPtr cbd) -{ - size_t i = 0; - - while (i < nCallbackDriver && callbackDrvArray[i] != cbd) - i++; - - if (i < nCallbackDriver) { - memmove(&callbackDrvArray[i], &callbackDrvArray[i+1], - (nCallbackDriver - i - 1) * sizeof(callbackDrvArray[i])); - callbackDrvArray[i] = 0; - nCallbackDriver--; - } -} - - -void -virNWFilterCallbackDriversLock(void) -{ - size_t i; - - for (i = 0; i < nCallbackDriver; i++) - callbackDrvArray[i]->vmDriverLock(); -} - - -void -virNWFilterCallbackDriversUnlock(void) -{ - size_t i; - - for (i = 0; i < nCallbackDriver; i++) - callbackDrvArray[i]->vmDriverUnlock(); -} - - -static virDomainObjListIterator virNWFilterDomainFWUpdateCB; -static void *virNWFilterDomainFWUpdateOpaque; - -/** - * virNWFilterInstFiltersOnAllVMs: - * Apply all filters on all running VMs. Don't terminate in case of an - * error. This should be called upon reloading of the driver. - */ -int -virNWFilterInstFiltersOnAllVMs(void) -{ - size_t i; - struct domUpdateCBStruct cb = { - .opaque = virNWFilterDomainFWUpdateOpaque, - .step = STEP_APPLY_CURRENT, - .skipInterfaces = NULL, /* not needed */ - }; - - for (i = 0; i < nCallbackDriver; i++) - callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB, - &cb); - - return 0; -} - - -int -virNWFilterTriggerVMFilterRebuild(void) -{ - size_t i; - int ret = 0; - struct domUpdateCBStruct cb = { - .opaque = virNWFilterDomainFWUpdateOpaque, - .step = STEP_APPLY_NEW, - .skipInterfaces = virHashCreate(0, NULL), - }; - - if (!cb.skipInterfaces) - return -1; - - for (i = 0; i < nCallbackDriver; i++) { - if (callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB, - &cb) < 0) - ret = -1; - } - - if (ret < 0) { - cb.step = STEP_TEAR_NEW; /* rollback */ - - for (i = 0; i < nCallbackDriver; i++) - callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB, - &cb); - } else { - cb.step = STEP_TEAR_OLD; /* switch over */ - - for (i = 0; i < nCallbackDriver; i++) - callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB, - &cb); - } - - virHashFree(cb.skipInterfaces); - - return ret; -} - - int virNWFilterDeleteDef(const char *configDir, virNWFilterDefPtr def) @@ -3204,16 +3089,18 @@ virNWFilterDefFormat(const virNWFilterDef *def) return NULL; } +static virNWFilterTriggerRebuildCallback rebuildCallback; +static void *rebuildOpaque; int -virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB, +virNWFilterConfLayerInit(virNWFilterTriggerRebuildCallback cb, void *opaque) { if (initialized) return -1; - virNWFilterDomainFWUpdateCB = domUpdateCB; - virNWFilterDomainFWUpdateOpaque = opaque; + rebuildCallback = cb; + rebuildOpaque = opaque; initialized = true; @@ -3233,8 +3120,17 @@ virNWFilterConfLayerShutdown(void) virRWLockDestroy(&updateLock); initialized = false; - virNWFilterDomainFWUpdateOpaque = NULL; - virNWFilterDomainFWUpdateCB = NULL; + rebuildCallback = NULL; + rebuildOpaque = NULL; +} + + +int +virNWFilterTriggerRebuild(void) +{ + if (rebuildCallback) + return rebuildCallback(rebuildOpaque); + return 0; } diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h index 08fc07c55c..9f8ad51bf2 100644 --- a/src/conf/nwfilter_conf.h +++ b/src/conf/nwfilter_conf.h @@ -546,20 +546,6 @@ struct _virNWFilterDef { }; -typedef enum { - STEP_APPLY_NEW, - STEP_TEAR_NEW, - STEP_TEAR_OLD, - STEP_APPLY_CURRENT, -} UpdateStep; - -struct domUpdateCBStruct { - void *opaque; - UpdateStep step; - virHashTablePtr skipInterfaces; -}; - - void virNWFilterRuleDefFree(virNWFilterRuleDefPtr def); @@ -567,7 +553,7 @@ void virNWFilterDefFree(virNWFilterDefPtr def); int -virNWFilterTriggerVMFilterRebuild(void); +virNWFilterTriggerRebuild(void); int virNWFilterDeleteDef(const char *configDir, @@ -599,44 +585,15 @@ virNWFilterReadLockFilterUpdates(void); void virNWFilterUnlockFilterUpdates(void); +typedef int (*virNWFilterTriggerRebuildCallback)(void *opaque); + int -virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB, +virNWFilterConfLayerInit(virNWFilterTriggerRebuildCallback cb, void *opaque); void virNWFilterConfLayerShutdown(void); -int -virNWFilterInstFiltersOnAllVMs(void); - -typedef int -(*virNWFilterRebuild)(virDomainObjListIterator domUpdateCB, - void *data); - -typedef void -(*virNWFilterVoidCall)(void); - -typedef struct _virNWFilterCallbackDriver virNWFilterCallbackDriver; -typedef virNWFilterCallbackDriver *virNWFilterCallbackDriverPtr; -struct _virNWFilterCallbackDriver { - const char *name; - - virNWFilterRebuild vmFilterRebuild; - virNWFilterVoidCall vmDriverLock; - virNWFilterVoidCall vmDriverUnlock; -}; - -void -virNWFilterRegisterCallbackDriver(virNWFilterCallbackDriverPtr); - -void -virNWFilterUnRegisterCallbackDriver(virNWFilterCallbackDriverPtr); - -void -virNWFilterCallbackDriversLock(void); - -void -virNWFilterCallbackDriversUnlock(void); char * virNWFilterPrintTCPFlags(uint8_t flags); diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 87d7e72703..0136a0d56c 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -276,7 +276,7 @@ virNWFilterObjTestUnassignDef(virNWFilterObjPtr obj) obj->wantRemoved = true; /* trigger the update on VMs referencing the filter */ - if (virNWFilterTriggerVMFilterRebuild() < 0) + if (virNWFilterTriggerRebuild() < 0) rc = -1; obj->wantRemoved = false; @@ -358,7 +358,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters, obj->newDef = def; /* trigger the update on VMs referencing the filter */ - if (virNWFilterTriggerVMFilterRebuild() < 0) { + if (virNWFilterTriggerRebuild() < 0) { obj->newDef = NULL; virNWFilterObjUnlock(obj); return NULL; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 427c53eae4..42547e64ed 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -787,8 +787,6 @@ virDomainNumatuneSpecifiedMaxNode; # conf/nwfilter_conf.h -virNWFilterCallbackDriversLock; -virNWFilterCallbackDriversUnlock; virNWFilterChainSuffixTypeToString; virNWFilterConfLayerInit; virNWFilterConfLayerShutdown; @@ -797,12 +795,10 @@ virNWFilterDefFree; virNWFilterDefParseFile; virNWFilterDefParseString; virNWFilterDeleteDef; -virNWFilterInstFiltersOnAllVMs; virNWFilterJumpTargetTypeToString; virNWFilterPrintStateMatchFlags; virNWFilterPrintTCPFlags; virNWFilterReadLockFilterUpdates; -virNWFilterRegisterCallbackDriver; virNWFilterRuleActionTypeToString; virNWFilterRuleDirectionTypeToString; virNWFilterRuleIsProtocolEthernet; @@ -810,9 +806,8 @@ virNWFilterRuleIsProtocolIPv4; virNWFilterRuleIsProtocolIPv6; virNWFilterRuleProtocolTypeToString; virNWFilterSaveConfig; -virNWFilterTriggerVMFilterRebuild; +virNWFilterTriggerRebuild; virNWFilterUnlockFilterUpdates; -virNWFilterUnRegisterCallbackDriver; virNWFilterWriteLockFilterUpdates; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index cfb431488d..bde0ff6ad4 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -66,7 +66,6 @@ #include "virfdstream.h" #include "domain_audit.h" #include "domain_nwfilter.h" -#include "nwfilter_conf.h" #include "virinitctl.h" #include "virnetdev.h" #include "virnetdevtap.h" @@ -95,31 +94,6 @@ static int lxcStateInitialize(bool privileged, static int lxcStateCleanup(void); virLXCDriverPtr lxc_driver = NULL; -/* callbacks for nwfilter */ -static int -lxcVMFilterRebuild(virDomainObjListIterator iter, void *data) -{ - return virDomainObjListForEach(lxc_driver->domains, iter, data); -} - -static void -lxcVMDriverLock(void) -{ - lxcDriverLock(lxc_driver); -} - -static void -lxcVMDriverUnlock(void) -{ - lxcDriverUnlock(lxc_driver); -} - -static virNWFilterCallbackDriver lxcCallbackDriver = { - .name = "LXC", - .vmFilterRebuild = lxcVMFilterRebuild, - .vmDriverLock = lxcVMDriverLock, - .vmDriverUnlock = lxcVMDriverUnlock, -}; /** * lxcDomObjFromDomain: @@ -1672,7 +1646,6 @@ static int lxcStateInitialize(bool privileged, NULL, NULL) < 0) goto cleanup; - virNWFilterRegisterCallbackDriver(&lxcCallbackDriver); virObjectUnref(caps); return 0; @@ -1744,7 +1717,6 @@ static int lxcStateCleanup(void) if (lxc_driver == NULL) return -1; - virNWFilterUnRegisterCallbackDriver(&lxcCallbackDriver); virObjectUnref(lxc_driver->domains); virObjectUnref(lxc_driver->domainEventState); diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 1449b67c72..e49e0e7406 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -163,6 +163,15 @@ nwfilterDriverInstallDBusMatches(DBusConnection *sysbus ATTRIBUTE_UNUSED) #endif /* HAVE_FIREWALLD */ +static int +virNWFilterTriggerRebuildImpl(void *opaque) +{ + virNWFilterDriverStatePtr nwdriver = opaque; + + return virNWFilterBuildAll(nwdriver, true); +} + + /** * nwfilterStateInitialize: * @@ -207,7 +216,7 @@ nwfilterStateInitialize(bool privileged, if (virNWFilterTechDriversInit(privileged) < 0) goto err_dhcpsnoop_shutdown; - if (virNWFilterConfLayerInit(virNWFilterDomainFWUpdateCB, + if (virNWFilterConfLayerInit(virNWFilterTriggerRebuildImpl, driver) < 0) goto err_techdrivers_shutdown; @@ -302,15 +311,14 @@ nwfilterStateReload(void) nwfilterDriverLock(); virNWFilterWriteLockFilterUpdates(); - virNWFilterCallbackDriversLock(); virNWFilterObjListLoadAllConfigs(driver->nwfilters, driver->configDir); - virNWFilterCallbackDriversUnlock(); virNWFilterUnlockFilterUpdates(); - nwfilterDriverUnlock(); - virNWFilterInstFiltersOnAllVMs(); + virNWFilterBuildAll(driver, false); + + nwfilterDriverUnlock(); return 0; } @@ -547,7 +555,6 @@ nwfilterDefineXML(virConnectPtr conn, nwfilterDriverLock(); virNWFilterWriteLockFilterUpdates(); - virNWFilterCallbackDriversLock(); if (!(def = virNWFilterDefParseString(xml))) goto cleanup; @@ -572,7 +579,6 @@ nwfilterDefineXML(virConnectPtr conn, if (obj) virNWFilterObjUnlock(obj); - virNWFilterCallbackDriversUnlock(); virNWFilterUnlockFilterUpdates(); nwfilterDriverUnlock(); return nwfilter; @@ -588,7 +594,6 @@ nwfilterUndefine(virNWFilterPtr nwfilter) nwfilterDriverLock(); virNWFilterWriteLockFilterUpdates(); - virNWFilterCallbackDriversLock(); if (!(obj = nwfilterObjFromNWFilter(nwfilter->uuid))) goto cleanup; @@ -615,7 +620,6 @@ nwfilterUndefine(virNWFilterPtr nwfilter) if (obj) virNWFilterObjUnlock(obj); - virNWFilterCallbackDriversUnlock(); virNWFilterUnlockFilterUpdates(); nwfilterDriverUnlock(); return ret; diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 4b55bd6ca4..d208d0188e 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -153,9 +153,9 @@ virNWFilterVarHashmapAddStdValues(virHashTablePtr table, if (!val) return -1; - if (virHashAddEntry(table, - NWFILTER_STD_VAR_MAC, - val) < 0) { + if (virHashUpdateEntry(table, + NWFILTER_STD_VAR_MAC, + val) < 0) { virNWFilterVarValueFree(val); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add variable 'MAC' to hashmap")); @@ -168,9 +168,9 @@ virNWFilterVarHashmapAddStdValues(virHashTablePtr table, if (!val) return -1; - if (virHashAddEntry(table, - NWFILTER_STD_VAR_IP, - val) < 0) { + if (virHashUpdateEntry(table, + NWFILTER_STD_VAR_IP, + val) < 0) { virNWFilterVarValueFree(val); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add variable 'IP' to hashmap")); @@ -973,68 +973,113 @@ virNWFilterTeardownFilter(virNWFilterBindingDefPtr binding) return ret; } +enum { + STEP_APPLY_NEW, + STEP_ROLLBACK, + STEP_SWITCH, + STEP_APPLY_CURRENT, +}; -int -virNWFilterDomainFWUpdateCB(virDomainObjPtr obj, - void *data) +static int +virNWFilterBuildOne(virNWFilterDriverStatePtr driver, + virNWFilterBindingDefPtr binding, + virHashTablePtr skipInterfaces, + int step) { - virDomainDefPtr vm = obj->def; - struct domUpdateCBStruct *cb = data; - size_t i; bool skipIface; int ret = 0; - - virObjectLock(obj); - - if (virDomainObjIsActive(obj)) { - for (i = 0; i < vm->nnets; i++) { - virDomainNetDefPtr net = vm->nets[i]; - virNWFilterBindingDefPtr binding; - - if ((net->filter) && (net->ifname) && - (binding = virNWFilterBindingDefForNet( - vm->name, vm->uuid, net))) { - - switch (cb->step) { - case STEP_APPLY_NEW: - ret = virNWFilterUpdateInstantiateFilter(cb->opaque, - binding, - &skipIface); - if (ret == 0 && skipIface) { - /* filter tree unchanged -- no update needed */ - ret = virHashAddEntry(cb->skipInterfaces, - net->ifname, - (void *)~0); - } - break; - - case STEP_TEAR_NEW: - if (!virHashLookup(cb->skipInterfaces, net->ifname)) - ret = virNWFilterRollbackUpdateFilter(binding); - break; - - case STEP_TEAR_OLD: - if (!virHashLookup(cb->skipInterfaces, net->ifname)) - ret = virNWFilterTearOldFilter(binding); - break; - - case STEP_APPLY_CURRENT: - ret = virNWFilterInstantiateFilter(cb->opaque, - binding); - if (ret) - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failure while applying current filter on " - "VM %s"), vm->name); - break; - } - virNWFilterBindingDefFree(binding); - if (ret) - break; - } + VIR_DEBUG("Building filter for portdev=%s step=%d", binding->portdevname, step); + + switch (step) { + case STEP_APPLY_NEW: + ret = virNWFilterUpdateInstantiateFilter(driver, + binding, + &skipIface); + if (ret == 0 && skipIface) { + /* filter tree unchanged -- no update needed */ + ret = virHashAddEntry(skipInterfaces, + binding->portdevname, + (void *)~0); } + break; + + case STEP_ROLLBACK: + if (!virHashLookup(skipInterfaces, binding->portdevname)) + ret = virNWFilterRollbackUpdateFilter(binding); + break; + + case STEP_SWITCH: + if (!virHashLookup(skipInterfaces, binding->portdevname)) + ret = virNWFilterTearOldFilter(binding); + break; + + case STEP_APPLY_CURRENT: + ret = virNWFilterInstantiateFilter(driver, + binding); + break; } - virObjectUnlock(obj); + return ret; +} + + +struct virNWFilterBuildData { + virNWFilterDriverStatePtr driver; + virHashTablePtr skipInterfaces; + int step; +}; + +static int +virNWFilterBuildIter(virNWFilterBindingObjPtr binding, void *opaque) +{ + struct virNWFilterBuildData *data = opaque; + virNWFilterBindingDefPtr def = virNWFilterBindingObjGetDef(binding); + + return virNWFilterBuildOne(data->driver, def, + data->skipInterfaces, data->step); +} + +int +virNWFilterBuildAll(virNWFilterDriverStatePtr driver, + bool newFilters) +{ + struct virNWFilterBuildData data = { + .driver = driver, + }; + int ret = 0; + + VIR_DEBUG("Build all filters newFilters=%d", newFilters); + + if (newFilters) { + if (!(data.skipInterfaces = virHashCreate(0, NULL))) + return -1; + + data.step = STEP_APPLY_NEW; + if (virNWFilterBindingObjListForEach(driver->bindings, + virNWFilterBuildIter, + &data) < 0) + ret = -1; + + if (ret == -1) { + data.step = STEP_ROLLBACK; + virNWFilterBindingObjListForEach(driver->bindings, + virNWFilterBuildIter, + &data); + } else { + data.step = STEP_SWITCH; + virNWFilterBindingObjListForEach(driver->bindings, + virNWFilterBuildIter, + &data); + } + + virHashFree(data.skipInterfaces); + } else { + data.step = STEP_APPLY_CURRENT; + if (virNWFilterBindingObjListForEach(driver->bindings, + virNWFilterBuildIter, + &data) < 0) + ret = -1; + } return ret; } diff --git a/src/nwfilter/nwfilter_gentech_driver.h b/src/nwfilter/nwfilter_gentech_driver.h index 6b51096a0d..481fdd2413 100644 --- a/src/nwfilter/nwfilter_gentech_driver.h +++ b/src/nwfilter/nwfilter_gentech_driver.h @@ -54,8 +54,8 @@ int virNWFilterTeardownFilter(virNWFilterBindingDefPtr binding); virHashTablePtr virNWFilterCreateVarHashmap(const char *macaddr, const virNWFilterVarValue *value); -int virNWFilterDomainFWUpdateCB(virDomainObjPtr vm, - void *data); +int virNWFilterBuildAll(virNWFilterDriverStatePtr driver, + bool newFilters); virNWFilterBindingDefPtr virNWFilterBindingDefForNet(const char *vmname, const unsigned char *vmuuid, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 129bacdd34..4e94b4f095 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -84,7 +84,6 @@ #include "cpu/cpu.h" #include "virsysinfo.h" #include "domain_nwfilter.h" -#include "nwfilter_conf.h" #include "virhook.h" #include "virstoragefile.h" #include "virfile.h" @@ -164,28 +163,6 @@ static int qemuARPGetInterfaces(virDomainObjPtr vm, static virQEMUDriverPtr qemu_driver; - -static void -qemuVMDriverLock(void) -{} -static void -qemuVMDriverUnlock(void) -{} - -static int -qemuVMFilterRebuild(virDomainObjListIterator iter, void *data) -{ - return virDomainObjListForEach(qemu_driver->domains, iter, data); -} - -static virNWFilterCallbackDriver qemuCallbackDriver = { - .name = QEMU_DRIVER_NAME, - .vmFilterRebuild = qemuVMFilterRebuild, - .vmDriverLock = qemuVMDriverLock, - .vmDriverUnlock = qemuVMDriverUnlock, -}; - - /** * qemuDomObjFromDomain: * @domain: Domain pointer that has to be looked up @@ -941,7 +918,6 @@ qemuStateInitialize(bool privileged, qemuProcessReconnectAll(qemu_driver); - virNWFilterRegisterCallbackDriver(&qemuCallbackDriver); return 0; error: @@ -1081,7 +1057,6 @@ qemuStateCleanup(void) if (!qemu_driver) return -1; - virNWFilterUnRegisterCallbackDriver(&qemuCallbackDriver); virThreadPoolFree(qemu_driver->workerPool); virObjectUnref(qemu_driver->config); virObjectUnref(qemu_driver->hostdevMgr); diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 0c5b7fcda7..c77988f01e 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -55,7 +55,6 @@ #include "datatypes.h" #include "virlog.h" #include "domain_nwfilter.h" -#include "nwfilter_conf.h" #include "virfile.h" #include "virfdstream.h" #include "configmake.h" @@ -143,25 +142,6 @@ static int umlMonitorCommand(const struct uml_driver *driver, static struct uml_driver *uml_driver; -static int -umlVMFilterRebuild(virDomainObjListIterator iter, void *data) -{ - return virDomainObjListForEach(uml_driver->domains, iter, data); -} - -static void -umlVMDriverLock(void) -{ - umlDriverLock(uml_driver); -} - -static void -umlVMDriverUnlock(void) -{ - umlDriverUnlock(uml_driver); -} - - static virDomainObjPtr umlDomObjFromDomainLocked(struct uml_driver *driver, const unsigned char *uuid) @@ -194,13 +174,6 @@ umlDomObjFromDomain(struct uml_driver *driver, } -static virNWFilterCallbackDriver umlCallbackDriver = { - .name = "UML", - .vmFilterRebuild = umlVMFilterRebuild, - .vmDriverLock = umlVMDriverLock, - .vmDriverUnlock = umlVMDriverUnlock, -}; - struct umlAutostartData { struct uml_driver *driver; virConnectPtr conn; @@ -604,7 +577,6 @@ umlStateInitialize(bool privileged, VIR_FREE(userdir); - virNWFilterRegisterCallbackDriver(¨CallbackDriver); return 0; out_of_memory: @@ -697,7 +669,6 @@ umlStateCleanup(void) return -1; umlDriverLock(uml_driver); - virNWFilterRegisterCallbackDriver(¨CallbackDriver); if (uml_driver->inotifyWatch != -1) virEventRemoveHandle(uml_driver->inotifyWatch); VIR_FORCE_CLOSE(uml_driver->inotifyFD); -- 2.17.1
Wire up the ListAll, LookupByPortDev and GetXMLDesc APIs to allow the virsh nwfilter-binding-list & nwfilter-binding-dumpxml commands to work. Reviewed-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/nwfilter/nwfilter_driver.c | 76 ++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index e49e0e7406..79509fc4c0 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -714,6 +714,79 @@ nwfilterTeardownFilter(virDomainNetDefPtr net) } +static virNWFilterBindingPtr +nwfilterBindingLookupByPortDev(virConnectPtr conn, + const char *portdev) +{ + virNWFilterBindingPtr ret = NULL; + virNWFilterBindingObjPtr obj; + virNWFilterBindingDefPtr def; + + obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, + portdev); + if (!obj) + goto cleanup; + + def = virNWFilterBindingObjGetDef(obj); + if (virNWFilterBindingLookupByPortDevEnsureACL(conn, def) < 0) + goto cleanup; + + ret = virGetNWFilterBinding(conn, def->portdevname, def->filter); + + cleanup: + virNWFilterBindingObjEndAPI(&obj); + return ret; +} + + +static int +nwfilterConnectListAllNWFilterBindings(virConnectPtr conn, + virNWFilterBindingPtr **bindings, + unsigned int flags) +{ + int ret; + + virCheckFlags(0, -1); + + if (virConnectListAllNWFilterBindingsEnsureACL(conn) < 0) + return -1; + + ret = virNWFilterBindingObjListExport(driver->bindings, + conn, + bindings, + virConnectListAllNWFilterBindingsCheckACL); + + return ret; +} + + +static char * +nwfilterBindingGetXMLDesc(virNWFilterBindingPtr binding, + unsigned int flags) +{ + virNWFilterBindingObjPtr obj; + virNWFilterBindingDefPtr def; + char *ret = NULL; + + virCheckFlags(0, NULL); + + obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, + binding->portdev); + if (!obj) + goto cleanup; + + def = virNWFilterBindingObjGetDef(obj); + if (virNWFilterBindingGetXMLDescEnsureACL(binding->conn, def) < 0) + goto cleanup; + + ret = virNWFilterBindingDefFormat(def); + + cleanup: + virNWFilterBindingObjEndAPI(&obj); + return ret; +} + + static virNWFilterDriver nwfilterDriver = { .name = "nwfilter", .connectNumOfNWFilters = nwfilterConnectNumOfNWFilters, /* 0.8.0 */ @@ -724,6 +797,9 @@ static virNWFilterDriver nwfilterDriver = { .nwfilterDefineXML = nwfilterDefineXML, /* 0.8.0 */ .nwfilterUndefine = nwfilterUndefine, /* 0.8.0 */ .nwfilterGetXMLDesc = nwfilterGetXMLDesc, /* 0.8.0 */ + .nwfilterBindingLookupByPortDev = nwfilterBindingLookupByPortDev, /* 4.5.0 */ + .connectListAllNWFilterBindings = nwfilterConnectListAllNWFilterBindings, /* 4.5.0 */ + .nwfilterBindingGetXMLDesc = nwfilterBindingGetXMLDesc, /* 4.5.0 */ }; -- 2.17.1
This allows the virsh commands nwfilter-binding-create and nwfilter-binding-delete to be used. Note using these commands lets you delete filters that were previously created automatically by the virt drivers, or add filters for VM nics that were not there before. Generally it is expected these new APIs will only be used by virt drivers. It is the admin's responsibility to not shoot themselves in the foot. Reviewed-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/nwfilter/nwfilter_driver.c | 86 ++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 79509fc4c0..83a2e19dbe 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -787,6 +787,90 @@ nwfilterBindingGetXMLDesc(virNWFilterBindingPtr binding, } +static virNWFilterBindingPtr +nwfilterBindingCreateXML(virConnectPtr conn, + const char *xml, + unsigned int flags) +{ + virNWFilterBindingObjPtr obj; + virNWFilterBindingDefPtr def; + virNWFilterBindingPtr ret = NULL; + + virCheckFlags(0, NULL); + + def = virNWFilterBindingDefParseString(xml); + if (!def) + return NULL; + + if (virNWFilterBindingCreateXMLEnsureACL(conn, def) < 0) + goto cleanup; + + obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, def->portdevname); + if (obj) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Filter already present for NIC %s"), def->portdevname); + goto cleanup; + } + + obj = virNWFilterBindingObjListAdd(driver->bindings, + def); + if (!obj) + goto cleanup; + + if (!(ret = virGetNWFilterBinding(conn, def->portdevname, def->filter))) + goto cleanup; + + if (virNWFilterInstantiateFilter(driver, def) < 0) { + virNWFilterBindingObjListRemove(driver->bindings, obj); + virObjectUnref(ret); + ret = NULL; + goto cleanup; + } + virNWFilterBindingObjSave(obj, driver->bindingDir); + + cleanup: + if (!obj) + virNWFilterBindingDefFree(def); + virNWFilterBindingObjEndAPI(&obj); + + return ret; +} + + +/* + * Note that this is primarily intended for usage by the hypervisor + * drivers. it is exposed to the admin, however, and nothing stops + * an admin from deleting filter bindings created by the hypervisor + * drivers. IOW, it is the admin's responsibility not to shoot + * themself in the foot + */ +static int +nwfilterBindingDelete(virNWFilterBindingPtr binding) +{ + virNWFilterBindingObjPtr obj; + virNWFilterBindingDefPtr def; + int ret = -1; + + obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, binding->portdev); + if (!obj) + return -1; + + def = virNWFilterBindingObjGetDef(obj); + if (virNWFilterBindingDeleteEnsureACL(binding->conn, def) < 0) + goto cleanup; + + virNWFilterTeardownFilter(def); + virNWFilterBindingObjDelete(obj, driver->bindingDir); + virNWFilterBindingObjListRemove(driver->bindings, obj); + + ret = 0; + + cleanup: + virNWFilterBindingObjEndAPI(&obj); + return ret; +} + + static virNWFilterDriver nwfilterDriver = { .name = "nwfilter", .connectNumOfNWFilters = nwfilterConnectNumOfNWFilters, /* 0.8.0 */ @@ -800,6 +884,8 @@ static virNWFilterDriver nwfilterDriver = { .nwfilterBindingLookupByPortDev = nwfilterBindingLookupByPortDev, /* 4.5.0 */ .connectListAllNWFilterBindings = nwfilterConnectListAllNWFilterBindings, /* 4.5.0 */ .nwfilterBindingGetXMLDesc = nwfilterBindingGetXMLDesc, /* 4.5.0 */ + .nwfilterBindingCreateXML = nwfilterBindingCreateXML, /* 4.5.0 */ + .nwfilterBindingDelete = nwfilterBindingDelete, /* 4.5.0 */ }; -- 2.17.1
Remove the callbacks that the nwfilter driver registers with the domain object config layer. Instead make the current helper methods call into the public API for creating/deleting nwfilter bindings. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/domain_nwfilter.c | 135 +++++++++++++++++++++---- src/conf/domain_nwfilter.h | 16 +-- src/libvirt_private.syms | 1 - src/lxc/lxc_process.c | 2 +- src/nwfilter/nwfilter_driver.c | 82 +++------------ src/nwfilter/nwfilter_gentech_driver.c | 42 -------- src/nwfilter/nwfilter_gentech_driver.h | 4 - src/qemu/qemu_hotplug.c | 4 +- src/qemu/qemu_interface.c | 4 +- src/qemu/qemu_process.c | 6 +- src/remote/remote_daemon.c | 1 + src/uml/uml_conf.c | 2 +- 12 files changed, 142 insertions(+), 157 deletions(-) diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c index 7570e0ae83..948b32481e 100644 --- a/src/conf/domain_nwfilter.c +++ b/src/conf/domain_nwfilter.c @@ -28,45 +28,146 @@ #include "datatypes.h" #include "domain_conf.h" #include "domain_nwfilter.h" +#include "virnwfilterbindingdef.h" #include "virerror.h" +#include "viralloc.h" +#include "virstring.h" +#include "virlog.h" -#define VIR_FROM_THIS VIR_FROM_NWFILTER -static virDomainConfNWFilterDriverPtr nwfilterDriver; +VIR_LOG_INIT("conf.domain_nwfilter"); -void -virDomainConfNWFilterRegister(virDomainConfNWFilterDriverPtr driver) +#define VIR_FROM_THIS VIR_FROM_NWFILTER + +static virNWFilterBindingDefPtr +virNWFilterBindingDefForNet(const char *vmname, + const unsigned char *vmuuid, + virDomainNetDefPtr net) { - nwfilterDriver = driver; + virNWFilterBindingDefPtr ret; + + if (VIR_ALLOC(ret) < 0) + return NULL; + + if (VIR_STRDUP(ret->ownername, vmname) < 0) + goto error; + + memcpy(ret->owneruuid, vmuuid, sizeof(ret->owneruuid)); + + if (VIR_STRDUP(ret->portdevname, net->ifname) < 0) + goto error; + + if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT && + VIR_STRDUP(ret->linkdevname, net->data.direct.linkdev) < 0) + goto error; + + ret->mac = net->mac; + + if (VIR_STRDUP(ret->filter, net->filter) < 0) + goto error; + + if (!(ret->filterparams = virNWFilterHashTableCreate(0))) + goto error; + + if (net->filterparams && + virNWFilterHashTablePutAll(net->filterparams, ret->filterparams) < 0) + goto error; + + return ret; + + error: + virNWFilterBindingDefFree(ret); + return NULL; } + int virDomainConfNWFilterInstantiate(const char *vmname, const unsigned char *vmuuid, - virDomainNetDefPtr net) + virDomainNetDefPtr net, + bool ignoreExists) { - if (nwfilterDriver != NULL) - return nwfilterDriver->instantiateFilter(vmname, vmuuid, net); + virConnectPtr conn = virGetConnectNWFilter(); + virNWFilterBindingDefPtr def = NULL; + virNWFilterBindingPtr binding = NULL; + char *xml; + int ret = -1; + + VIR_DEBUG("vmname=%s portdev=%s filter=%s ignoreExists=%d", + vmname, NULLSTR(net->ifname), NULLSTR(net->filter), ignoreExists); + + if (!conn) + goto cleanup; + + if (ignoreExists) { + binding = virNWFilterBindingLookupByPortDev(conn, net->ifname); + if (binding) { + ret = 0; + goto cleanup; + } + } - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("No network filter driver available")); - return -1; + if (!(def = virNWFilterBindingDefForNet(vmname, vmuuid, net))) + goto cleanup; + + if (!(xml = virNWFilterBindingDefFormat(def))) + goto cleanup; + + if (!(binding = virNWFilterBindingCreateXML(conn, xml, 0))) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(xml); + virNWFilterBindingDefFree(def); + virObjectUnref(binding); + virObjectUnref(conn); + return ret; } + +static void +virDomainConfNWFilterTeardownImpl(virConnectPtr conn, + virDomainNetDefPtr net) +{ + virNWFilterBindingPtr binding; + + binding = virNWFilterBindingLookupByPortDev(conn, net->ifname); + if (!binding) + return; + + virNWFilterBindingDelete(binding); + + virObjectUnref(binding); +} + + void virDomainConfNWFilterTeardown(virDomainNetDefPtr net) { - if (nwfilterDriver != NULL) - nwfilterDriver->teardownFilter(net); + virConnectPtr conn = virGetConnectNWFilter(); + + if (!conn) + return; + + virDomainConfNWFilterTeardownImpl(conn, net); + + virObjectUnref(conn); } void virDomainConfVMNWFilterTeardown(virDomainObjPtr vm) { size_t i; + virConnectPtr conn = virGetConnectNWFilter(); - if (nwfilterDriver != NULL) { - for (i = 0; i < vm->def->nnets; i++) - virDomainConfNWFilterTeardown(vm->def->nets[i]); - } + if (!conn) + return; + + + for (i = 0; i < vm->def->nnets; i++) + virDomainConfNWFilterTeardownImpl(conn, vm->def->nets[i]); + + virObjectUnref(conn); } diff --git a/src/conf/domain_nwfilter.h b/src/conf/domain_nwfilter.h index 857cac6c2a..6bda228fc8 100644 --- a/src/conf/domain_nwfilter.h +++ b/src/conf/domain_nwfilter.h @@ -23,22 +23,10 @@ #ifndef DOMAIN_NWFILTER_H # define DOMAIN_NWFILTER_H -typedef int (*virDomainConfInstantiateNWFilter)(const char *vmname, - const unsigned char *vmuuid, - virDomainNetDefPtr net); -typedef void (*virDomainConfTeardownNWFilter)(virDomainNetDefPtr net); - -typedef struct { - virDomainConfInstantiateNWFilter instantiateFilter; - virDomainConfTeardownNWFilter teardownFilter; -} virDomainConfNWFilterDriver; -typedef virDomainConfNWFilterDriver *virDomainConfNWFilterDriverPtr; - -void virDomainConfNWFilterRegister(virDomainConfNWFilterDriverPtr driver); - int virDomainConfNWFilterInstantiate(const char *vmname, const unsigned char *vmuuid, - virDomainNetDefPtr net); + virDomainNetDefPtr net, + bool ignoreExists); void virDomainConfNWFilterTeardown(virDomainNetDefPtr net); void virDomainConfVMNWFilterTeardown(virDomainObjPtr vm); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 42547e64ed..f81333baf6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -651,7 +651,6 @@ virDomainQemuMonitorEventStateRegisterID; # conf/domain_nwfilter.h virDomainConfNWFilterInstantiate; -virDomainConfNWFilterRegister; virDomainConfNWFilterTeardown; virDomainConfVMNWFilterTeardown; diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 60ae7daaed..14502e12fe 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -303,7 +303,7 @@ virLXCProcessSetupInterfaceTap(virDomainDefPtr vm, } if (net->filter && - virDomainConfNWFilterInstantiate(vm->name, vm->uuid, net) < 0) + virDomainConfNWFilterInstantiate(vm->name, vm->uuid, net, false) < 0) goto cleanup; ret = containerVeth; diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 83a2e19dbe..d385b46f5f 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -655,65 +655,6 @@ nwfilterGetXMLDesc(virNWFilterPtr nwfilter, } -static int -nwfilterInstantiateFilter(const char *vmname, - const unsigned char *vmuuid, - virDomainNetDefPtr net) -{ - virNWFilterBindingObjPtr obj; - virNWFilterBindingDefPtr def; - int ret; - - obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, net->ifname); - if (obj) { - virNWFilterBindingObjEndAPI(&obj); - return 0; - } - - if (!(def = virNWFilterBindingDefForNet(vmname, vmuuid, net))) - return -1; - - obj = virNWFilterBindingObjListAdd(driver->bindings, - def); - if (!obj) { - virNWFilterBindingDefFree(def); - return -1; - } - - ret = virNWFilterInstantiateFilter(driver, def); - - if (ret >= 0) - virNWFilterBindingObjSave(obj, driver->bindingDir); - else - virNWFilterBindingObjListRemove(driver->bindings, obj); - - virNWFilterBindingObjEndAPI(&obj); - - return ret; -} - - -static void -nwfilterTeardownFilter(virDomainNetDefPtr net) -{ - virNWFilterBindingObjPtr obj; - virNWFilterBindingDefPtr def; - if (!net->ifname) - return; - - obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, net->ifname); - if (!obj) - return; - - def = virNWFilterBindingObjGetDef(obj); - virNWFilterTeardownFilter(def); - virNWFilterBindingObjDelete(obj, driver->bindingDir); - - virNWFilterBindingObjListRemove(driver->bindings, obj); - virNWFilterBindingObjEndAPI(&obj); -} - - static virNWFilterBindingPtr nwfilterBindingLookupByPortDev(virConnectPtr conn, const char *portdev) @@ -724,8 +665,11 @@ nwfilterBindingLookupByPortDev(virConnectPtr conn, obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, portdev); - if (!obj) + if (!obj) { + virReportError(VIR_ERR_NO_NWFILTER_BINDING, + _("no nwfilter binding for port dev '%s'"), portdev); goto cleanup; + } def = virNWFilterBindingObjGetDef(obj); if (virNWFilterBindingLookupByPortDevEnsureACL(conn, def) < 0) @@ -772,8 +716,11 @@ nwfilterBindingGetXMLDesc(virNWFilterBindingPtr binding, obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, binding->portdev); - if (!obj) + if (!obj) { + virReportError(VIR_ERR_NO_NWFILTER_BINDING, + _("no nwfilter binding for port dev '%s'"), binding->portdev); goto cleanup; + } def = virNWFilterBindingObjGetDef(obj); if (virNWFilterBindingGetXMLDescEnsureACL(binding->conn, def) < 0) @@ -852,8 +799,11 @@ nwfilterBindingDelete(virNWFilterBindingPtr binding) int ret = -1; obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, binding->portdev); - if (!obj) + if (!obj) { + virReportError(VIR_ERR_NO_NWFILTER_BINDING, + _("no nwfilter binding for port dev '%s'"), binding->portdev); return -1; + } def = virNWFilterBindingObjGetDef(obj); if (virNWFilterBindingDeleteEnsureACL(binding->conn, def) < 0) @@ -914,13 +864,6 @@ static virStateDriver stateDriver = { .stateReload = nwfilterStateReload, }; - -static virDomainConfNWFilterDriver domainNWFilterDriver = { - .instantiateFilter = nwfilterInstantiateFilter, - .teardownFilter = nwfilterTeardownFilter, -}; - - int nwfilterRegister(void) { if (virRegisterConnectDriver(&nwfilterConnectDriver, false) < 0) @@ -929,6 +872,5 @@ int nwfilterRegister(void) return -1; if (virRegisterStateDriver(&stateDriver) < 0) return -1; - virDomainConfNWFilterRegister(&domainNWFilterDriver); return 0; } diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index d208d0188e..e5dea91f83 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -1082,45 +1082,3 @@ virNWFilterBuildAll(virNWFilterDriverStatePtr driver, } return ret; } - - -virNWFilterBindingDefPtr -virNWFilterBindingDefForNet(const char *vmname, - const unsigned char *vmuuid, - virDomainNetDefPtr net) -{ - virNWFilterBindingDefPtr ret; - - if (VIR_ALLOC(ret) < 0) - return NULL; - - if (VIR_STRDUP(ret->ownername, vmname) < 0) - goto error; - - memcpy(ret->owneruuid, vmuuid, sizeof(ret->owneruuid)); - - if (VIR_STRDUP(ret->portdevname, net->ifname) < 0) - goto error; - - if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT && - VIR_STRDUP(ret->linkdevname, net->data.direct.linkdev) < 0) - goto error; - - ret->mac = net->mac; - - if (VIR_STRDUP(ret->filter, net->filter) < 0) - goto error; - - if (!(ret->filterparams = virNWFilterHashTableCreate(0))) - goto error; - - if (net->filterparams && - virNWFilterHashTablePutAll(net->filterparams, ret->filterparams) < 0) - goto error; - - return ret; - - error: - virNWFilterBindingDefFree(ret); - return NULL; -} diff --git a/src/nwfilter/nwfilter_gentech_driver.h b/src/nwfilter/nwfilter_gentech_driver.h index 481fdd2413..2cd19c90fc 100644 --- a/src/nwfilter/nwfilter_gentech_driver.h +++ b/src/nwfilter/nwfilter_gentech_driver.h @@ -57,8 +57,4 @@ virHashTablePtr virNWFilterCreateVarHashmap(const char *macaddr, int virNWFilterBuildAll(virNWFilterDriverStatePtr driver, bool newFilters); -virNWFilterBindingDefPtr virNWFilterBindingDefForNet(const char *vmname, - const unsigned char *vmuuid, - virDomainNetDefPtr net); - #endif diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7a1bbc7c8c..58cb0539e1 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3009,7 +3009,7 @@ qemuDomainChangeNetFilter(virDomainObjPtr vm, if (newdev->filter && virDomainConfNWFilterInstantiate(vm->def->name, - vm->def->uuid, newdev) < 0) { + vm->def->uuid, newdev, false) < 0) { virErrorPtr errobj; virReportError(VIR_ERR_OPERATION_FAILED, @@ -3018,7 +3018,7 @@ qemuDomainChangeNetFilter(virDomainObjPtr vm, olddev->ifname); virErrorPreserveLast(&errobj); ignore_value(virDomainConfNWFilterInstantiate(vm->def->name, - vm->def->uuid, olddev)); + vm->def->uuid, olddev, false)); virErrorRestore(&errobj); return -1; } diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c index 5d54a85c53..a3f13093f5 100644 --- a/src/qemu/qemu_interface.c +++ b/src/qemu/qemu_interface.c @@ -467,7 +467,7 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def, goto cleanup; if (net->filter && - virDomainConfNWFilterInstantiate(def->name, def->uuid, net) < 0) { + virDomainConfNWFilterInstantiate(def->name, def->uuid, net, false) < 0) { goto cleanup; } @@ -586,7 +586,7 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def, goto cleanup; if (net->filter && - virDomainConfNWFilterInstantiate(def->name, def->uuid, net) < 0) { + virDomainConfNWFilterInstantiate(def->name, def->uuid, net, false) < 0) { goto cleanup; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7e9ad01e61..ac32dafcbe 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3018,14 +3018,14 @@ qemuProcessNotifyNets(virDomainDefPtr def) } static int -qemuProcessFiltersInstantiate(virDomainDefPtr def) +qemuProcessFiltersInstantiate(virDomainDefPtr def, bool ignoreExists) { size_t i; for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; if ((net->filter) && (net->ifname)) { - if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net) < 0) + if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net, ignoreExists) < 0) return 1; } } @@ -7650,7 +7650,7 @@ qemuProcessReconnect(void *opaque) qemuProcessNotifyNets(obj->def); - if (qemuProcessFiltersInstantiate(obj->def)) + if (qemuProcessFiltersInstantiate(obj->def, true)) goto error; if (qemuProcessRefreshDisks(driver, obj, QEMU_ASYNC_JOB_NONE) < 0) diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 21ab22499d..9f3a5f38ad 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -283,6 +283,7 @@ static int daemonErrorLogFilter(virErrorPtr err, int priority) case VIR_ERR_NO_NODE_DEVICE: case VIR_ERR_NO_INTERFACE: case VIR_ERR_NO_NWFILTER: + case VIR_ERR_NO_NWFILTER_BINDING: case VIR_ERR_NO_SECRET: case VIR_ERR_NO_DOMAIN_SNAPSHOT: case VIR_ERR_OPERATION_INVALID: diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 9c548f0e80..f116e619ef 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -137,7 +137,7 @@ umlConnectTapDevice(virDomainDefPtr vm, } if (net->filter) { - if (virDomainConfNWFilterInstantiate(vm->name, vm->uuid, net) < 0) { + if (virDomainConfNWFilterInstantiate(vm->name, vm->uuid, net, false) < 0) { if (template_ifname) VIR_FREE(net->ifname); goto error; -- 2.17.1
On 06/26/2018 06:23 AM, Daniel P. Berrangé wrote:
Remove the callbacks that the nwfilter driver registers with the domain object config layer. Instead make the current helper methods call into the public API for creating/deleting nwfilter bindings.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/domain_nwfilter.c | 135 +++++++++++++++++++++---- src/conf/domain_nwfilter.h | 16 +-- src/libvirt_private.syms | 1 - src/lxc/lxc_process.c | 2 +- src/nwfilter/nwfilter_driver.c | 82 +++------------ src/nwfilter/nwfilter_gentech_driver.c | 42 -------- src/nwfilter/nwfilter_gentech_driver.h | 4 - src/qemu/qemu_hotplug.c | 4 +- src/qemu/qemu_interface.c | 4 +- src/qemu/qemu_process.c | 6 +- src/remote/remote_daemon.c | 1 + src/uml/uml_conf.c | 2 +- 12 files changed, 142 insertions(+), 157 deletions(-)
My CPU's were kept quite busy during the avocado tests and the environment is a bit fragile - causing a re-run because of bad user on keyboard |-/. But I got through what was failing before, the aggressive create/destroy filter while start/stop domain was successful unless it was too busy smoking the CPU... Visually the code covers the issues I was seeing before, so Reviewed-by: John Ferlan <jferlan@redhat.com> John
participants (2)
-
Daniel P. Berrangé -
John Ferlan