Re: [libvirt] [PATCH v3 13/13] Enable PCI Multifunction hotplug/unplug

The flow is to parse and create a list of devices and pass onto the hotplug functions. The patch also removes all checks forbidding the multifunction hotplug. Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com&gt; --- src/qemu/qemu_driver.c | 166 ++++++++++++++++++++++++++++++++++++++--------- src/qemu/qemu_hotplug.c | 66 +------------------ 2 files changed, 140 insertions(+), 92 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e970ad6..0519a33 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7453,6 +7453,77 @@ qemuDomainUndefine(virDomainPtr dom) return qemuDomainUndefineFlags(dom, 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) @@ -7469,6 +7540,8 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + bool multifunction = false; + size_t i; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -7494,14 +7567,25 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob; - dev = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - parse_flags); - if (!dev || VIR_ALLOC(devlist) < 0) - goto endjob; + multifunction = isMultifunctionDeviceXML(xml); - if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) - goto endjob; + if (multifunction) { + if (!(devlist = virDomainDeviceDefParseXMLMany(xml, vm->def, + caps, driver->xmlopt, + parse_flags))) + goto endjob; + if (virDomainPCIMultifunctionDeviceAddressValidateAssign(priv->pciaddrs, devlist) < 0) + goto endjob; + } else { + dev = virDomainDeviceDefParse(xml, vm->def, + caps, driver->xmlopt, + parse_flags); + if (!dev || VIR_ALLOC(devlist) < 0) + goto endjob; + if (!qemuDomainPCIAddressIsSingleFunctionAddr(dev) || + VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) + goto endjob; + } devcopylist = devlist; @@ -7513,15 +7597,16 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, */ if (VIR_ALLOC(devcopylist) < 0) goto endjob; - if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], - vm->def, caps, driver->xmlopt) < 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. */ @@ -7529,14 +7614,14 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (!vmdef) goto endjob; - if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, devlist, - dom->conn)) < 0) + if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, devlist, dom->conn)) < 0) goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { if ((ret = qemuDomainAttachDeviceLive(vm, devcopylist, 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, @@ -7595,6 +7680,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + bool multifunction = false; + size_t i; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -7618,14 +7705,22 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - dev = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE); - if (!dev || VIR_ALLOC(devlist) < 0) - goto endjob; + multifunction = isMultifunctionDeviceXML(xml); - if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) - goto endjob; + if (multifunction) { + if (!(devlist = virDomainDeviceDefParseXMLMany(xml, vm->def, + caps, driver->xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE))) + goto endjob; + } else { + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE); + if (!dev || VIR_ALLOC(devlist) < 0) + goto endjob; + + if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) + goto endjob; + } devcopylist = devlist; @@ -7640,9 +7735,10 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, */ if (VIR_ALLOC(devcopylist) < 0) goto endjob; - if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], - vm->def, caps, driver->xmlopt) < 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) @@ -7714,6 +7810,7 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + bool multifunction = false; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -7741,14 +7838,21 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, !(flags & VIR_DOMAIN_AFFECT_LIVE)) parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; - dev = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - parse_flags); - if (!dev || VIR_ALLOC(devlist) < 0) - goto endjob; + multifunction = isMultifunctionDeviceXML(xml); - if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) - goto endjob; + if (multifunction) { + if (!(devlist = virDomainDeviceDefParseXMLMany(xml, vm->def, + caps, driver->xmlopt, + parse_flags))) + goto endjob; + } else { + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, parse_flags); + if (!dev || VIR_ALLOC(devlist) < 0) + goto endjob; + if (!qemuDomainPCIAddressIsSingleFunctionAddr(dev) || + VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) + goto endjob; + } devcopylist = devlist; @@ -7811,7 +7915,7 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, virDomainDefFree(vmdef); if (devlist != devcopylist) virDomainDeviceDefListFree(devcopylist); - virDomainDeviceDefFree(dev); + virDomainDeviceDefListFree(devlist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index adee545..35c1071 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2793,35 +2793,6 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, return ret; } - -static int qemuComparePCIDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, - virDomainDeviceDefPtr device ATTRIBUTE_UNUSED, - virDomainDeviceInfoPtr info1, - void *opaque) -{ - virDomainDeviceInfoPtr info2 = opaque; - - if (info1->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI || - info2->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) - return 0; - - if (info1->addr.pci.domain == info2->addr.pci.domain && - info1->addr.pci.bus == info2->addr.pci.bus && - info1->addr.pci.slot == info2->addr.pci.slot && - info1->addr.pci.function != info2->addr.pci.function) - return -1; - return 0; -} - -static bool qemuIsMultiFunctionDevice(virDomainDefPtr def, - virDomainDeviceInfoPtr dev) -{ - if (virDomainDeviceInfoIterate(def, qemuComparePCIDevice, dev) < 0) - return true; - return false; -} - - static int qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3430,13 +3401,6 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; - if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("cannot hot unplug multifunction PCI device: %s"), - detach->dst); - goto cleanup; - } - if (qemuDomainMachineIsS390CCW(vm->def) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { if (!virDomainDeviceAddressIsValid(&detach->info, @@ -3659,14 +3623,6 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver, goto cleanup; } - if (detach->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && - qemuIsMultiFunctionDevice(vm->def, &detach->info)) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("cannot hot unplug multifunction PCI device: %s"), - dev->data.disk->dst); - goto cleanup; - } - if (qemuDomainControllerIsBusy(vm, detach)) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("device cannot be detached: device is busy")); @@ -3702,17 +3658,8 @@ qemuDomainDetachHostPCIDevice(virQEMUDriverPtr driver, virDomainHostdevDefPtr detach) { qemuDomainObjPrivatePtr priv = vm->privateData; - virDomainHostdevSubsysPCIPtr pcisrc = &detach->source.subsys.u.pci; int ret; - if (qemuIsMultiFunctionDevice(vm->def, detach->info)) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("cannot hot unplug multifunction PCI device: %.4x:%.2x:%.2x.%.1x"), - pcisrc->addr.domain, pcisrc->addr.bus, - pcisrc->addr.slot, pcisrc->addr.function); - return -1; - } - if (!virDomainDeviceAddressIsValid(detach->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { virReportError(VIR_ERR_OPERATION_FAILED, @@ -3944,13 +3891,6 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver, "%s", _("device cannot be detached without a PCI address")); goto cleanup; } - - if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("cannot hot unplug multifunction PCI device :%s"), - dev->data.disk->dst); - goto cleanup; - } } if (!detach->info.alias) { @@ -4556,9 +4496,13 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) return -1; - for (i = 0; i < devlist->count; i++) + for (i = 0; i < devlist->count; i++) { if (qemuDomainDetachDeviceLiveInternal(vm, devlist->devs[i], dom) < 0) return -1; + /* Detaching any one of the functions is sufficient to unplug */ + if (ARCH_IS_X86(vm->def->os.arch)) + break; + } return 0; } -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list