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 <wujin...@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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
