On 10/14/2012 11:23 AM, Laine Stump wrote: > (V1 of this patch is in the following thread: > > https://www.redhat.com/archives/libvir-list/2012-October/msg00542.html > > I'm posting it because PATCH 4/4 of that series attempts to use > apparently non-existent/non-working functionality in qemu; modifying > PATCH 3/4 slightly gives back some of the functionality lost by not > having 4/4. Once this is ACKed, I will push 1-3 only, and leave 4/4 > untilits problem is discovered/resolved) > > **** > > This patch resolves: > > https://bugzilla.redhat.com/show_bug.cgi?id=805071 > > to the extent that it can be resolved with current qemu functionality. > It attempts to detect as many situations as possible when the simple > operation of disconnecting an existing tap device from one bridge and > attaching it to another will satisfy the change requested in > virDomainUpdateDeviceFlags() for a network device. Before this patch, > that situation could only be detected if the pre-change interface > *and* the post-change interface definition were both "type='bridge'". > After this patch, it can also be detected if the before or after > interfaces are any combination of type='bridge' and type='network' > (the networks can be <forward mode='nat|route|bridge'>, as long as > they use a Linux host bridge and not macvtap connections). > > This extra effort is especially useful since the recent discovery that > a netdev_del+netdev_add combo (to reconnect the network device with > completely different hostside configuration) doesn't work properly > with current qemu (1.2) unless it is accompanied by the matching > device_del+device_add - see this mailing list message for details: > > http://lists.nongnu.org/archive/html/qemu-devel/2012-10/msg02355.html > > (A slight modification of the patch referenced there has been prepared > to apply on top of this patch, but won't be pushed until qemu can be > made to work with it.) > > * qemuDomainChangeNet needs access to the virDomainDeviceDef that > holds the new netdef (so that it can clear out the virDomainDeviceDef > if it ends up using the NetDef to replace the original), so the > virDomainNetDefPtr arg is replaced with a virDomainDeviceDefPtr. > > * qemuDomainChangeNet previously checked for *some* changes to the > interface config, but this check was by no means complete. It was also > a bit disorganized. > > This refactoring of the code is (I believe) complete in its check of > all NetDef attributes that might be changed, and either returns a > failure (for changes that are simply impossible), or sets one of three > flags: > > needLinkStateChange - if the device link state needs to go up/down > needBridgeChange - if everything else is the same, but it needs > to be connected to a difference linux host > bridge > needReconnect - if the entire host side of the device needs > to be torn down and reconstructed (currently > non-working, as mentioned above) > > Note that this function will refuse to make any change that requires > the *guest* side of the device to be detached (e.g. changing the PCI > address or mac address). Those would be disruptive enough to the guest > that it's reasonable to require an explicit detach/attach sequence > from the management application. > > * As mentioned above, qemuDomainChangeNet also does its best to > understand when a simple change in attached bridge for the existing > tap device will work vs. the need to completely tear down/reconstruct > the host side of the device (including tap device). > > This patch *does not* implement the "reconnect" code anyway - there is > a placeholder that turns that into an error. Rather, the purpose of > this patch is to replicate existing behavior with code that is ready > to have that functionality plugged in in a later patch. > > * The expanded uses for qemuDomainChangeNetBridge meant that it needed s/it/it's/ > to be enhanced as well - it no longer replaces the original brname > string in olddev with the new brname; instead, it relies on the > caller to replace the *entire* olddev with newdev (since we've gone > to great lengths to assure they are functionally identical other > than the name of the bridge, this is now not only safe, but more > correct). Additionally, qemuDomainNetChangeBridge can now set the > bridge for type='network' interfaces as well as plain type='bridge' > interfaces. (Note that I had to make this change simultaneous to the > reorganization of qemuDomainChangeNet because the two are too > closely intertwined to separate). > --- > src/qemu/qemu_driver.c | 2 +- > src/qemu/qemu_hotplug.c | 502 ++++++++++++++++++++++++++++++++++++++---------- > src/qemu/qemu_hotplug.h | 4 +- > 3 files changed, 401 insertions(+), 107 deletions(-) > [...] > + if (olddev->type == newdev->type && oldType == newType) { > > + /* if type hasn't changed, check the relevant fields for the type */ > + switch (newdev->type) { > + case VIR_DOMAIN_NET_TYPE_USER: > + break; > + > + case VIR_DOMAIN_NET_TYPE_ETHERNET: > + if (STRNEQ_NULLABLE(olddev->data.ethernet.dev, > + newdev->data.ethernet.dev) || > + STRNEQ_NULLABLE(olddev->script, newdev->script) || No need to check this, it was checked few lines before, but no harm done here. Everything looks good, so ACK. Martin -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list