Re: [libvirt] Using Libvirt to change the bridge a virtual network card of a running vm is connected to
I wrote a patch to change the mapping between a virtual bridge interface and the host bridge while the host is up. It's based on commit 6fba577e505611e6c25c68e322942eab7754de7e. The host and the interface definition I used for testing are also attached. I would be glad if the patch could be added to the repo. Patch: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4b60839..f791795 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -40,6 +40,7 @@ #include "qemu_cgroup.h" #include "locking/domain_lock.h" #include "network/bridge_driver.h" +#include "util/virnetdevbridge.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1163,6 +1164,28 @@ static virDomainNetDefPtr qemuDomainFindNet(virDomainObjPtr vm, return NULL; } +int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev, + virDomainNetDefPtr newdev) +{ + const char *oldbridge=olddev->data.bridge.brname; + const char *newbridge=newdev->data.bridge.brname; + VIR_DEBUG("Change bridge for interface %s: %s -> %s", olddev->ifname, oldbridge, newbridge); + if(virNetDevBridgeRemovePort(oldbridge, olddev->ifname)<0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("unable to remove port from bridge")); + return -1; + } + if(virNetDevBridgeAddPort(newbridge, newdev->ifname)<0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("unable to add port to bridge")); + return -1; + } + VIR_FREE(olddev->data.bridge.brname); + olddev->data.bridge.brname=strdup(newbridge); + return 0; +} + + int qemuDomainChangeNetLinkState(struct qemud_driver *driver, virDomainObjPtr vm, virDomainNetDefPtr dev, @@ -1293,6 +1316,12 @@ int qemuDomainChangeNet(struct qemud_driver *driver, return -1; } + if(olddev->type==VIR_DOMAIN_NET_TYPE_BRIDGE + && dev->type==VIR_DOMAIN_NET_TYPE_BRIDGE + && STRNEQ(olddev->data.bridge.brname, dev->data.bridge.brname)) { + qemuDomainChangeNetBridge(olddev, dev); + } + if (olddev->linkstate != dev->linkstate) { if ((ret = qemuDomainChangeNetLinkState(driver, vm, olddev, dev->linkstate)) < 0) return ret; diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 0310361..1e1f75c 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -80,6 +80,8 @@ int qemuDomainChangeNetLinkState(struct qemud_driver *driver, virDomainObjPtr vm, virDomainNetDefPtr dev, int linkstate); +int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev, + virDomainNetDefPtr newdev); int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev); Host definition: <domain type='kvm'> <name>test</name> <memory>32768</memory> <currentMemory>32768</currentMemory> <vcpu>1</vcpu> <os> <type arch='x86_64' machine='pc-0.12'>hvm</type> <boot dev='cdrom'/> <bootmenu enable='no'/> </os> <features> <acpi/> <apic/> <pae/> </features> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> <devices> <emulator>/usr/bin/kvm</emulator> <controller type='ide' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <interface type='bridge'> <mac address='52:54:00:ab:cd:02'/> <source bridge='br0'/> <target dev='testif'/> <link state='up'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='pty'> <target port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> </console> <input type='tablet' bus='usb'/> <input type='mouse' bus='ps2'/> <graphics type='vnc' port='-1' autoport='yes'/> <video> <model type='cirrus' vram='9216' heads='1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </memballoon> </devices> </domain> Interface definition: <interface type="bridge"> <mac address="52:54:00:ab:cd:02"/> <source bridge="br1"/> <target dev="testif"/> <link state="up"/> <alias name="net0"/> <address type="pci" domain="0x0000" bus="0x00" slot="0x03" function="0x0"/> </interface> Best regards Hendrik Schwartke
On Wed, Jan 25, 2012 at 04:09:22PM +0100, Hendrik Schwartke wrote:
I wrote a patch to change the mapping between a virtual bridge interface and the host bridge while the host is up. It's based on commit 6fba577e505611e6c25c68e322942eab7754de7e. The host and the interface definition I used for testing are also attached. I would be glad if the patch could be added to the repo.
Great !
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4b60839..f791795 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -40,6 +40,7 @@ #include "qemu_cgroup.h" #include "locking/domain_lock.h" #include "network/bridge_driver.h" +#include "util/virnetdevbridge.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -1163,6 +1164,28 @@ static virDomainNetDefPtr qemuDomainFindNet(virDomainObjPtr vm, return NULL; }
+int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev, + virDomainNetDefPtr newdev) +{ + const char *oldbridge=olddev->data.bridge.brname; + const char *newbridge=newdev->data.bridge.brname; + VIR_DEBUG("Change bridge for interface %s: %s -> %s", olddev->ifname, oldbridge, newbridge);
+ if(virNetDevBridgeRemovePort(oldbridge, olddev->ifname)<0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("unable to remove port from bridge"));
Good idea to include the bridge device name too
+ return -1; + } + if(virNetDevBridgeAddPort(newbridge, newdev->ifname)<0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("unable to add port to bridge")); + return -1; + }
Hmm, we'd end up with no NIC backend command at all here. So we'd probably want to move the VIR_FREE() line before here, so if we fail half-way through, we still update the XML. To avoid stupid mistakes we also probably want to check for existance of the target bridge before unplugging fromthe source
+ VIR_FREE(olddev->data.bridge.brname); + olddev->data.bridge.brname=strdup(newbridge);
We could just steal newbridge from the newdev and avoid the OOM potential
+ return 0; +}
Various whitespace fixes - For assignment, we prefer 'foo = bar' instead of 'foo=bar' - For conditionals 'if (' instead of 'if(' - For tests "foo < 0" instead of "foo<0" So all in all I think I'd write this as int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev, virDomainNetDefPtr newdev) { const char *oldbridge = olddev->data.bridge.brname; const char *newbridge = newdev->data.bridge.brname; VIR_DEBUG("Change bridge for interface %s: %s -> %s", olddev->ifname, oldbridge, newbridge); if (!virNetDevExists(newbridge)) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("unable to remove port from bridge")); return -1; } if (oldbridge && virNetDevBridgeRemovePort(oldbridge, olddev->ifname) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("unable to remove port from bridge %s"), oldbridge); return -1; } VIR_FREE(olddev->data.bridge.brname); if (virNetDevBridgeAddPort(newbridge, newdev->ifname) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("unable to add port to bridge"), newbridge); return -1; } olddev->data.bridge.brname = newdev->data.bridge.brname; newdev->data.bridge.brname = NULL; return 0; }
+ int qemuDomainChangeNetLinkState(struct qemud_driver *driver, virDomainObjPtr vm, virDomainNetDefPtr dev, @@ -1293,6 +1316,12 @@ int qemuDomainChangeNet(struct qemud_driver *driver, return -1; }
+ if(olddev->type==VIR_DOMAIN_NET_TYPE_BRIDGE + && dev->type==VIR_DOMAIN_NET_TYPE_BRIDGE + && STRNEQ(olddev->data.bridge.brname, dev->data.bridge.brname)) { + qemuDomainChangeNetBridge(olddev, dev); + }
And use STRNEQ_NULLABLE here to avoid risk of NULL brname field, and if qemuDomainChangeNetBridge() returns -1, we need to return to the caller here.
+ if (olddev->linkstate != dev->linkstate) { if ((ret = qemuDomainChangeNetLinkState(driver, vm, olddev, dev->linkstate)) < 0) return ret; diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 0310361..1e1f75c 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -80,6 +80,8 @@ int qemuDomainChangeNetLinkState(struct qemud_driver *driver, virDomainObjPtr vm, virDomainNetDefPtr dev, int linkstate); +int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev, + virDomainNetDefPtr newdev); int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev);
Host definition: <domain type='kvm'> <name>test</name> <memory>32768</memory> <currentMemory>32768</currentMemory> <vcpu>1</vcpu> <os> <type arch='x86_64' machine='pc-0.12'>hvm</type> <boot dev='cdrom'/> <bootmenu enable='no'/> </os> <features> <acpi/> <apic/> <pae/> </features> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> <devices> <emulator>/usr/bin/kvm</emulator> <controller type='ide' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <interface type='bridge'> <mac address='52:54:00:ab:cd:02'/> <source bridge='br0'/> <target dev='testif'/> <link state='up'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='pty'> <target port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> </console> <input type='tablet' bus='usb'/> <input type='mouse' bus='ps2'/> <graphics type='vnc' port='-1' autoport='yes'/> <video> <model type='cirrus' vram='9216' heads='1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </memballoon> </devices> </domain>
Interface definition: <interface type="bridge"> <mac address="52:54:00:ab:cd:02"/> <source bridge="br1"/> <target dev="testif"/> <link state="up"/> <alias name="net0"/> <address type="pci" domain="0x0000" bus="0x00" slot="0x03" function="0x0"/> </interface>
Best regards Hendrik Schwartke
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 01/26/2012 04:10 AM, Daniel P. Berrange wrote:
On Wed, Jan 25, 2012 at 04:09:22PM +0100, Hendrik Schwartke wrote:
I wrote a patch to change the mapping between a virtual bridge interface and the host bridge while the host is up. It's based on commit 6fba577e505611e6c25c68e322942eab7754de7e. The host and the interface definition I used for testing are also attached. I would be glad if the patch could be added to the repo.
Great !
...
Various whitespace fixes
- For assignment, we prefer 'foo = bar' instead of 'foo=bar' - For conditionals 'if (' instead of 'if(' - For tests "foo < 0" instead of "foo<0"
So all in all I think I'd write this as
int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev, virDomainNetDefPtr newdev) {
... Are you planning on resubmitting this patch with the noted improvements? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
I was sure that I did that before. Apparently I was wrong. Sorry for that. diff --git a/.gnulib b/.gnulib index d5612c7..e9e8aba 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit d5612c714c87555f1059d71d347e20271dced322 +Subproject commit e9e8aba12af3c903edd422fa036a356c5b2f313a diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1e56354..de54710 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -40,6 +40,8 @@ #include "qemu_cgroup.h" #include "locking/domain_lock.h" #include "network/bridge_driver.h" +#include "util/virnetdev.h" +#include "util/virnetdevbridge.h" #include "virnetdevtap.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1191,6 +1193,44 @@ static virDomainNetDefPtr qemuDomainFindNet(virDomainObjPtr vm, return NULL; } +int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev, + virDomainNetDefPtr newdev) +{ + const char *oldbridge = olddev->data.bridge.brname; + const char *newbridge = newdev->data.bridge.brname; + + VIR_DEBUG("Change bridge for interface %s: %s -> %s", + olddev->ifname, oldbridge, newbridge); + + if (!virNetDevExists(newbridge)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("unable to remove port from bridge")); + return -1; + } + + if (oldbridge&& + virNetDevBridgeRemovePort(oldbridge, olddev->ifname)< 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("unable to remove port from bridge %s"), oldbridge); + return -1; + } + if (virNetDevBridgeAddPort(newbridge, newdev->ifname)< 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("unable to add port to bridge %s"), newbridge); + if (virNetDevBridgeAddPort(oldbridge, olddev->ifname)< 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("unable to recover former state by adding port to bridge %s"), oldbridge); + VIR_FREE(olddev->data.bridge.brname); + } + return -1; + } + VIR_FREE(olddev->data.bridge.brname); + olddev->data.bridge.brname = newdev->data.bridge.brname; + newdev->data.bridge.brname = NULL; + return 0; +} + + int qemuDomainChangeNetLinkState(struct qemud_driver *driver, virDomainObjPtr vm, virDomainNetDefPtr dev, @@ -1321,6 +1361,13 @@ int qemuDomainChangeNet(struct qemud_driver *driver, return -1; } + if (olddev->type == VIR_DOMAIN_NET_TYPE_BRIDGE +&& dev->type == VIR_DOMAIN_NET_TYPE_BRIDGE +&& STRNEQ_NULLABLE(olddev->data.bridge.brname, dev->data.bridge.brname)) { + if ((ret = qemuDomainChangeNetBridge(olddev, dev))< 0) + return ret; + } + if (olddev->linkstate != dev->linkstate) { if ((ret = qemuDomainChangeNetLinkState(driver, vm, olddev, dev->linkstate))< 0) return ret; diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 0310361..1e1f75c 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -80,6 +80,8 @@ int qemuDomainChangeNetLinkState(struct qemud_driver *driver, virDomainObjPtr vm, virDomainNetDefPtr dev, int linkstate); +int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev, + virDomainNetDefPtr newdev); int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev); On 16.03.2012 00:06, Eric Blake wrote:
On 01/26/2012 04:10 AM, Daniel P. Berrange wrote:
On Wed, Jan 25, 2012 at 04:09:22PM +0100, Hendrik Schwartke wrote:
I wrote a patch to change the mapping between a virtual bridge interface and the host bridge while the host is up. It's based on commit 6fba577e505611e6c25c68e322942eab7754de7e. The host and the interface definition I used for testing are also attached. I would be glad if the patch could be added to the repo. Great !
...
Various whitespace fixes
- For assignment, we prefer 'foo = bar' instead of 'foo=bar' - For conditionals 'if (' instead of 'if(' - For tests "foo< 0" instead of "foo<0"
So all in all I think I'd write this as
int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev, virDomainNetDefPtr newdev) { ...
Are you planning on resubmitting this patch with the noted improvements?
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Hendrik Schwartke