7:30 a.m.
https://bugzilla.redhat.com/show_bug.cgi?id=1607202
It's essentially stated in the nwfilterBindingDelete that we
will allow the admin to shoot themselves in the foot by deleting
the nwfilter binding which then allows them to undefine the
nwfilter that is in use for the running guest...
However, by allowing this we cause a problem for libvirtd
restart reconnect processing which would then try to recreate
the missing binding attempting to use the deleted filter
resulting in an error and thus shutting the guest down.
So rather than keep adding virDomainConfNWFilterInstantiate
flags to "ignore" specific error conditions and since (so far)
this is the only path that cared about checking if the filter
already exists and ignoring, let's just ignore all errors and
make the qemuProcessFiltersInstantiate be a void which will
attempt to check all networks for bindings and reload all filters
that exist. Using the VIR_INFO in order to at least "log" the
avoided issue.
This also means virDomainConfNWFilterInstantiate no longer
needs to handle/check the ignoreExists possbility.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
v1: https://www.redhat.com/archives/libvir-list/2018-August/msg01407.html
Changes - removed the ignoreExists and just change the logic for
reconnect processing to essentially ignore all errors. If it's felt
the VIR_INFO would be too chatty (especially since it'll be generated
for every already defined filter binding), I can remove it. Another
option would be to keep the ignoreExists logic and only generate that
VIR_INFO for "other" messages. In that case, I'd probably want to change
it to a VIR_WARN. Still figured I'd post the remove it all option first
for consideration with this caveat so that "option" can be considered
as well.
src/conf/domain_nwfilter.c | 15 +++------------
src/conf/domain_nwfilter.h | 3 +--
src/lxc/lxc_process.c | 2 +-
src/qemu/qemu_hotplug.c | 4 ++--
src/qemu/qemu_interface.c | 4 ++--
src/qemu/qemu_process.c | 23 +++++++++++++++--------
src/uml/uml_conf.c | 2 +-
7 files changed, 25 insertions(+), 28 deletions(-)
diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c
index f39c8a1f9b..51c9063ca7 100644
--- a/src/conf/domain_nwfilter.c
+++ b/src/conf/domain_nwfilter.c
@@ -84,8 +84,7 @@ virNWFilterBindingDefForNet(const char *vmname,
int
virDomainConfNWFilterInstantiate(const char *vmname,
const unsigned char *vmuuid,
- virDomainNetDefPtr net,
- bool ignoreExists)
+ virDomainNetDefPtr net)
{
virConnectPtr conn = virGetConnectNWFilter();
virNWFilterBindingDefPtr def = NULL;
@@ -93,20 +92,12 @@ virDomainConfNWFilterInstantiate(const char *vmname,
char *xml = NULL;
int ret = -1;
- VIR_DEBUG("vmname=%s portdev=%s filter=%s ignoreExists=%d",
- vmname, NULLSTR(net->ifname), NULLSTR(net->filter), ignoreExists);
+ VIR_DEBUG("vmname=%s portdev=%s filter=%s",
+ vmname, NULLSTR(net->ifname), NULLSTR(net->filter));
if (!conn)
goto cleanup;
- if (ignoreExists) {
- binding = virNWFilterBindingLookupByPortDev(conn, net->ifname);
- if (binding) {
- ret = 0;
- goto cleanup;
- }
- }
-
if (!(def = virNWFilterBindingDefForNet(vmname, vmuuid, net)))
goto cleanup;
diff --git a/src/conf/domain_nwfilter.h b/src/conf/domain_nwfilter.h
index 6bda228fc8..d2ebeff853 100644
--- a/src/conf/domain_nwfilter.h
+++ b/src/conf/domain_nwfilter.h
@@ -25,8 +25,7 @@
int virDomainConfNWFilterInstantiate(const char *vmname,
const unsigned char *vmuuid,
- virDomainNetDefPtr net,
- bool ignoreExists);
+ virDomainNetDefPtr net);
void virDomainConfNWFilterTeardown(virDomainNetDefPtr net);
void virDomainConfVMNWFilterTeardown(virDomainObjPtr vm);
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 33c806630b..86f7463e53 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, false) < 0)
+ virDomainConfNWFilterInstantiate(vm->name, vm->uuid, net) < 0)
goto cleanup;
ret = containerVeth;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0b84a503bb..38c74bd9b1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3435,7 +3435,7 @@ qemuDomainChangeNetFilter(virDomainObjPtr vm,
if (newdev->filter &&
virDomainConfNWFilterInstantiate(vm->def->name,
- vm->def->uuid, newdev, false) < 0) {
+ vm->def->uuid, newdev) < 0) {
virErrorPtr errobj;
virReportError(VIR_ERR_OPERATION_FAILED,
@@ -3444,7 +3444,7 @@ qemuDomainChangeNetFilter(virDomainObjPtr vm,
olddev->ifname);
virErrorPreserveLast(&errobj);
ignore_value(virDomainConfNWFilterInstantiate(vm->def->name,
- vm->def->uuid, olddev,
false));
+ vm->def->uuid, olddev));
virErrorRestore(&errobj);
return -1;
}
diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index a3f13093f5..5d54a85c53 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, false) < 0)
{
+ virDomainConfNWFilterInstantiate(def->name, def->uuid, net) < 0) {
goto cleanup;
}
@@ -586,7 +586,7 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def,
goto cleanup;
if (net->filter &&
- virDomainConfNWFilterInstantiate(def->name, def->uuid, net, false) < 0)
{
+ virDomainConfNWFilterInstantiate(def->name, def->uuid, net) < 0) {
goto cleanup;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ab749389ee..48d9bab128 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3160,22 +3160,30 @@ qemuProcessNotifyNets(virDomainDefPtr def)
}
}
-static int
-qemuProcessFiltersInstantiate(virDomainDefPtr def, bool ignoreExists)
+
+/* Attempt to instantiate the filters. Ignore failures because it's
+ * (primarily) possible that a filter binding either already exists
+ * or someone deleted it and the associated filter while the guest
+ * was running and we don't want that action to cause failure to
+ * keep the guest running during the reconnection processing. */
+static void
+qemuProcessFiltersInstantiate(virDomainDefPtr def)
{
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,
ignoreExists) < 0)
- return 1;
+ if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net) < 0)
{
+ VIR_INFO("filter '%s' instantiation for '%s' failed
'%s'",
+ net->filter, net->ifname, virGetLastErrorMessage());
+ virResetLastError();
+ }
}
}
-
- return 0;
}
+
static int
qemuProcessUpdateState(virQEMUDriverPtr driver, virDomainObjPtr vm)
{
@@ -7892,8 +7900,7 @@ qemuProcessReconnect(void *opaque)
qemuProcessNotifyNets(obj->def);
- if (qemuProcessFiltersInstantiate(obj->def, true))
- goto error;
+ qemuProcessFiltersInstantiate(obj->def);
if (qemuProcessRefreshDisks(driver, obj, QEMU_ASYNC_JOB_NONE) < 0)
goto error;
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index f116e619ef..9c548f0e80 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, false) <
0) {
+ if (virDomainConfNWFilterInstantiate(vm->name, vm->uuid, net) < 0) {
if (template_ifname)
VIR_FREE(net->ifname);
goto error;
--
2.17.1