>>>> On 8/22/19 10:43 AM, Xu Yandong (Yandong Xu) wrote: >>> 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 >> >> 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? > This patch is a little different with the original commit, since we only prefer live XML when using LIVE AND CONFIG flags at the same time. Still parse separate device def if attach with only one flag. So this patch is kinda a combination of 1e0534a7702 and 55ce6564634, just to narrow down the problem influence. And I verified as the steps in https://bugzilla.redhat.com/show_bug.cgi?id=1559867, work as expected.