[libvirt] [BUG] When PLUG a bridge interface to an active VM, the generated LIVE and CONFIG mac address are different
Hi, When plug a bridge interface to an active VM with both LIVE AND CONFIG flags, libvirt generate different mac address to LIVE and CONFIG instance, so After I reboot the VM, DHCP server doesn't assign the same IP address to the new bridge interface. Steps: 1. Having running VM ## virsh list Id Name State ------------------------------- 1 centos_7.6_64 running 2. Prepare a bridge interface xml ## cat interface.xml <interface type='bridge'> <source bridge='br0-int'/> <virtualport type='openvswitch'> </virtualport> <model type='virtio'/> <target dev='ovsvnet4'/> </interface> 3. Plug interface with --live and --config ##virsh attach-device centos_7.6_64 interface.xml --config --live Device attached successfully 4. Check LIVE XML ## cat /var/run/libvirt/qemu/centos_7.6_64.xml | grep ovsvnet4 -A 4 -B 6 <interface type='bridge'> <mac address='52:54:00:23:7d:fb'/> <source bridge='br0-int'/> <virtualport type='openvswitch'> <parameters interfaceid='c9f07f2b-3888-4f25-bd80-f5ba04d07bf8'/> </virtualport> <target dev='ovsvnet4'/> <model type='virtio'/> <alias name='net0'/> <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> </interface> 5. Check Config XML ## cat /etc/libvirt/qemu/centos_7.6_64.xml | grep ovsvnet4 -A 3 -B 6 <interface type='bridge'> <mac address='52:54:00:bb:d4:53'/> <source bridge='br0-int'/> <virtualport type='openvswitch'> <parameters interfaceid='189234ea-878e-4e8a-9f63-f43fac50aebd'/> </virtualport> <target dev='ovsvnet4'/> <model type='virtio'/> <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> </interface> The mac address and virtualport interfaceid does not match in LIVE and CONFIG XML. This phenomenon was introduced by the commit: commit 55ce65646348884656fd7bf3f109ebf8f7603494 Author: John Ferlan <jferlan@redhat.com> Date: Tue Jun 12 08:44:21 2018 -0400 qemu: Use the correct vm def on cold attach https://bugzilla.redhat.com/show_bug.cgi?id=1559867 When attaching a device to the domain we need to be sure to use the correct domain definition (vm->def or vm->newDef) when calling virDomainDeviceDefParse because the post parse processing algorithms that may assign an address for the device will use whatever domain definition was passed in.
On 8/22/19 10:43 AM, Xu Yandong (Yandong Xu) wrote:
Hi,
When plug a bridge interface to an active VM with both LIVE AND CONFIG flags, libvirt generate different mac address to LIVE and CONFIG instance, so After I reboot the VM, DHCP server doesn't assign the same IP address to the new bridge interface.
This is kind of expected. In general, live and inactive XMLs can be significantly different. And I guess it's out of libvirt's scope to try and autofill missing data in such way that fits both libe and inactive XMLs. For instance, a PCI address can be taken/free in live XML because of hotplug/hotunplug but that might not be the case for inactive XML. Should libvirt try and find a slot that suits both XMLs? Michal
On Tue, Aug 27, 2019 at 02:13:28PM +0200, Michal Privoznik wrote:
On 8/22/19 10:43 AM, Xu Yandong (Yandong Xu) wrote:
Hi,
When plug a bridge interface to an active VM with both LIVE AND CONFIG flags, libvirt generate different mac address to LIVE and CONFIG instance, so After I reboot the VM, DHCP server doesn't assign the same IP address to the new bridge interface.
This is kind of expected. In general, live and inactive XMLs can be significantly different.
Well, if the live and inactive XMLs have sufficiently diverged, then I don't see the point of calling AttachDevice with both AFFECT_LIVE and AFFECT_CONFIG - you might as well use two different API calls. However for a guest with the definitions in sync, quietly accepting a both flags while attaching a device with different MAC addresses (so essentially two different devices) feels wrong.
And I guess it's out of libvirt's scope to try and autofill missing data in such way that fits both libe and inactive XMLs. For instance, a PCI address can be taken/free in live XML because of hotplug/hotunplug but that might not be the case for inactive XML. Should libvirt try and find a slot that suits both XMLs?
That possibly might be out of scope, but autofilling the mac address as early as virDomainNetDefParseXML also is not ideal. Jano
Michal
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 2019/8/27 20:44, Ján Tomko wrote:
On Tue, Aug 27, 2019 at 02:13:28PM +0200, Michal Privoznik wrote:
On 8/22/19 10:43 AM, Xu Yandong (Yandong Xu) wrote:
Hi,
When plug a bridge interface to an active VM with both LIVE AND CONFIG flags, libvirt generate different mac address to LIVE and CONFIG instance, so After I reboot the VM, DHCP server doesn't assign the same IP address to the new bridge interface.
This is kind of expected. In general, live and inactive XMLs can be significantly different.
Well, if the live and inactive XMLs have sufficiently diverged, then I don't see the point of calling AttachDevice with both AFFECT_LIVE and AFFECT_CONFIG - you might as well use two different API calls.
Yes, when the live and inactive XMLs have sufficiently diverged, it's really difficult or even impossible to try to automatically fill in the missing data while applying to both live and inactive XML.
However for a guest with the definitions in sync, quietly accepting a both flags while attaching a device with different MAC addresses (so essentially two different devices) feels wrong.
And I guess it's out of libvirt's scope to try and autofill missing data in such way that fits both libe and inactive XMLs. For instance, a PCI address can be taken/free in live XML because of hotplug/hotunplug but that might not be the case for inactive XML. Should libvirt try and find a slot that suits both XMLs?
That possibly might be out of scope, but autofilling the mac address as early as virDomainNetDefParseXML also is not ideal.
We have pushed a patch bellow that can restore the situation to an older state. Subject: [PATCH] qemu: use the same def when attaching device live and config Patch 55ce6564634 caused a problem fixed in 1e0534a7702, described as below: Some members are generated during XML parse (e.g. MAC address of an interface); However, with current implementation, if we are plugging a device both to persistent and live config, we parse given XML twice: first time for live, second for config. This is wrong then as the second time we are not guaranteed to generate same values as we did for the first time. To solve that, we should use the same def when attaching a device config and live. And leave the current process unchanged when using --config and --live separately. Signed-off-by: Wu Jing <wujing42@huawei.com> --- src/qemu/qemu_driver.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 617d7d5..eca54d0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8615,6 +8615,22 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; + if (flags & VIR_DOMAIN_AFFECT_CONFIG && + flags & VIR_DOMAIN_AFFECT_LIVE) { + /* If we are affecting both CONFIG and LIVE + * use the same xml of device preferentially + * to make the configuration consistent. + */ + devLive = virDomainDeviceDefParse(xml, vm->def, + caps, driver->xmlopt, + parse_flags); + if (!devLive) + goto cleanup; + devConf = virDomainDeviceDefCopy(devLive, vm->def, caps, driver->xmlopt); + if (!devConf) + goto cleanup; + } + /* The config and live post processing address auto-generation algorithms * rely on the correct vm->def or vm->newDef being passed, so call the * device parse based on which definition is in use */ @@ -8623,7 +8639,8 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm, if (!vmdef) goto cleanup; - if (!(devConf = virDomainDeviceDefParse(xml, vmdef, caps, + if (!devConf && + !(devConf = virDomainDeviceDefParse(xml, vmdef, caps, driver->xmlopt, parse_flags))) goto cleanup; @@ -8643,7 +8660,8 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm, } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!(devLive = virDomainDeviceDefParse(xml, vm->def, caps, + if (!devLive && + !(devLive = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, parse_flags))) goto cleanup; -- 2.19.1 Perhaps consider merging that patch? Yandong
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 8/28/19 9:55 AM, Xu Yandong wrote:
On 2019/8/27 20:44, Ján Tomko wrote:
On Tue, Aug 27, 2019 at 02:13:28PM +0200, Michal Privoznik wrote:
On 8/22/19 10:43 AM, Xu Yandong (Yandong Xu) wrote:
Hi,
When plug a bridge interface to an active VM with both LIVE AND CONFIG flags, libvirt generate different mac address to LIVE and CONFIG instance, so After I reboot the VM, DHCP server doesn't assign the same IP address to the new bridge interface.
This is kind of expected. In general, live and inactive XMLs can be significantly different.
Well, if the live and inactive XMLs have sufficiently diverged, then I don't see the point of calling AttachDevice with both AFFECT_LIVE and AFFECT_CONFIG - you might as well use two different API calls.
Yes, when the live and inactive XMLs have sufficiently diverged, it's really difficult or even impossible to try to automatically fill in the missing data while applying to both live and inactive XML.
However for a guest with the definitions in sync, quietly accepting a both flags while attaching a device with different MAC addresses (so essentially two different devices) feels wrong.
And I guess it's out of libvirt's scope to try and autofill missing data in such way that fits both libe and inactive XMLs. For instance, a PCI address can be taken/free in live XML because of hotplug/hotunplug but that might not be the case for inactive XML. Should libvirt try and find a slot that suits both XMLs?
That possibly might be out of scope, but autofilling the mac address as early as virDomainNetDefParseXML also is not ideal.
We have pushed a patch bellow that can restore the situation to an older state.
Subject: [PATCH] qemu: use the same def when attaching device live and config
Patch 55ce6564634 caused a problem fixed in 1e0534a7702, described as below: Some members are generated during XML parse (e.g. MAC address of an interface); However, with current implementation, if we are plugging a device both to persistent and live config, we parse given XML twice: first time for live, second for config. This is wrong then as the second time we are not guaranteed to generate same values as we did for the first time.
To solve that, we should use the same def when attaching a device config and live. And leave the current process unchanged when using --config and --live separately.
Signed-off-by: Wu Jing <wujing42@huawei.com> --- src/qemu/qemu_driver.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 617d7d5..eca54d0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8615,6 +8615,22 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup;
+ if (flags & VIR_DOMAIN_AFFECT_CONFIG && + flags & VIR_DOMAIN_AFFECT_LIVE) { + /* If we are affecting both CONFIG and LIVE + * use the same xml of device preferentially + * to make the configuration consistent. + */ + devLive = virDomainDeviceDefParse(xml, vm->def,
This use of vm->def prefers live XML. This means that for instance PCI address are assigned based on current PCI layout in live XML which in general is different to inactive XML. And it's not only PCI addresses, we autogenerate some out aspects of <interface/> (e.g. model) and even more for other devices. Just take a look at qemuDomainDeviceDefPostParse(). All functions there which take domain def as an argument do so because they are basing autogenerated value on domain definition. Plus there's more in parser code. While your patch might work for your use case, it can break others. BTW: have you read the original commit that caused this? I'm failing to see how we would not re-introduce the problem with your patch. If you're using live XML to validate/generate device addresses, how can we generate valid address for inactive XML? Michal
On 8/27/19 2:44 PM, Ján Tomko wrote:
On Tue, Aug 27, 2019 at 02:13:28PM +0200, Michal Privoznik wrote:
On 8/22/19 10:43 AM, Xu Yandong (Yandong Xu) wrote:
Hi,
When plug a bridge interface to an active VM with both LIVE AND CONFIG flags, libvirt generate different mac address to LIVE and CONFIG instance, so After I reboot the VM, DHCP server doesn't assign the same IP address to the new bridge interface.
This is kind of expected. In general, live and inactive XMLs can be significantly different.
Well, if the live and inactive XMLs have sufficiently diverged, then I don't see the point of calling AttachDevice with both AFFECT_LIVE and AFFECT_CONFIG - you might as well use two different API calls.
However for a guest with the definitions in sync, quietly accepting a both flags while attaching a device with different MAC addresses (so essentially two different devices) feels wrong.
And I guess it's out of libvirt's scope to try and autofill missing data in such way that fits both libe and inactive XMLs. For instance, a PCI address can be taken/free in live XML because of hotplug/hotunplug but that might not be the case for inactive XML. Should libvirt try and find a slot that suits both XMLs?
That possibly might be out of scope, but autofilling the mac address as early as virDomainNetDefParseXML also is not ideal.
I'm failing to see how moving MAC address generation to a post parse callback would solve this issue, sorry. But as I said in the other reply I've just sent, we are autogenerating much more than MAC addresses. And even more so for other devices. I see two ways to fix this: 1) prefer live XML in this case and if the device with autogenerated values is not fit to inactive XML then hopefully qemuDomainAttachDeviceConfig() will fail, or 2) document this behaviour. But aparently, my wish made in 1) is not happening, is it? The original commit 55ce65646348884656fd7bf3f109ebf8f7603494 that caused this issue claims clearly that we would generate the same address for two different devices. And nothing in our code raised the red flag. Therefore, I like 2) more. After all, if you asked libvirt to autofill some values then don't mind if it did so. Michal
participants (4)
-
Ján Tomko -
Michal Privoznik -
Xu Yandong -
Xu Yandong (Yandong Xu)