qemu: change the name of tap device for a tap and bridge network

[libvirt] [PATCH] qemu: change the name of tap device for a tap and bridge network

Posted by ZhiPeng Lu 6 years, 11 months ago

Creating tap device and adding the device to bridge are not atomic operation.
Similarly deleting tap device and removing it from bridge are not atomic operation.
The Problem occurs when two vms start and shutdown. When one vm with the nic
named "vnet0" stopping, it deleted tap device but not removing 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 deleted the tap device from the same bridge.
Finally, the tap device of the second vm don't attached to the bridge.
So, we can add domid to vm's nic name. For example, the vm's domid is 1 and vnet0
is renamed to vnet1.0.

Signed-off-by: ZhiPeng Lu <lu.zhipeng@zte.com.cn>
---
 src/qemu/qemu_interface.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index d0850c0..17d40a7 100644
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -512,6 +512,7 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def,
     bool template_ifname = false;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
     const char *tunpath = "/dev/net/tun";
+    char *domIdPlusIndex = NULL;
 
     if (net->backend.tap) {
         tunpath = net->backend.tap;
@@ -531,8 +532,13 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def,
         STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) ||
         strchr(net->ifname, '%')) {
         VIR_FREE(net->ifname);
-        if (VIR_STRDUP(net->ifname, VIR_NET_GENERATED_PREFIX "%d") < 0)
+        if (virAsprintf(&domIdPlusIndex, "%s%d.%s",
+           VIR_NET_GENERATED_PREFIX, def->id, "%d") < 0) {
+            goto cleanup;
+        }
+        if (VIR_STRDUP(net->ifname, domIdPlusIndex) < 0) {
             goto cleanup;
+        }
         /* avoid exposing vnet%d in getXMLDesc or error outputs */
         template_ifname = true;
     }
@@ -594,6 +600,7 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def,
     ret = 0;
 
  cleanup:
+    VIR_FREE(domIdPlusIndex);
     if (ret < 0) {
         size_t i;
         for (i = 0; i < *tapfdSize && tapfd[i] >= 0; i++)
-- 
1.8.3.1


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

Re: [libvirt] [PATCH] qemu: change the name of tap device for a tap and bridge network

Posted by Daniel P. Berrange 6 years, 11 months ago

On Fri, Apr 28, 2017 at 05:23:19PM +0800, ZhiPeng Lu wrote:
> Creating tap device and adding the device to bridge are not atomic operation.
> Similarly deleting tap device and removing it from bridge are not atomic operation.
> The Problem occurs when two vms start and shutdown. When one vm with the nic
> named "vnet0" stopping, it deleted tap device but not removing 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 deleted the tap device from the same bridge.
> Finally, the tap device of the second vm don't attached to the bridge.
> So, we can add domid to vm's nic name. For example, the vm's domid is 1 and vnet0
> is renamed to vnet1.0.

Surely deleting the NIC automatically removes it from the bridge so we
can just remove the code that delets the bridge port.


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

Re: [libvirt] [PATCH] qemu: change the name of tap device for a tap and bridge network

Posted by Laine Stump 6 years, 11 months ago

On 04/28/2017 05:33 AM, Daniel P. Berrange wrote:
> On Fri, Apr 28, 2017 at 05:23:19PM +0800, ZhiPeng Lu wrote:
>> Creating tap device and adding the device to bridge are not atomic operation.
>> Similarly deleting tap device and removing it from bridge are not atomic operation.
>> The Problem occurs when two vms start and shutdown. When one vm with the nic
>> named "vnet0" stopping, it deleted tap device but not removing 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 deleted the tap device from the same bridge.
>> Finally, the tap device of the second vm don't attached to the bridge.
>> So, we can add domid to vm's nic name. For example, the vm's domid is 1 and vnet0
>> is renamed to vnet1.0.
> 
> Surely deleting the NIC automatically removes it from the bridge so we
> can just remove the code that delets the bridge port.

That is true when using a Linux host bridge, but I recall that for
openvswitch (which I think is what ZhiPeng is using, based on an earlier
patch), you must explicitly remove the port from the bridge - apparently
the port is still there in openvswitch's table as some sort of "zombie"
connection even after the tap device itself no longer exists.

But instead of changing the naming scheme, maybe we should just delete
the bridge port *before* deleting the tap device instead of after. (Am I
recalling correctly that the tap device is deleted automatically when
the qemu process is killed? If so, then what's needed is to move the
loop in qemuProcessStop() that cleans up network interfaces so that it
happens before qemuProcessKill() is called.

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

Re: [libvirt] [PATCH] qemu: change the name of tap device for a tap and bridge network

Posted by Daniel P. Berrange 6 years, 11 months ago

On Fri, Apr 28, 2017 at 01:08:45PM -0400, Laine Stump wrote:
> On 04/28/2017 05:33 AM, Daniel P. Berrange wrote:
> > On Fri, Apr 28, 2017 at 05:23:19PM +0800, ZhiPeng Lu wrote:
> >> Creating tap device and adding the device to bridge are not atomic operation.
> >> Similarly deleting tap device and removing it from bridge are not atomic operation.
> >> The Problem occurs when two vms start and shutdown. When one vm with the nic
> >> named "vnet0" stopping, it deleted tap device but not removing 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 deleted the tap device from the same bridge.
> >> Finally, the tap device of the second vm don't attached to the bridge.
> >> So, we can add domid to vm's nic name. For example, the vm's domid is 1 and vnet0
> >> is renamed to vnet1.0.
> > 
> > Surely deleting the NIC automatically removes it from the bridge so we
> > can just remove the code that delets the bridge port.
> 
> That is true when using a Linux host bridge, but I recall that for
> openvswitch (which I think is what ZhiPeng is using, based on an earlier
> patch), you must explicitly remove the port from the bridge - apparently
> the port is still there in openvswitch's table as some sort of "zombie"
> connection even after the tap device itself no longer exists.
> 
> 
> But instead of changing the naming scheme, maybe we should just delete
> the bridge port *before* deleting the tap device instead of after. (Am I
> recalling correctly that the tap device is deleted automatically when
> the qemu process is killed? If so, then what's needed is to move the
> loop in qemuProcessStop() that cleans up network interfaces so that it
> happens before qemuProcessKill() is called.

Agreed, we should just reverse the order.

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