In qemuProcessStop we explicitly remove the port from the openvswitch bridge after
qemuProcessKill is called. But there is a certain interval of time between
deleting tap device and removing it from bridge. The problem occurs when two vms
start and shutdown with the same name's network interface attached to the same
openvswitch bridge. When one vm with the nic named "vnet0" stopping, it deleted
tap device without timely removing the port from bridge.
At this time, another vm created the tap device named "vnet0" and added port to the
same bridge. Then, the first vm removed the port from the same bridge.
Finally, the tap device of the second vm did not attached to the bridge.
We need to delete the bridge port before deleting the tap device instead of after.
So what's needed is to move the loop in qemuProcessStop that cleans up
network interfaces so that it happens before qemuProcessKill is called.

Signed-off-by: Zhipeng Lu <lu.zhipeng@zte.com.cn>
Signed-off-by: Laine Stump <laine@laine.org>
---
 src/qemu/qemu_process.c | 99 ++++++++++++++++++++++++-------------------------
 1 file changed, 49 insertions(+), 50 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 53170d7..f76ced1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6253,56 +6253,6 @@ void qemuProcessStop(virQEMUDriverPtr driver,
                                         qemuProcessCleanupChardevDevice,
                                         NULL));
 
-
-    /* shut it off for sure */
-    ignore_value(qemuProcessKill(vm,
-                                 VIR_QEMU_PROCESS_KILL_FORCE|
-                                 VIR_QEMU_PROCESS_KILL_NOCHECK));
-
-    qemuDomainCleanupRun(driver, vm);
-
-    /* Stop autodestroy in case guest is restarted */
-    qemuProcessAutoDestroyRemove(driver, vm);
-
-    /* now that we know it's stopped call the hook if present */
-    if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
-        char *xml = qemuDomainDefFormatXML(driver, vm->def, 0);
-
-        /* we can't stop the operation even if the script raised an error */
-        ignore_value(virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
-                                 VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END,
-                                 NULL, xml, NULL));
-        VIR_FREE(xml);
-    }
-
-    /* Reset Security Labels unless caller don't want us to */
-    if (!(flags & VIR_QEMU_PROCESS_STOP_NO_RELABEL))
-        qemuSecurityRestoreAllLabel(driver, vm,
-                                    !!(flags & VIR_QEMU_PROCESS_STOP_MIGRATED));
-
-    qemuSecurityReleaseLabel(driver->securityManager, vm->def);
-
-    for (i = 0; i < vm->def->ndisks; i++) {
-        virDomainDeviceDef dev;
-        virDomainDiskDefPtr disk = vm->def->disks[i];
-
-        dev.type = VIR_DOMAIN_DEVICE_DISK;
-        dev.data.disk = disk;
-        ignore_value(qemuRemoveSharedDevice(driver, &dev, vm->def->name));
-    }
-
-    /* Clear out dynamically assigned labels */
-    for (i = 0; i < vm->def->nseclabels; i++) {
-        if (vm->def->seclabels[i]->type == VIR_DOMAIN_SECLABEL_DYNAMIC)
-            VIR_FREE(vm->def->seclabels[i]->label);
-        VIR_FREE(vm->def->seclabels[i]->imagelabel);
-    }
-
-    virStringListFree(priv->qemuDevices);
-    priv->qemuDevices = NULL;
-
-    qemuHostdevReAttachDomainDevices(driver, vm->def);
-
     def = vm->def;
     for (i = 0; i < def->nnets; i++) {
         virDomainNetDefPtr net = def->nets[i];
@@ -6360,6 +6310,55 @@ void qemuProcessStop(virQEMUDriverPtr driver,
         networkReleaseActualDevice(vm->def, net);
     }
 
+    /* shut it off for sure */
+    ignore_value(qemuProcessKill(vm,
+                                 VIR_QEMU_PROCESS_KILL_FORCE|
+                                 VIR_QEMU_PROCESS_KILL_NOCHECK));
+
+    qemuDomainCleanupRun(driver, vm);
+
+    /* Stop autodestroy in case guest is restarted */
+    qemuProcessAutoDestroyRemove(driver, vm);
+
+    /* now that we know it's stopped call the hook if present */
+    if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
+        char *xml = qemuDomainDefFormatXML(driver, vm->def, 0);
+
+        /* we can't stop the operation even if the script raised an error */
+        ignore_value(virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
+                                 VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END,
+                                 NULL, xml, NULL));
+        VIR_FREE(xml);
+    }
+
+    /* Reset Security Labels unless caller don't want us to */
+    if (!(flags & VIR_QEMU_PROCESS_STOP_NO_RELABEL))
+        qemuSecurityRestoreAllLabel(driver, vm,
+                                    !!(flags & VIR_QEMU_PROCESS_STOP_MIGRATED));
+
+    qemuSecurityReleaseLabel(driver->securityManager, vm->def);
+
+    for (i = 0; i < vm->def->ndisks; i++) {
+        virDomainDeviceDef dev;
+        virDomainDiskDefPtr disk = vm->def->disks[i];
+
+        dev.type = VIR_DOMAIN_DEVICE_DISK;
+        dev.data.disk = disk;
+        ignore_value(qemuRemoveSharedDevice(driver, &dev, vm->def->name));
+    }
+
+    /* Clear out dynamically assigned labels */
+    for (i = 0; i < vm->def->nseclabels; i++) {
+        if (vm->def->seclabels[i]->type == VIR_DOMAIN_SECLABEL_DYNAMIC)
+            VIR_FREE(vm->def->seclabels[i]->label);
+        VIR_FREE(vm->def->seclabels[i]->imagelabel);
+    }
+
+    virStringListFree(priv->qemuDevices);
+    priv->qemuDevices = NULL;
+
+    qemuHostdevReAttachDomainDevices(driver, vm->def);
+
  retry:
     if ((ret = qemuRemoveCgroup(vm)) < 0) {
         if (ret == -EBUSY && (retries++ < 5)) {
-- 
1.8.3.1


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

On Mon, May 08, 2017 at 03:03:30PM +0800, ZhiPeng Lu wrote:
> In qemuProcessStop we explicitly remove the port from the openvswitch bridge after
> qemuProcessKill is called. But there is a certain interval of time between
> deleting tap device and removing it from bridge. The problem occurs when two vms
> start and shutdown with the same name's network interface attached to the same
> openvswitch bridge. When one vm with the nic named "vnet0" stopping, it deleted
> tap device without timely removing the port from bridge.
> At this time, another vm created the tap device named "vnet0" and added port to the
> same bridge. Then, the first vm removed the port from the same bridge.
> Finally, the tap device of the second vm did not attached to the bridge.
> We need to delete the bridge port before deleting the tap device instead of after.
> So what's needed is to move the loop in qemuProcessStop that cleans up
> network interfaces so that it happens before qemuProcessKill is called.

This fix won't work correctly either. You cannot assume that libvirt has
control over when the QEMU process exits. It may exit itself *before*
libvirt runs any of its cleanup code.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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