118 lines
5.5 KiB
Diff
118 lines
5.5 KiB
Diff
From b2e0155b59ae9f038bcf21da7c6b7fb0a99a7b67 Mon Sep 17 00:00:00 2001
|
|
Message-Id: <b2e0155b59ae9f038bcf21da7c6b7fb0a99a7b67@dist-git>
|
|
From: Laine Stump <laine@redhat.com>
|
|
Date: Tue, 1 Dec 2020 22:01:00 -0500
|
|
Subject: [PATCH] conf: properly clear out autogenerated macvtap names when
|
|
formatting/parsing
|
|
|
|
Back when macvtap support was added in commit 315baab9443 in Feb. 2010
|
|
(libvirt-0.7.7), it was setup to autogenerate a name for the device if
|
|
one wasn't supplied, in the pattern "macvtap%d" (or "macvlan%d"),
|
|
similar to the way an unspecified standard tap device name will lead
|
|
to an autogenerated "vnet%d".
|
|
|
|
As a matter of fact, in commit ca1b7cc8e45 added in May 2010, the code
|
|
was changed to *always* ignore a supplied device name for macvtap
|
|
interfaces by deleting *any* name immediately during the <interface>
|
|
parsing (this was intended to prevent one domain which had failed to
|
|
completely start from deleting the macvtap device of another domain
|
|
which had subsequently been provided the same device name (this will
|
|
seem mildly ironic later). This was later fixed to only clear the
|
|
device name when inactive XML was being parsed. HOWEVER - this was
|
|
only done if the xml was <interface type='direct'> - autogenerated
|
|
names were not cleared for <interface type='network'> (which could
|
|
also result in a macvtap device).
|
|
|
|
Although the names of "vnetX" tap devices had always been
|
|
automatically cleared when parsing <interface> (see commit d1304583d
|
|
from July 2008 (!)), at the time macvtap support was added, both vnetX
|
|
and macvtapX device names were always included when formatting the
|
|
XML.
|
|
|
|
Then in commit a8be259d0cc (July 2011, libvirt-0.9.4), <interface>
|
|
formatting was changed to also clear out "vnetX" device names during
|
|
XML formatting as well. However the same treatment wasn't given to
|
|
"macvtapX".
|
|
|
|
Now in 2020, there has been a report that a failed migration leads to
|
|
the macvtap device of some other unrelated guest on the destination
|
|
host losing its network connectivity. It was determined that this was
|
|
due to the domain XML in the migration containing a macvtap device
|
|
name, e.g. "macvtap0", that was already in use by the other guest on
|
|
the destination. Normally this wouldn't be a problem, because libvirt
|
|
would see that the device was already in use, and then find a
|
|
different unused name. But in this case, other external problems were
|
|
causing the migration to fail prior to selecting a macvtap device and
|
|
successfully opening it, and during error recovery, qemuProcessStop()
|
|
was called, which went through all def->nets objects and (if they were
|
|
macvtap) deleted the device specified in net->ifname; since libvirt
|
|
hadn't gotten to the point of replacing the incoming "macvtap0" with
|
|
the name of a device it actually created for this guest, that meant
|
|
that "macvtap0" was deleted, *even though it was currently in use by a
|
|
different guest*!
|
|
|
|
Whew!
|
|
|
|
So, it turns out that when formatting "migratable" XML, "vnetX"
|
|
devices are omitted, just as when formatting "inactive" XML. By making
|
|
the code in both interface parsing and formatting consistent for
|
|
"vnetX", "macvtapX", and "macvlanX", we can thus make sure that the
|
|
autogenerated (and unneeded / completely *not* wanted) macvtap device
|
|
name will not be sent with the migration XML. This way when a
|
|
migration fails, net->ifname will be NULL, and libvirt won't have any
|
|
device to try and (erroneously) delete.
|
|
|
|
Signed-off-by: Laine Stump <laine@redhat.com>
|
|
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
|
|
(cherry picked from commit 282d135ddbb7203565cd5527b451469b14953994)
|
|
|
|
https://bugzilla.redhat.com/1872610
|
|
|
|
Signed-off-by: Laine Stump <laine@redhat.com>
|
|
Message-Id: <20201202030100.458879-1-laine@redhat.com>
|
|
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
|
---
|
|
src/conf/domain_conf.c | 12 ++++--------
|
|
1 file changed, 4 insertions(+), 8 deletions(-)
|
|
|
|
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
|
|
index f41559f33e..cd5c15f297 100644
|
|
--- a/src/conf/domain_conf.c
|
|
+++ b/src/conf/domain_conf.c
|
|
@@ -12183,14 +12183,6 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
|
|
}
|
|
|
|
def->data.direct.linkdev = g_steal_pointer(&dev);
|
|
-
|
|
- if (ifname &&
|
|
- flags & VIR_DOMAIN_DEF_PARSE_INACTIVE &&
|
|
- (STRPREFIX(ifname, VIR_NET_GENERATED_MACVTAP_PREFIX) ||
|
|
- STRPREFIX(ifname, VIR_NET_GENERATED_MACVLAN_PREFIX))) {
|
|
- VIR_FREE(ifname);
|
|
- }
|
|
-
|
|
break;
|
|
|
|
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
|
|
@@ -12238,6 +12230,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
|
|
if (def->managed_tap != VIR_TRISTATE_BOOL_NO && ifname &&
|
|
(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
|
|
(STRPREFIX(ifname, VIR_NET_GENERATED_TAP_PREFIX) ||
|
|
+ STRPREFIX(ifname, VIR_NET_GENERATED_MACVTAP_PREFIX) ||
|
|
+ STRPREFIX(ifname, VIR_NET_GENERATED_MACVLAN_PREFIX) ||
|
|
(prefix && STRPREFIX(ifname, prefix)))) {
|
|
/* An auto-generated target name, blank it out */
|
|
VIR_FREE(ifname);
|
|
@@ -25996,6 +25990,8 @@ virDomainNetDefFormat(virBufferPtr buf,
|
|
(def->managed_tap == VIR_TRISTATE_BOOL_NO ||
|
|
!((flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&
|
|
(STRPREFIX(def->ifname, VIR_NET_GENERATED_TAP_PREFIX) ||
|
|
+ STRPREFIX(def->ifname, VIR_NET_GENERATED_MACVTAP_PREFIX) ||
|
|
+ STRPREFIX(def->ifname, VIR_NET_GENERATED_MACVLAN_PREFIX) ||
|
|
(prefix && STRPREFIX(def->ifname, prefix)))))) {
|
|
/* Skip auto-generated target names for inactive config. */
|
|
virBufferEscapeString(&attrBuf, " dev='%s'", def->ifname);
|
|
--
|
|
2.29.2
|
|
|