[libvirt] [PATCH] Fix macvtap device tear down problem on virsh destroy

[Stefan Berger]

Ed Swierk <eswierk at aristanetworks.com> wrote on 02/17/2010 02:05:44 AM:

> 
> On Tue, Feb 16, 2010 at 5:22 AM, Stefan Berger <stefanb at us.ibm.com> 
wrote:
> > As an RFC, now attached the patch that tears down the macvtap device 
upon
> > termination of the Qemu process without checking whether the device(s)
> > with the given MAC address are in use.
> 
> Another problem comes to mind: it is incorrect to assume that an
> existing macvtap interface with a given MAC address has anything to do
> with the one we want to delete. Consider a host with two NICs, eth0
> and eth1. Suppose some application previously created macvtap0 with
> MAC 52:54:00:12:34:56 from lower interface eth0. Now we want to create
> macvtap1 with the same MAC address 52:54:00:12:34:56 from lower
> interface eth1; this is fine as long as eth0 and eth1 are on different
> LANs or VLANs. We would be unhappy if libvirt blew away macvtap0
> before creating macvtap1.

Right.

> 
> This can be solved by comparing not only the MAC address but also the
> lower interface name: if they both match, then it's safe to delete the
> macvtap interface. It should be possible to query the name of the
> lower interface for a given macvtap interface via netlink (ip link
> show reveals this information).

I now took the route through reading the necessary information from sysfs 
rather than going
through what seems to be the more costly netlink route in terms of code 
that needs to be written.
The new patch is attached. I still tear down all unused macvtap interfaces 
that happen
to have the same MAC address and link device, but test for whether one of 
them is in use if requested.
I assume that those interfaces with the same properties, as the one to be 
created, are stale.

> 
> I'd stick with just one function delMacvtap(char *macaddress, char
> *linkdev, int checkTapInUse), which can be called from
> qemudPhysIfaceConnect(), qemudDomainDetachNetDevice() and
> qemudShutdownVMDaemon(). I don't see any reason for a separate
> hasBusyDev parameter; it seems sufficient to return 1 if the tap
> device is in use.
> 

Thanks for the hints and feedback.

Signed-off-by: Stefan Berger <stefanb at us.ibm.com>



> --Ed
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100217/2842785c/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: macvtap_virsh_destroy.diff
Type: application/octet-stream
Size: 8202 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100217/2842785c/attachment-0001.obj>