[libvirt] [PATCH v2 8/8] Enable PCI Multifunction hotplug/unplug

The flow is to parse and create a list of devices. Go on each device at each step. Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com&gt; --- src/qemu/qemu_driver.c | 328 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 256 insertions(+), 72 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9cff397..70c241c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8191,6 +8191,77 @@ 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"); +} + static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) @@ -8198,7 +8269,8 @@ 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; + virDomainDeviceDefListPtr rollbacklist = NULL; int ret = -1; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; @@ -8207,6 +8279,11 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + virDomainDeviceDefPtr dev = NULL; + bool multifunction = false, pciHostdevs = false; + size_t i = 0, j, d; + virDomainDeviceAction actionFlag = VIR_DOMAIN_DEVICE_ACTION_ATTACH; + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -8231,27 +8308,49 @@ 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) + 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; + } 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; + goto endjob; if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Make a copy for updated domain. */ @@ -8259,27 +8358,42 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, 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 (devlist->devs[0]->type == VIR_DOMAIN_DEVICE_HOSTDEV && + devlist->devs[0]->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + pciHostdevs = true; + 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 (dev_copy->type == VIR_DOMAIN_DEVICE_HOSTDEV && - dev_copy->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && - qemuDomainAttachPCIHostDevicePrepare(driver,vm->def, dev_copy->data.hostdev, qemuCaps) < 0) - goto endjob; + for (d = 0; d < devcopylist->count; d++) + if (pciHostdevs && + qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, + devcopylist->devs[d]->data.hostdev, qemuCaps) < 0) + goto undoprepare; - if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom)) < 0) - goto undoprepare; + /* Keep a copy for reverts. The devices are initialised to null in + * AttachDeviceLive function */ + if (VIR_ALLOC(rollbacklist) < 0) + goto endjob; + for (i = 0; i < devcopylist->count; i++) { + if (virDomainDeviceDefListAddCopy(rollbacklist, devlist->devs[i], + vm->def, caps, driver->xmlopt) < 0) + goto rollback_livehotplug; + if ((ret = qemuDomainAttachDeviceLive(vm, devcopylist->devs[i], dom)) < 0) + goto rollback_livehotplug; + } /* * update domain status forcibly because the domain status may be * changed even if we failed to attach the device. For example, @@ -8306,19 +8420,39 @@ 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); + virDomainDeviceDefListDispose(rollbacklist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg); virNWFilterUnlockFilterUpdates(); return ret; - undoprepare: - if (dev_copy->type == VIR_DOMAIN_DEVICE_HOSTDEV && - dev_copy->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) - qemuHostdevReAttachPCIDevices(driver, vm->def->name, &dev_copy->data.hostdev, 1); + rollback_livehotplug: + /* The last hotplug on list failed. So "count-1". + * Rollback takes care of cleaning up of any preparations done + * by the respective devices. So, not necessary to do + * any cleanup explicitly here. + */ + for (j = 0; j < rollbacklist->count-1; j++) { + if ((ret = qemuDomainDetachDeviceLive(vm, rollbacklist->devs[j], dom)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Couldnt revert hotplug")); + goto endjob; + } + } + + undoprepare: + if (pciHostdevs) { + /* Devices from o to rollbacklist->count are reattached as part of + * device_deleted event rest of the devices should be bound back to host + * here. 'd' is the last device we detached */ + for (j = rollbacklist ? rollbacklist->count : 0; j < d; j++) + qemuHostdevReAttachPCIDevices(driver, vm->def->name, + &devcopylist->devs[j]->data.hostdev, 1); + } + ret = -1; goto endjob; } @@ -8336,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 | @@ -8366,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; @@ -8381,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) @@ -8396,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, @@ -8438,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); @@ -8455,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); @@ -8489,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) @@ -8517,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, @@ -8558,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);