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(a)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