On Sun, Jan 19, 2020 at 10:24:19PM -0500, Laine Stump wrote: > Current virtio-net drivers that support the failover feature match up > the virtio backup device with its corresponding hostdev device by > looking for an interface with a matching MAC address. Since libvirt > will assign a different random MAC address to each interface that > isn't given an explicit MAC address, this means that the configuration > for any failover pairs must include explicit matching MAC addresses. > > To make life easier, we could have libvirt populate the XML config > with the same auto-generated MAC address for both interfaces when it > detects a failover pair that have no MAC addresses provided (a > failover pair can be detected by matching <alias name='blah'/> of the > virtio interface with <driver backupAlias='blah'/> of the hostdev > interface), but then we would be stuck with that behavior even if the > virtio guest driver later eliminated the requirement that mac > addresses match. > > Additionally, some management software uses the MAC address as the > primary index for its list of network devices, and can't properly deal > with two interfaces having the same MAC address (oVirt). Even > libvirt's own virsh utility uses MAC address (combined with interface > type) to uniquely identify interfaces for the virsh detach-interface > command (in this case, fortunately the runtime interface type is used, > so one of the interfaces will always be of type='hostdev' and the > other type='something-else", so it doesn't currently cause any problem). > > In order to remove the necessity of explicitly setting interface MAC > addresses, as well as permit the two interfaces of a failover pair to > each have a unique index while still fulfilling the current guest > driver requirement that the MAC addresses matchin the guest, this > patch adds a new attribute "useBackupMAC" that is set on the hostdev > interface of the pair. When useBackupMAC='yes', the setup for the > hostdev interface will find the virtio failover interface (using > backupAlias) and use that interface's MAC address to initialize the > MAC address of the hostdev interface; the MAC address in the hostdev > interface config remains unchanged, it just isnt used for device > initialization. > > I made this patch to followup on > https://bugzilla.redhat.com/show_bug.cgi?id=1693587#c12 (where I > suggested this attribute as a possible remedy to oVirt's requirement > that each network device have a unique MAC address). > > Truthfully, I'm not convinced that I want it though, as it seems > "a bit" hackish. In particular, I've thought for a long time that the > "hostdev manager" code in util/virhostdev.c should really live in the > node device driver and be called from the hypervisors via a public API > (so that there is one central place on the host that maintains the > list of in-use PCI devices and their status), but this patch adds an > obstacle to that goal by adding a virDomainDefPtr to more of the APIs in > that library - if this was turned into a public API, then entire > virDomainDef would need to be serialized and sent in the API call, > then parsed at the other end - yuck :-/. NB: there are already > functions in virhostdev.h that take a virDomainDefPtr, so maybe I'm > being too sensitive. > > On the upside, it solves a problem, and default bevahior is unchanged. I don't believe it does solve a real problem. If a mgmt app is capable of setting useBackupMAC=yes when writing the XML doc, then I don't see why it cannot just as easily set the matching MAC address when wring the XML doc. It can still treat both NICs as having a different MAC address in its own internal code. All it has to do is use the matching MAC address when writing out the XML config it gives to libvirt. I know oVirt has a facility for hook scripts that are add-ons which can arbitrarily munge the XML that VDSM creates. So AFAICT this doesn't even need to involve changes to VDSM itself. There can be a hook script which looks for the config indicating a failover pair, and rewrites the XML to have the matching MAC addr. Such a workaround then only needs to exist for as long as the mgmt app has this problematic limitation without impacting libvirt's maint. So I don't want to take this application specific hack in libvirt. 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 :|
