Blame SOURCES/libvirt-conf-properly-clear-out-autogenerated-macvtap-names-when-formatting-parsing.patch

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