qemu: Don't remove macvtaps on failed start

If a domain is configured to create a macvtap/macvlan but the
target link already exists, startup fails (as expected) with:

  error: error creating macvtap interface test@eth0 (52:54:00:d9:0b:db): File exists

Okay, we could make that error message better, but that's not the
point. Since this error originated while generating cmd line
(the caller is qemuProcessStart(), transitively), the cleanup
after failed start is performed (qemuProcessStop()). Here,
virNetDevMacVLanDeleteWithVPortProfile() is called which removes
the macvtap interface we did not create (as it made us fail in
the first place).

Therefore, we need to track which macvtap/macvlan interface was
created successfully and remove only those.

You'll notice that only qemuProcessStop() has the new check. For
the (failed) hotplug case (qemuDomainAttachNetDevice()) this
function is already in place (the @iface_connected variable), or
not needed (qemuDomainRemoveNetDevice() - we're removing an
interface that was already attached to QEMU).

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2166235
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1757a6aaada58beca71a37bddc12bcdb2951f81c..e9bc0f375db4281bdec6fa3e62be3317b083cf97 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1235,6 +1235,31 @@ qemuDomainTPMPrivateFormat(const virDomainTPMDef *tpm,
 }
 
 
+static int
+qemuDomainNetworkPrivateParse(xmlXPathContextPtr ctxt,
+                              virDomainNetDef *net)
+{
+    qemuDomainNetworkPrivate *priv = QEMU_DOMAIN_NETWORK_PRIVATE(net);
+
+    priv->created = virXPathBoolean("string(./created[@value='yes'])", ctxt);
+
+    return 0;
+}
+
+
+static int
+qemuDomainNetworkPrivateFormat(const virDomainNetDef *net,
+                               virBuffer *buf)
+{
+    qemuDomainNetworkPrivate *priv = QEMU_DOMAIN_NETWORK_PRIVATE(net);
+
+    if (priv->created)
+        virBufferAddLit(buf, "<created value='yes'/>\n");
+
+    return 0;
+}
+
+
 /* qemuDomainSecretInfoSetup:
  * @priv: pointer to domain private object
  * @alias: alias of the secret
@@ -3328,6 +3353,8 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = {
     .vsockNew = qemuDomainVsockPrivateNew,
     .graphicsNew = qemuDomainGraphicsPrivateNew,
     .networkNew = qemuDomainNetworkPrivateNew,
+    .networkParse = qemuDomainNetworkPrivateParse,
+    .networkFormat = qemuDomainNetworkPrivateFormat,
     .videoNew = qemuDomainVideoPrivateNew,
     .tpmNew = qemuDomainTPMPrivateNew,
     .tpmParse = qemuDomainTPMPrivateParse,

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index eca5404cdcc376abb96c393b8fe2980b5e2655b0..1053d1d4cb6743c212b1350801ec74f1c5d0801c 100644 (file)
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -412,6 +412,11 @@ typedef struct _qemuDomainNetworkPrivate qemuDomainNetworkPrivate;
 struct _qemuDomainNetworkPrivate {
     virObject parent;
 
+    /* True if the device was created by us. Otherwise we should
+     * avoid removing it. Currently only used for
+     * VIR_DOMAIN_NET_TYPE_DIRECT. */
+    bool created;
+
     qemuSlirp *slirp;
 
     /* file descriptor transfer helpers */

diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index ed2c209167d70a8fa24863d4017889504ccd5bf3..076640cbde178c1ede0e0a2356b4f5f9de0cadc6 100644 (file)
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -268,6 +268,7 @@ qemuInterfaceDirectConnect(virDomainDef *def,
     char *res_ifname = NULL;
     g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
     unsigned int macvlan_create_flags = VIR_NETDEV_MACVLAN_CREATE_WITH_TAP;
+    qemuDomainNetworkPrivate *netpriv = QEMU_DOMAIN_NETWORK_PRIVATE(net);
 
     if (qemuInterfaceIsVnetCompatModel(net))
         macvlan_create_flags |= VIR_NETDEV_MACVLAN_VNET_HDR;
@@ -285,6 +286,8 @@ qemuInterfaceDirectConnect(virDomainDef *def,
                                                macvlan_create_flags) < 0)
         goto cleanup;
 
+    netpriv->created = true;
+
     virDomainAuditNetDevice(def, net, res_ifname, true);
     VIR_FREE(net->ifname);
     net->ifname = res_ifname;

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 9a31a11acc893541d0e761f5f6463dfd394a293c..ff219e0030756a1d13e8dd28329fc2ce4466879a 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8443,11 +8443,13 @@ void qemuProcessStop(virQEMUDriver *driver,
         vport = virDomainNetGetActualVirtPortProfile(net);
         switch (virDomainNetGetActualType(net)) {
         case VIR_DOMAIN_NET_TYPE_DIRECT:
-            virNetDevMacVLanDeleteWithVPortProfile(net->ifname, &net->mac,
-                                                   virDomainNetGetActualDirectDev(net),
-                                                   virDomainNetGetActualDirectMode(net),
-                                                   virDomainNetGetActualVirtPortProfile(net),
-                                                   cfg->stateDir);
+            if (QEMU_DOMAIN_NETWORK_PRIVATE(net)->created) {
+                virNetDevMacVLanDeleteWithVPortProfile(net->ifname, &net->mac,
+                                                       virDomainNetGetActualDirectDev(net),
+                                                       virDomainNetGetActualDirectMode(net),
+                                                       virDomainNetGetActualVirtPortProfile(net),
+                                                       cfg->stateDir);
+            }
             break;
         case VIR_DOMAIN_NET_TYPE_ETHERNET:
             if (net->managed_tap != VIR_TRISTATE_BOOL_NO && net->ifname) {