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