On 8/28/19 11:42 AM, wujing (O) wrote:
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.

Well, then try to coldplug <hostdev mode='subsystem' type='scsi'/> (that is only to inactive XML), and then try hotplug different <hostdev type'scsi'/> to both live and inactive XMLs and the bug will reproduce. Problem is, that inactive XML contains a device which is not contained in live XML and thus when generating device address based solely on live XML a conflicting address is generated.

If we'd prefer inactive XML in your patch then the steps in my reproducer need to be swapped. At any rate, the issue is still there.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to