[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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

> On Tue, Feb 16, 2010 at 5:22 AM, Stefan Berger <stefanb 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.


> 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 us ibm com>

> --Ed

Attachment: macvtap_virsh_destroy.diff
Description: Binary data

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]