[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



Shivaprasad G Bhat <shivaprasadbhat gmail com> 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 linux vnet ibm com>
> ---
>  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 redhat com
> https://www.redhat.com/mailman/listinfo/libvir-list


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]