Shivaprasad G Bhat <shivaprasadbhat@xxxxxxxxx> writes:
> 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@xxxxxxxxxxxxxxxxxx>
> ---
> 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;
Remove this, not supported for PCI
> + 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)
isPCIMultifunctionDeviceXML
> +{
> + 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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list