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.

Additionally, some devices (SCSI hostdev and SCSI disk) use
algorithms that rely on knowing what already exists of the
other type when generating the new device's address. Using
the wrong VM definition could result in duplicated addresses.

In the case of the bz, two hostdev's with no domain address
provided were added to the running domain's config only.
However, the parsing algorithm used the live domain in
order to figure out the host device address resulting in
the same address being used and a subsequent start failing
due to duplicate address.

Fix this by separating the checks/code into CONFIG and LIVE
processing using the correct definition for each block and
performing cleanup for both options as necessary.

Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
This commit is contained in:
John Ferlan 2018-06-12 08:44:21 -04:00
parent 7564daca8a
commit 55ce656463

View File

@ -8483,7 +8483,8 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
{ {
virDomainDefPtr vmdef = NULL; virDomainDefPtr vmdef = NULL;
virQEMUDriverConfigPtr cfg = NULL; virQEMUDriverConfigPtr cfg = NULL;
virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; virDomainDeviceDefPtr devConf = NULL;
virDomainDeviceDefPtr devLive = NULL;
int ret = -1; int ret = -1;
virCapsPtr caps = NULL; virCapsPtr caps = NULL;
unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
@ -8497,49 +8498,43 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
if (!(caps = virQEMUDriverGetCapabilities(driver, false))) if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup; goto cleanup;
dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, /* The config and live post processing address auto-generation algorithms
caps, driver->xmlopt, * rely on the correct vm->def or vm->newDef being passed, so call the
parse_flags); * device parse based on which definition is in use */
if (dev == NULL)
goto cleanup;
if (virDomainDeviceValidateAliasForHotplug(vm, dev, flags) < 0)
goto cleanup;
if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
flags & VIR_DOMAIN_AFFECT_LIVE) {
/* If we are affecting both CONFIG and LIVE
* create a deep copy of device as adding
* to CONFIG takes one instance.
*/
dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt);
if (!dev_copy)
goto cleanup;
}
if (flags & VIR_DOMAIN_AFFECT_CONFIG) { if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
/* Make a copy for updated domain. */
vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt); vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
if (!vmdef) if (!vmdef)
goto cleanup; goto cleanup;
if (virDomainDefCompatibleDevice(vmdef, dev, NULL, if (!(devConf = virDomainDeviceDefParse(xml, vmdef, caps,
driver->xmlopt, parse_flags)))
goto cleanup;
if (virDomainDefCompatibleDevice(vmdef, devConf, NULL,
VIR_DOMAIN_DEVICE_ACTION_ATTACH, VIR_DOMAIN_DEVICE_ACTION_ATTACH,
false) < 0) false) < 0)
goto cleanup; goto cleanup;
if ((ret = qemuDomainAttachDeviceConfig(vmdef, dev, caps,
if ((ret = qemuDomainAttachDeviceConfig(vmdef, devConf, caps,
parse_flags, parse_flags,
driver->xmlopt)) < 0) driver->xmlopt)) < 0)
goto cleanup; goto cleanup;
} }
if (flags & VIR_DOMAIN_AFFECT_LIVE) { if (flags & VIR_DOMAIN_AFFECT_LIVE) {
if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL, if (!(devLive = virDomainDeviceDefParse(xml, vm->def, caps,
driver->xmlopt, parse_flags)))
goto cleanup;
if (virDomainDeviceValidateAliasForHotplug(vm, devLive, flags) < 0)
goto cleanup;
if (virDomainDefCompatibleDevice(vm->def, devLive, NULL,
VIR_DOMAIN_DEVICE_ACTION_ATTACH, VIR_DOMAIN_DEVICE_ACTION_ATTACH,
true) < 0) true) < 0)
goto cleanup; goto cleanup;
if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, driver)) < 0) if ((ret = qemuDomainAttachDeviceLive(vm, devLive, driver)) < 0)
goto cleanup; goto cleanup;
/* /*
* update domain status forcibly because the domain status may be * update domain status forcibly because the domain status may be
@ -8563,9 +8558,8 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
cleanup: cleanup:
virDomainDefFree(vmdef); virDomainDefFree(vmdef);
if (dev != dev_copy) virDomainDeviceDefFree(devConf);
virDomainDeviceDefFree(dev_copy); virDomainDeviceDefFree(devLive);
virDomainDeviceDefFree(dev);
virObjectUnref(cfg); virObjectUnref(cfg);
virObjectUnref(caps); virObjectUnref(caps);