[PATCH v2 1/4] qemu: Remove interface backend only after frontend is gone

[Jiri Denemark <jdenemar@xxxxxxxxxx>]

[1] reported that we are removing network's backend too early. I didn't
really get the reproducer but libvirt behaves strangely when a guest
does not confirm the removal, e.g., it does not support PCI hotplug. In
such case, detaching a network device leaves its frontend in place but
removes the backend, which makes the device unusable for the guest.
Moreover attaching the same device again succeeds and both the guest and
libvirt will see two network interfaces attached but only one of them is
actually working.

I checked with Paolo Bonzini and he confirmed we should only remove a
backend after seeing DEVICE_DELETED event for a corresponding frontend.

[1] https://www.redhat.com/archives/libvir-list/2014-March/msg01740.html

Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
---

Notes:
    Version 2:
    - return int and propagate errors

 src/qemu/qemu_hotplug.c | 68 +++++++++++++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 31 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0acf18a..4d8d3cd 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2630,25 +2630,55 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
 }
 
 
-static void
+static int
 qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
                           virDomainObjPtr vm,
                           virDomainNetDefPtr net)
 {
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    qemuDomainObjPrivatePtr priv = vm->privateData;
     virNetDevVPortProfilePtr vport;
     virObjectEventPtr event;
+    char *hostnet_name = NULL;
     size_t i;
+    int ret = -1;
 
     if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
         /* this function handles all hostdev and netdev cleanup */
         qemuDomainRemoveHostDevice(driver, vm, virDomainNetGetActualHostdev(net));
+        ret = 0;
         goto cleanup;
     }
 
     VIR_DEBUG("Removing network interface %s from domain %p %s",
               net->info.alias, vm, vm->def->name);
 
+    if (virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0)
+        goto cleanup;
+
+    qemuDomainObjEnterMonitor(driver, vm);
+    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV) &&
+        virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
+        if (qemuMonitorRemoveNetdev(priv->mon, hostnet_name) < 0) {
+            qemuDomainObjExitMonitor(driver, vm);
+            virDomainAuditNet(vm, net, NULL, "detach", false);
+            goto cleanup;
+        }
+    } else {
+        int vlan;
+        if ((vlan = qemuDomainNetVLAN(net)) < 0 ||
+            qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) {
+            if (vlan < 0) {
+                virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+                               _("unable to determine original VLAN"));
+            }
+            qemuDomainObjExitMonitor(driver, vm);
+            virDomainAuditNet(vm, net, NULL, "detach", false);
+            goto cleanup;
+        }
+    }
+    qemuDomainObjExitMonitor(driver, vm);
+
     virDomainAuditNet(vm, net, NULL, "detach", true);
 
     event = virDomainEventDeviceRemovedNewFromObj(vm, net->info.alias);
@@ -2689,9 +2719,12 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
 
     networkReleaseActualDevice(vm->def, net);
     virDomainNetDefFree(net);
+    ret = 0;
 
  cleanup:
     virObjectUnref(cfg);
+    VIR_FREE(hostnet_name);
+    return ret;
 }
 
 
@@ -3379,8 +3412,6 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
     int detachidx, ret = -1;
     virDomainNetDefPtr detach = NULL;
     qemuDomainObjPrivatePtr priv = vm->privateData;
-    int vlan;
-    char *hostnet_name = NULL;
     int rc;
 
     if ((detachidx = virDomainNetFindIdx(vm->def, dev->data.net)) < 0)
@@ -3418,15 +3449,6 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
         }
     }
 
-    if ((vlan = qemuDomainNetVLAN(detach)) < 0) {
-        virReportError(VIR_ERR_OPERATION_FAILED,
-                       "%s", _("unable to determine original VLAN"));
-        goto cleanup;
-    }
-
-    if (virAsprintf(&hostnet_name, "host%s", detach->info.alias) < 0)
-        goto cleanup;
-
     qemuDomainMarkDeviceForRemoval(vm, &detach->info);
 
     qemuDomainObjEnterMonitor(driver, vm);
@@ -3444,32 +3466,16 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
             goto cleanup;
         }
     }
-
-    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV) &&
-        virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
-        if (qemuMonitorRemoveNetdev(priv->mon, hostnet_name) < 0) {
-            qemuDomainObjExitMonitor(driver, vm);
-            virDomainAuditNet(vm, detach, NULL, "detach", false);
-            goto cleanup;
-        }
-    } else {
-        if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) {
-            qemuDomainObjExitMonitor(driver, vm);
-            virDomainAuditNet(vm, detach, NULL, "detach", false);
-            goto cleanup;
-        }
-    }
     qemuDomainObjExitMonitor(driver, vm);
 
     rc = qemuDomainWaitForDeviceRemoval(vm);
     if (rc == 0 || rc == 1)
-        qemuDomainRemoveNetDevice(driver, vm, detach);
-
-    ret = 0;
+        ret = qemuDomainRemoveNetDevice(driver, vm, detach);
+    else
+        ret = 0;
 
  cleanup:
     qemuDomainResetDeviceRemoval(vm);
-    VIR_FREE(hostnet_name);
     return ret;
 }
 
-- 
2.0.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list