Re: [libvirt PATCH] conf: properly clear out autogenerated macvtap names when formatting/parsing

On 8/23/20 1:52 AM, Laine Stump wrote: > Back when macvtap support was added in commit 315baab9443 in Feb. 2010 > (libvirt-0.7.7), it was setup to autogenerate a name for the device if > one wasn't supplied, in the pattern "macvtap%d" (or "macvlan%d"), > similar to the way an unspecified standard tap device name will lead > to an autogenerated "vnet%d". > > As a matter of fact, in commit ca1b7cc8e45 added in May 2010, the code > was changed to *always* ignore a supplied device name for macvtap > interfaces by deleting any name immediately during the <interface> > parsing (this was intended to prevent one domain which had failed to > completely start from deleting the macvtap device of another domain > which had subsequently been provided the same device name (this will > seem mildly ironic later). This was later fixed to only clear the > device name when inactive XML was being parsed. HOWEVER - this was > only done if the xml was <interface type='direct'> - autogenerated > names were not cleared for <interface type='network'> (which could > also result in a macvtap device). > > Although the names of "vnetX" tap devices had always been > automatically cleared when parsing <interface> (see commit d1304583d > from July 2008 (!)), at the time macvtap support was added, both vnetX > and macvtapX device names were always included when formatting the > XML. > > Then in commit a8be259d0cc (July 2011, libvirt-0.9.4), <interface> > formatting was changed to also clear out "vnetX" device names during > XML formatting as well. However the same treatment wasn't given to > "macvtapX". > > The result of all this (among other things) was that when a running guest I guess this is a stray sentence ^

> > Now in 2020, there has been a report that a failed migration leads to > the macvtap device of some other unrelated guest on the destination > host losing its network connectivity. It was determined that this was > due to the domain XML in the migration containing a macvtap device > name, e.g. "macvtap0", that was already in use on the > destination. Normally this wouldn't be a problem, because libvirt > would see that the device was already in use, and then find a > different unused name. But in this case, other external problems were > causing the migration to fail prior to selecting a macvtap device and > successfully opening it, and during error recovery, qemuProcessStop() > was called, which went through all def->nets objects and (if they were > macvtap) deleted the device specified in net->ifname; since libvirt > hadn't gotten to the point of replacing the incoming "macvtap0" with > the name of a device it actually created for this guest, that meant > that "macvtap0" was deleted, *even though it was currently in use by a > different guest*! > > Whew! > > So, it turns out that when formatting "migratable" XML, "vnetX" > devices are omitted, just as when formatting "inactive" XML (I'm sure > there's a bit of code somewhere that says "if (migratable) then set > inactive too", but found it was easier to just try it out with "virsh > dumpxml blah --migratable"). By making the code in both interface > parsing and formatting consistent for "vnetX", "macvtapX", and > "macvlanX", we can thus make sure that the autogenerated (and unneeded > / completely *not* wanted) macvtap device name will not be sent with > the migration XML. This way when a migration fails, net->ifname will > be NULL, and libvirt won't have any device to try and delete. > > Signed-off-by: Laine Stump <laine(a)redhat.com&gt; > --- Cool story and good patch.