[libvirt] [PATCH 7/7] Enable PCI Multifunction hotplug/unplug

The curent patch just assumes the user has given the correct target PCI bus/slot/function address for the device in the xml. Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com&gt; --- src/qemu/qemu_driver.c | 331 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 263 insertions(+), 68 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 37d970e..883994e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8191,6 +8191,117 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, return 0; } +static bool +qemuDomainPCIAddressIsSingleFunctionAddr(virDomainDeviceDefPtr dev) +{ + + virDomainDeviceInfoPtr info = NULL; + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + info = &dev->data.disk->info; + break; + case VIR_DOMAIN_DEVICE_NET: + info = &dev->data.net->info; + break; + case VIR_DOMAIN_DEVICE_RNG: + info = &dev->data.rng->info; + break; + case VIR_DOMAIN_DEVICE_HOSTDEV: + info = dev->data.hostdev->info; + break; + case VIR_DOMAIN_DEVICE_CONTROLLER: + info = &dev->data.controller->info; + break; + case VIR_DOMAIN_DEVICE_CHR: + info = &dev->data.chr->info; + break; + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_LAST: + break; + } + + if (info && info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + /* We do not support hotplug multi-function PCI device now, so we should + * reserve the whole slot. The function of the PCI device must be 0. + */ + if (info->addr.pci.function != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Single function device addresses with function=0" + " expected")); + return false; + } + } + return true; +} + +static bool isMultifunctionDeviceXML(const char *xml) +{ + xmlDocPtr xmlptr; + + if (!(xmlptr = virXMLParse(NULL, xml, _("(device_definition)")))) { + /* We report error anyway later */ + return false; + } + + return STREQ((const char *)(xmlDocGetRootElement(xmlptr))->name, "devices"); +} + +/* + * This function focusses on preparations that have dependencies across + * functions. The hostdevice cant be hotplugged if any of other functions + * are bound to hostdriver. + * + * Any known such preparations can be handled here. + */ +static int +qemuDomainPreparePCIMultifunctionDevices(virQEMUDriverPtr driver, + virDomainDefPtr def, + virDomainDeviceDefListPtr devlist, + virQEMUCapsPtr qemuCaps) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + unsigned int flags = 0; + size_t i = 0, j = 0; + int ret = -1; + + if (devlist->devs[0]->type != VIR_DOMAIN_DEVICE_HOSTDEV) + return 0; + + if (!cfg->relaxedACS) + flags |= VIR_HOSTDEV_STRICT_ACS_CHECK; + + for (i = 0; i < devlist->count; i++) + if (qemuHostdevPreparePCIDevices(driver, def->name, def->uuid, + &devlist->devs[i]->data.hostdev, + 1, qemuCaps, flags) < 0) + goto revert; + + ret = 0; + exit: + return ret; + revert: + while (j < i) + qemuHostdevReAttachPCIDevices(driver, def->name, + &devlist->devs[j++]->data.hostdev, 1); + + goto exit; +} static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) @@ -8198,7 +8309,7 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; - virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; int ret = -1; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; @@ -8207,6 +8318,11 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + virDomainDeviceDefPtr dev = NULL; + bool multifunction = false; + size_t i; + virDomainDeviceAction actionFlag = VIR_DOMAIN_DEVICE_ACTION_ATTACH; + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -8231,50 +8347,79 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - parse_flags); - if (dev == NULL) + if (priv->qemuCaps) + qemuCaps = virObjectRef(priv->qemuCaps); + else if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, vm->def->emulator))) + goto endjob; + + multifunction = isMultifunctionDeviceXML(xml); + + if (VIR_ALLOC(devlist) < 0) goto endjob; + if (multifunction) { + if (virDomainPCIMultifunctionDeviceDefParseNode(xml, vm->def, devlist, + caps, driver->xmlopt, + parse_flags) < 0) + goto endjob; + + if (virDomainPCIMultifunctionDeviceAddressAssign(priv->pciaddrs, devlist) < 0) + goto endjob; + + if (qemuDomainPreparePCIMultifunctionDevices(driver, vm->def, + devlist, qemuCaps) < 0) + goto endjob; + + } else { + dev = virDomainDeviceDefParse(xml, vm->def, + caps, driver->xmlopt, + parse_flags); + if (!dev || !qemuDomainPCIAddressIsSingleFunctionAddr(dev) || + virDomainDeviceDefListAddCopy(devlist, dev, vm->def, caps, + driver->xmlopt) < 0) + goto endjob; + } + + devcopylist = devlist; + 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) + if (VIR_ALLOC(devcopylist) < 0) goto endjob; + for (i = 0; i < devlist->count; i++) + if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[i], + vm->def, caps, driver->xmlopt) < 0) + goto endjob; } - if (priv->qemuCaps) - qemuCaps = virObjectRef(priv->qemuCaps); - else if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, vm->def->emulator))) - goto cleanup; - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Make a copy for updated domain. */ vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt); if (!vmdef) goto endjob; - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) - goto endjob; + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], actionFlag) < 0) + goto endjob; - if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, dev, - dom->conn)) < 0) - goto endjob; + for (i = 0; i < devlist->count; i++) + if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, + devlist->devs[i], dom->conn)) < 0) + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (virDomainDefCompatibleDevice(vm->def, dev_copy, - VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) - goto endjob; + for (i = 0; i < devcopylist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devcopylist->devs[i], actionFlag) < 0) + goto endjob; - if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom)) < 0) - goto endjob; + for (i = 0; i < devcopylist->count; i++) + if ((ret = qemuDomainAttachDeviceLive(vm, devcopylist->devs[i], dom)) < 0) + goto endjob; /* * update domain status forcibly because the domain status may be * changed even if we failed to attach the device. For example, @@ -8301,9 +8446,9 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, cleanup: virObjectUnref(qemuCaps); virDomainDefFree(vmdef); - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); - virDomainDeviceDefFree(dev); + if (devlist != devcopylist) + virDomainDeviceDefListDispose(devcopylist); + virDomainDeviceDefListDispose(devlist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg); @@ -8325,13 +8470,17 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; - virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; + virDomainDeviceDefPtr dev = NULL; bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0; int ret = -1; virQEMUCapsPtr qemuCaps = NULL; + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + bool multifunction = false; + size_t i; + virDomainDeviceAction actionFlag = VIR_DOMAIN_DEVICE_ACTION_UPDATE; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -8355,12 +8504,25 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE); - if (dev == NULL) + multifunction = isMultifunctionDeviceXML(xml); + + if (VIR_ALLOC(devlist) < 0) goto endjob; + if (multifunction) { + if (virDomainPCIMultifunctionDeviceDefParseNode(xml, vm->def, devlist, + caps, driver->xmlopt, VIR_DOMAIN_DEF_PARSE_INACTIVE) < 0) + goto endjob; + } else { + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE); + if (!dev || virDomainDeviceDefListAddCopy(devlist, dev, vm->def, caps, + driver->xmlopt) < 0) + goto endjob; + } + + devcopylist = devlist; + if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob; @@ -8370,9 +8532,12 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, * 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) + if (VIR_ALLOC(devcopylist) < 0) goto endjob; + for (i = 0; i < devlist->count; i++) + if (virDomainDeviceDefListAddCopy(devcopylist, dev, vm->def, + caps, driver->xmlopt) < 0) + goto endjob; } if (priv->qemuCaps) @@ -8385,22 +8550,26 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt); if (!vmdef) goto endjob; + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], actionFlag) < 0) + goto endjob; - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - goto endjob; - - if ((ret = qemuDomainUpdateDeviceConfig(qemuCaps, vmdef, dev)) < 0) - goto endjob; + for (i = 0; i < devlist->count; i++) + if ((ret = qemuDomainUpdateDeviceConfig(qemuCaps, vmdef, devlist->devs[i])) < 0) + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (virDomainDefCompatibleDevice(vm->def, dev_copy, - VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - goto endjob; + for (i = 0; i < devcopylist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devcopylist->devs[i], + actionFlag) < 0) + goto endjob; - if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, dev_copy, dom, force)) < 0) - goto endjob; + for (i = 0; i < devcopylist->count; i++) + if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, + devcopylist->devs[i], dom, + force)) < 0) + goto endjob; /* * update domain status forcibly because the domain status may be * changed even if we failed to attach the device. For example, @@ -8427,9 +8596,9 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, cleanup: virObjectUnref(qemuCaps); virDomainDefFree(vmdef); - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); - virDomainDeviceDefFree(dev); + if (devlist != devcopylist) + virDomainDeviceDefListDispose(devcopylist); + virDomainDeviceDefListDispose(devlist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg); @@ -8444,13 +8613,17 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; - virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; + virDomainDeviceDefPtr dev = NULL; + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; int ret = -1; unsigned int parse_flags = 0; virQEMUCapsPtr qemuCaps = NULL; qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + bool multifunction = false; + size_t i; + virDomainDeviceAction actionFlag = VIR_DOMAIN_DEVICE_ACTION_DETACH; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -8478,21 +8651,38 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, !(flags & VIR_DOMAIN_AFFECT_LIVE)) parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - parse_flags); - if (dev == NULL) + multifunction = isMultifunctionDeviceXML(xml); + + if (VIR_ALLOC(devlist) < 0) goto endjob; + if (multifunction) { + if (virDomainPCIMultifunctionDeviceDefParseNode(xml, vm->def, devlist, + caps, driver->xmlopt, + parse_flags) < 0) + goto endjob; + } else { + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, parse_flags); + if (!dev || !qemuDomainPCIAddressIsSingleFunctionAddr(dev) || + virDomainDeviceDefListAddCopy(devlist, dev, vm->def, caps, + driver->xmlopt) < 0) + goto endjob; + } + + devcopylist = devlist; + 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) + if (VIR_ALLOC(devcopylist) < 0) goto endjob; + for (i = 0; i < devcopylist->count; i++) + if (virDomainDeviceDefListAddCopy(devcopylist, dev, vm->def, caps, + driver->xmlopt) < 0) + goto endjob; } if (priv->qemuCaps) @@ -8506,21 +8696,26 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, if (!vmdef) goto endjob; - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) - goto endjob; - - if ((ret = qemuDomainDetachDeviceConfig(vmdef, dev)) < 0) - goto endjob; + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], actionFlag) < 0) + goto endjob; + for (i = 0; i < devlist->count; i++) + if ((ret = qemuDomainDetachDeviceConfig(vmdef, devlist->devs[i])) < 0) + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (virDomainDefCompatibleDevice(vm->def, dev_copy, - VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) - goto endjob; + for (i = 0; i < devcopylist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devcopylist->devs[i], actionFlag) < 0) + goto endjob; - if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom)) < 0) - goto endjob; + for (i = 0; i < devcopylist->count; i++) { + if ((ret = qemuDomainDetachDeviceLive(vm, devcopylist->devs[i], dom)) < 0) + goto endjob; + /* Detaching any one of the functions is sufficient to unplug */ + if (ARCH_IS_X86(vm->def->os.arch)) + break; + } /* * update domain status forcibly because the domain status may be * changed even if we failed to attach the device. For example, @@ -8547,9 +8742,9 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, cleanup: virObjectUnref(qemuCaps); virDomainDefFree(vmdef); - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); - virDomainDeviceDefFree(dev); + if (devlist != devcopylist) + virDomainDeviceDefListDispose(devcopylist); + virDomainDeviceDefListDispose(devlist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg);