On 8/20/19 11:30 AM, Michal Privoznik wrote: > KVM style of PCI devices assignment was dropped in kernel in > favor of vfio pci (see kernel commit v4.12-rc1~68^2~65). Since > vfio is around for quite some time now and is far superior > discourage people in using KVM style. > > Ideally, I'd make QEMU_CAPS_VFIO_PCI implicitly assumed but turns > out qemu-3.0.0 doesn't support vfio-pci device for RISC-V. > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > ---

>   src/qemu/qemu_capabilities.c |  6 ----- >   src/qemu/qemu_command.c      | 26 ++------------------- >   src/qemu/qemu_command.h      |  1 - >   src/qemu/qemu_driver.c       | 14 ++++-------- >   src/qemu/qemu_hostdev.c      | 44 +++--------------------------------- >   src/qemu/qemu_hostdev.h      |  1 - >   src/qemu/qemu_hotplug.c      | 20 ++-------------- >   tests/domaincapstest.c       |  3 +-- >   8 files changed, 12 insertions(+), 103 deletions(-) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index c9677315ab..73300128ea 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -5338,7 +5338,6 @@ static int >   virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps, > virDomainCapsDeviceHostdevPtr hostdev) >   { > -    bool supportsPassthroughKVM = > qemuHostdevHostSupportsPassthroughLegacy(); >       bool supportsPassthroughVFIO = > qemuHostdevHostSupportsPassthroughVFIO(); >         hostdev->supported = VIR_TRISTATE_BOOL_YES; > @@ -5374,11 +5373,6 @@ > virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps, > VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO); >       } >   -    if (supportsPassthroughKVM) { > -        VIR_DOMAIN_CAPS_ENUM_SET(hostdev->pciBackend, > - VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, > - VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM); > -    } >       return 0; >   } >   diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index e3f4ef624b..f7b5430db8 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -4904,7 +4904,6 @@ char * >   qemuBuildPCIHostdevDevStr(const virDomainDef *def, >                             virDomainHostdevDefPtr dev, >                             unsigned int bootIndex, /* used iff > dev->info->bootIndex == 0 */ > -                          const char *configfd, >                             virQEMUCapsPtr qemuCaps) >   { >       virBuffer buf = VIR_BUFFER_INITIALIZER; > @@ -4913,16 +4912,11 @@ qemuBuildPCIHostdevDevStr(const virDomainDef > *def, >         /* caller has to assign proper passthrough backend type */ >       switch ((virDomainHostdevSubsysPCIBackendType)backend) { > -    case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM: > -        virBufferAddLit(&buf, "pci-assign"); > -        if (configfd && *configfd) > -            virBufferAsprintf(&buf, ",configfd=%s", configfd); > -        break; > - >       case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO: >           virBufferAddLit(&buf, "vfio-pci"); >           break; >   +    case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM: >       case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: >       case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN: >       case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST: > @@ -5676,7 +5670,6 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, >                   } >               } >   -            char *configfd_name = NULL; >               unsigned int bootIndex = hostdev->info->bootIndex; >                 /* bootNet will be non-0 if boot order was set and no > other > @@ -5686,27 +5679,12 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, >                   bootIndex = *bootHostdevNet; >                   *bootHostdevNet = 0; >               } > -            if (backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { > -                int configfd = qemuOpenPCIConfig(hostdev); > - > -                if (configfd >= 0) { > -                    if (virAsprintf(&configfd_name, "%d", configfd) > < 0) { > -                        VIR_FORCE_CLOSE(configfd); > -                        return -1; > -                    } > - > -                    virCommandPassFD(cmd, configfd, > - VIR_COMMAND_PASS_FD_CLOSE_PARENT); > -                } > -            } >                 if (qemuCommandAddExtDevice(cmd, hostdev->info) < 0) >                   return -1; >                 virCommandAddArg(cmd, "-device"); > -            devstr = qemuBuildPCIHostdevDevStr(def, hostdev, bootIndex, > -                                               configfd_name, > qemuCaps); > -            VIR_FREE(configfd_name); > +            devstr = qemuBuildPCIHostdevDevStr(def, hostdev, > bootIndex, qemuCaps); >               if (!devstr) >                   return -1; >               virCommandAddArg(cmd, devstr); > diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h > index 7e2dc5a60a..e3983bedb2 100644 > --- a/src/qemu/qemu_command.h > +++ b/src/qemu/qemu_command.h > @@ -148,7 +148,6 @@ char > *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, >   char *qemuBuildPCIHostdevDevStr(const virDomainDef *def, >                                   virDomainHostdevDefPtr dev, >                                   unsigned int bootIndex, > -                                const char *configfd, >                                   virQEMUCapsPtr qemuCaps); >     char *qemuBuildRNGDevStr(const virDomainDef *def, > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 11f97dbc65..eb373c14d7 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -13517,7 +13517,6 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev, >       int ret = -1; >       virNodeDeviceDefPtr def = NULL; >       char *xml = NULL; > -    bool legacy = qemuHostdevHostSupportsPassthroughLegacy(); >       bool vfio = qemuHostdevHostSupportsPassthroughVFIO(); >       virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; >   @@ -13544,8 +13543,6 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr > dev, >       if (!driverName) { >           if (vfio) { >               driverName = "vfio"; > -        } else if (legacy) { > -            driverName = "kvm"; >           } else { >               virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", >                              _("neither VFIO nor KVM device > assignment is " Right here comes the error message:             virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",                            _("neither VFIO nor KVM device assignment is "                              "currently supported on this system"));             goto cleanup; I think this message should refer only to VFIO in this case - there's already a KVM device assignment message down below. Thanks, DHB > @@ -13563,13 +13560,10 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr > dev, >           } >           virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_VFIO); >       } else if (STREQ(driverName, "kvm")) { > -        if (!legacy) { > -            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", > -                           _("KVM device assignment is currently not " > -                             "supported on this system")); > -            goto cleanup; > -        } > -        virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_KVM); > +        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", > +                       _("KVM device assignment is currently not " > +                         "supported on this system")); > +        goto cleanup; >       } else { >           virReportError(VIR_ERR_INVALID_ARG, >                          _("unknown driver name '%s'"), driverName); > diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c > index 92b037e1ed..af41c32679 100644 > --- a/src/qemu/qemu_hostdev.c > +++ b/src/qemu/qemu_hostdev.c > @@ -133,44 +133,11 @@ qemuHostdevHostSupportsPassthroughVFIO(void) >   } >     -#if HAVE_LINUX_KVM_H > -# include <linux/kvm.h> > -bool > -qemuHostdevHostSupportsPassthroughLegacy(void) > -{ > -    int kvmfd = -1; > -    bool ret = false; > - > -    if ((kvmfd = open("/dev/kvm", O_RDONLY)) < 0) > -        goto cleanup; > - > -# ifdef KVM_CAP_IOMMU > -    if ((ioctl(kvmfd, KVM_CHECK_EXTENSION, KVM_CAP_IOMMU)) <= 0) > -        goto cleanup; > - > -    ret = true; > -# endif > - > - cleanup: > -    VIR_FORCE_CLOSE(kvmfd); > - > -    return ret; > -} > -#else > -bool > -qemuHostdevHostSupportsPassthroughLegacy(void) > -{ > -    return false; > -} > -#endif > - > - >   static bool > qemuHostdevPreparePCIDevicesCheckSupport(virDomainHostdevDefPtr > *hostdevs, >                                            size_t nhostdevs, >                                            virQEMUCapsPtr qemuCaps) >   { > -    bool supportsPassthroughKVM = > qemuHostdevHostSupportsPassthroughLegacy(); >       bool supportsPassthroughVFIO = > qemuHostdevHostSupportsPassthroughVFIO(); >       size_t i; >   @@ -189,8 +156,6 @@ > qemuHostdevPreparePCIDevicesCheckSupport(virDomainHostdevDefPtr > *hostdevs, >               if (supportsPassthroughVFIO && >                   virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { >                   *backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; > -            } else if (supportsPassthroughKVM) { > -                *backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM; >               } else { >                   virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >                                  _("host doesn't support passthrough > of " > @@ -209,12 +174,9 @@ > qemuHostdevPreparePCIDevicesCheckSupport(virDomainHostdevDefPtr > *hostdevs, >               break; >             case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM: > -            if (!supportsPassthroughKVM) { > -                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > -                               _("host doesn't support legacy PCI > passthrough")); > -                return false; > -            } > - > +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > +                           _("host doesn't support legacy PCI > passthrough")); > +            return false; >               break; >             case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN: > diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h > index f6d76c1c2a..e99c204961 100644 > --- a/src/qemu/qemu_hostdev.h > +++ b/src/qemu/qemu_hostdev.h > @@ -24,7 +24,6 @@ >   #include "qemu_conf.h" >   #include "domain_conf.h" >   -bool qemuHostdevHostSupportsPassthroughLegacy(void); >   bool qemuHostdevHostSupportsPassthroughVFIO(void); >     int qemuHostdevUpdateActiveMediatedDevices(virQEMUDriverPtr driver, > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index d8be63b71c..63acb9c451 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -1465,8 +1465,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr > driver, >       virDomainDeviceInfoPtr info = hostdev->info; >       int ret; >       char *devstr = NULL; > -    int configfd = -1; > -    char *configfd_name = NULL; >       bool releaseaddr = false; >       bool teardowncgroup = false; >       bool teardownlabel = false; > @@ -1547,13 +1545,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr > driver, >       if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0) >           goto error; >       releaseaddr = true; > -    if (backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { > -        configfd = qemuOpenPCIConfig(hostdev); > -        if (configfd >= 0) { > -            if (virAsprintf(&configfd_name, "fd-%s", info->alias) < 0) > -                goto error; > -        } > -    } >         if (!virDomainObjIsActive(vm)) { >           virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > @@ -1561,8 +1552,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr > driver, >           goto error; >       } >   -    if (!(devstr = qemuBuildPCIHostdevDevStr(vm->def, hostdev, 0, > -                                             configfd_name, > priv->qemuCaps))) > +    if (!(devstr = qemuBuildPCIHostdevDevStr(vm->def, hostdev, 0, > priv->qemuCaps))) >           goto error; >         qemuDomainObjEnterMonitor(driver, vm); > @@ -1570,10 +1560,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr > driver, >       if ((ret = qemuDomainAttachExtensionDevice(priv->mon, > hostdev->info)) < 0) >           goto exit_monitor; >   -    if ((ret = qemuMonitorAddDeviceWithFd(priv->mon, devstr, > -                                          configfd, configfd_name)) > < 0) { > +    if ((ret = qemuMonitorAddDevice(priv->mon, devstr)) < 0) > ignore_value(qemuDomainDetachExtensionDevice(priv->mon, hostdev->info)); > -    } >      exit_monitor: >       if (qemuDomainObjExitMonitor(driver, vm) < 0) > @@ -1586,8 +1574,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr > driver, >       vm->def->hostdevs[vm->def->nhostdevs++] = hostdev; >         VIR_FREE(devstr); > -    VIR_FREE(configfd_name); > -    VIR_FORCE_CLOSE(configfd); >         return 0; >   @@ -1607,8 +1593,6 @@ > qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, >       qemuHostdevReAttachPCIDevices(driver, vm->def->name, &hostdev, 1); >         VIR_FREE(devstr); > -    VIR_FREE(configfd_name); > -    VIR_FORCE_CLOSE(configfd); >      cleanup: >       return -1; > diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c > index 45ecfe61ac..77acefa6b0 100644 > --- a/tests/domaincapstest.c > +++ b/tests/domaincapstest.c > @@ -111,8 +111,7 @@ fillQemuCaps(virDomainCapsPtr domCaps, >                                     cfg->nfirmwares) < 0) >           goto cleanup; >   -    /* The function above tries to query host's KVM & VFIO > capabilities by > -     * calling qemuHostdevHostSupportsPassthroughLegacy() and > +    /* The function above tries to query host's VFIO capabilities by > calling >        * qemuHostdevHostSupportsPassthroughVFIO() which, however, > can't be >        * successfully mocked as they are not exposed as internal > APIs. Therefore, >        * instead of mocking set the expected values here by hand. */

There are two places where we need to create virPCIDevice from given virDomainHostdevDef. In both places the code is duplicated. Move them into a single function and call it from those two places. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; ---

src/util/virhostdev.c | 93 +++++++++++++++++++++++++------------------ 1 file changed, 54 insertions(+), 39 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 6861b8a84e..e3f48a9a2e 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -212,6 +212,51 @@ virHostdevManagerGetDefault(void) return virObjectRef(manager); } +/** + * virHostdevGetPCIHostDevice: + * @hostdev: domain hostdev definition + * @pci: returned PCI device + * + * For given @hostdev which represents a PCI device construct its + * virPCIDevice representation and returned it in @pci. If + * @hostdev does not represent a PCI device then @pci is set to + * NULL and 0 is returned. + * + * Returns: 0 on success (@pci might be NULL though), + * -1 otherwise (with error reported). + */ +static int +virHostdevGetPCIHostDevice(const virDomainHostdevDef *hostdev, + virPCIDevicePtr *pci) +{ + VIR_AUTOPTR(virPCIDevice) actual = NULL; + const virDomainHostdevSubsysPCI *pcisrc = &hostdev->source.subsys.u.pci; + + *pci = NULL; + + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || + hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + return 0; + + actual = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); + + if (!actual) + return -1; + + virPCIDeviceSetManaged(actual, hostdev->managed); + + if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) + virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_VFIO); + else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN) + virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_XEN); + else + virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_KVM); + + VIR_STEAL_PTR(*pci, actual); + return 0; +} + static virPCIDeviceListPtr virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs) { @@ -223,27 +268,13 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs) for (i = 0; i < nhostdevs; i++) { virDomainHostdevDefPtr hostdev = hostdevs[i]; - virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; VIR_AUTOPTR(virPCIDevice) pci = NULL; - if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) - continue; - if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) - continue; + if (virHostdevGetPCIHostDevice(hostdev, &pci) < 0) + return NULL; - pci = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, - pcisrc->addr.slot, pcisrc->addr.function); if (!pci) - return NULL; - - virPCIDeviceSetManaged(pci, hostdev->managed); - - if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) - virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_VFIO); - else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN) - virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_XEN); - else - virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_KVM); + continue; if (virPCIDeviceListAdd(pcidevs, pci) < 0) return NULL; @@ -253,7 +284,6 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs) VIR_RETURN_PTR(pcidevs); } - static int virHostdevPCISysfsPath(virDomainHostdevDefPtr hostdev, char **sysfs_path) @@ -1067,7 +1097,6 @@ virHostdevUpdateActivePCIDevices(virHostdevManagerPtr mgr, const char *drv_name, const char *dom_name) { - virDomainHostdevDefPtr hostdev = NULL; size_t i; int ret = -1; @@ -1078,32 +1107,18 @@ virHostdevUpdateActivePCIDevices(virHostdevManagerPtr mgr, virObjectLock(mgr->inactivePCIHostdevs); for (i = 0; i < nhostdevs; i++) { + const virDomainHostdevDef *hostdev = hostdevs[i]; VIR_AUTOPTR(virPCIDevice) actual = NULL; - virDomainHostdevSubsysPCIPtr pcisrc; - hostdev = hostdevs[i]; - pcisrc = &hostdev->source.subsys.u.pci; - if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) - continue; - if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) - continue; - - actual = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, - pcisrc->addr.slot, pcisrc->addr.function); + if (virHostdevGetPCIHostDevice(hostdev, &actual) < 0) + goto cleanup; if (!actual) + continue; + + if (virPCIDeviceSetUsedBy(actual, drv_name, dom_name) < 0) goto cleanup; - virPCIDeviceSetManaged(actual, hostdev->managed); - virPCIDeviceSetUsedBy(actual, drv_name, dom_name); - - if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) - virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_VFIO); - else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN) - virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_XEN); - else - virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_KVM); - /* Setup the original states for the PCI device */ virPCIDeviceSetUnbindFromStub(actual, hostdev->origstates.states.pci.unbind_from_stub); virPCIDeviceSetRemoveSlot(actual, hostdev->origstates.states.pci.remove_slot);

The KVM assignment is going to be removed shortly. Don't let the hostdev module configure it. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; ---

src/util/virhostdev.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index e3f48a9a2e..29e75ffc84 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -246,12 +246,16 @@ virHostdevGetPCIHostDevice(const virDomainHostdevDef *hostdev, virPCIDeviceSetManaged(actual, hostdev->managed); - if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) + if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_VFIO); - else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN) + } else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN) { virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_XEN); - else - virPCIDeviceSetStubDriver(actual, VIR_PCI_STUB_DRIVER_KVM); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("pci backend driver '%s' is not supported"), + virDomainHostdevSubsysPCIBackendTypeToString(pcisrc->backend)); + return -1; + } VIR_STEAL_PTR(*pci, actual); return 0;

This function is no longer used after previous commit. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; ---

src/libvirt_private.syms | 1 - src/util/virpci.c | 108 --------------------------------------- src/util/virpci.h | 1 - 3 files changed, 110 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9db4ac7933..6c57351866 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2696,7 +2696,6 @@ virPCIDeviceSetStubDriver; virPCIDeviceSetUnbindFromStub; virPCIDeviceSetUsedBy; virPCIDeviceUnbind; -virPCIDeviceWaitForCleanup; virPCIEDeviceInfoFree; virPCIELinkSpeedTypeFromString; virPCIELinkSpeedTypeToString; diff --git a/src/util/virpci.c b/src/util/virpci.c index 9c9ffa55c2..ea5be62033 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1552,114 +1552,6 @@ virPCIDeviceReattach(virPCIDevicePtr dev, return 0; } -/* Certain hypervisors (like qemu/kvm) map the PCI bar(s) on - * the host when doing device passthrough. This can lead to a race - * condition where the hypervisor is still cleaning up the device while - * libvirt is trying to re-attach it to the host device driver. To avoid - * this situation, we look through /proc/iomem, and if the hypervisor is - * still holding on to the bar (denoted by the string in the matcher - * variable), then we can wait around a bit for that to clear up. - * - * A typical /proc/iomem looks like this (snipped for brevity): - * 00010000-0008efff : System RAM - * 0008f000-0008ffff : reserved - * ... - * 00100000-cc9fcfff : System RAM - * 00200000-00483d3b : Kernel code - * 00483d3c-005c88df : Kernel data - * cc9fd000-ccc71fff : ACPI Non-volatile Storage - * ... - * d0200000-d02fffff : PCI Bus #05 - * d0200000-d021ffff : 0000:05:00.0 - * d0200000-d021ffff : e1000e - * d0220000-d023ffff : 0000:05:00.0 - * d0220000-d023ffff : e1000e - * ... - * f0000000-f0003fff : 0000:00:1b.0 - * f0000000-f0003fff : kvm_assigned_device - * - * Returns 0 if we are clear to continue, and 1 if the hypervisor is still - * holding on to the resource. - */ -int -virPCIDeviceWaitForCleanup(virPCIDevicePtr dev, const char *matcher) -{ - FILE *fp; - char line[160]; - char *tmp; - unsigned long long start, end; - unsigned int domain, bus, slot, function; - bool in_matching_device; - int ret; - size_t match_depth; - - fp = fopen("/proc/iomem", "r"); - if (!fp) { - /* If we failed to open iomem, we just basically ignore the error. The - * unbind might succeed anyway, and besides, it's very likely we have - * no way to report the error - */ - VIR_DEBUG("Failed to open /proc/iomem, trying to continue anyway"); - return 0; - } - - ret = 0; - in_matching_device = false; - match_depth = 0; - while (fgets(line, sizeof(line), fp) != 0) { - /* the logic here is a bit confusing. For each line, we look to - * see if it matches the domain:bus:slot.function we were given. - * If this line matches the DBSF, then any subsequent lines indented - * by 2 spaces are the PCI regions for this device. It's also - * possible that none of the PCI regions are currently mapped, in - * which case we have no indented regions. This code handles all - * of these situations - */ - if (in_matching_device && (strspn(line, " ") == (match_depth + 2))) { - /* expected format: <start>-<end> : <suffix> */ - if (/* start */ - virStrToLong_ull(line, &tmp, 16, &start) < 0 || *tmp != '-' || - /* end */ - virStrToLong_ull(tmp + 1, &tmp, 16, &end) < 0 || - (tmp = STRSKIP(tmp, " : ")) == NULL) - continue; - - if (STRPREFIX(tmp, matcher)) { - ret = 1; - break; - } - } else { - in_matching_device = false; - - /* expected format: <start>-<end> : <domain>:<bus>:<slot>.<function> */ - if (/* start */ - virStrToLong_ull(line, &tmp, 16, &start) < 0 || *tmp != '-' || - /* end */ - virStrToLong_ull(tmp + 1, &tmp, 16, &end) < 0 || - (tmp = STRSKIP(tmp, " : ")) == NULL || - /* domain */ - virStrToLong_ui(tmp, &tmp, 16, &domain) < 0 || *tmp != ':' || - /* bus */ - virStrToLong_ui(tmp + 1, &tmp, 16, &bus) < 0 || *tmp != ':' || - /* slot */ - virStrToLong_ui(tmp + 1, &tmp, 16, &slot) < 0 || *tmp != '.' || - /* function */ - virStrToLong_ui(tmp + 1, &tmp, 16, &function) < 0 || *tmp != '\n') - continue; - - if (domain != dev->address.domain || bus != dev->address.bus || - slot != dev->address.slot || function != dev->address.function) - continue; - in_matching_device = true; - match_depth = strspn(line, " "); - } - } - - VIR_FORCE_FCLOSE(fp); - - return ret; -} - static char * virPCIDeviceReadID(virPCIDevicePtr dev, const char *id_name) { diff --git a/src/util/virpci.h b/src/util/virpci.h index 4ac0d230a4..dc20f91710 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -197,7 +197,6 @@ char *virPCIDeviceGetIOMMUGroupDev(virPCIDevicePtr dev); int virPCIDeviceIsAssignable(virPCIDevicePtr dev, int strict_acs_check); -int virPCIDeviceWaitForCleanup(virPCIDevicePtr dev, const char *matcher); virPCIDeviceAddressPtr virPCIGetDeviceAddressFromSysfsLink(const char *device_link);

Now that nothing supports "pci-stub" driver (aka KVM style of PCI device assignment) there is no need for virpcimock to create it. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; ---

tests/virpcimock.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/virpcimock.c b/tests/virpcimock.c index c0dcd377d5..bbec77975a 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -992,7 +992,6 @@ init_env(void) MAKE_PCI_DRIVER("iwlwifi", 0x8086, 0x0044); MAKE_PCI_DRIVER("i915", 0x8086, 0x0046, 0x8086, 0x0047); - MAKE_PCI_DRIVER("pci-stub", -1, -1); pci_driver_new("vfio-pci", PCI_ACTION_BIND, -1, -1); # define MAKE_PCI_DEVICE(Id, Vendor, Device, ...) \

This enum was introduced to model how RHEL-7 kernel behaves - for some reason going with the old way (via new_id + bind) fails but using driver_override succeeds. Well, we don't need to care about that anymore since we don't create new_id file. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; ---

tests/virpcimock.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 3157037913..a5045ed97c 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -98,19 +98,11 @@ char *fakerootdir; * */ -enum driverActions { - PCI_ACTION_BIND = 1 << 0, - PCI_ACTION_UNBIND = 1 << 1, - PCI_ACTION_NEW_ID = 1 << 2, - PCI_ACTION_REMOVE_ID = 1 << 3, -}; - struct pciDriver { char *name; int *vendor; /* List of vendor:device IDs the driver can handle */ int *device; size_t len; /* @len is used for both @vendor and @device */ - unsigned int fail; /* Bitwise-OR of driverActions that should fail */ }; struct pciDeviceAddress { @@ -151,7 +143,7 @@ static void pci_device_new_from_stub(const struct pciDevice *data); static struct pciDevice *pci_device_find_by_id(struct pciDeviceAddress const *addr); static struct pciDevice *pci_device_find_by_content(const char *path); -static void pci_driver_new(const char *name, int fail, ...); +static void pci_driver_new(const char *name, ...); static struct pciDriver *pci_driver_find_by_dev(struct pciDevice *dev); static struct pciDriver *pci_driver_find_by_path(const char *path); static struct pciDriver *pci_driver_find_by_driver_override(struct pciDevice *dev); @@ -594,7 +586,7 @@ pci_driver_get_path(const struct pciDriver *driver, static void -pci_driver_new(const char *name, int fail, ...) +pci_driver_new(const char *name, ...) { struct pciDriver *driver; va_list args; @@ -606,12 +598,10 @@ pci_driver_new(const char *name, int fail, ...) !(driverpath = pci_driver_get_path(driver, NULL, true))) ABORT_OOM(); - driver->fail = fail; - if (virFileMakePath(driverpath) < 0) ABORT("Unable to create: %s", driverpath); - va_start(args, fail); + va_start(args, name); while ((vendor = va_arg(args, int)) != -1) { if ((device = va_arg(args, int)) == -1) @@ -805,7 +795,7 @@ pci_driver_handle_bind(const char *path) struct pciDevice *dev = pci_device_find_by_content(path); struct pciDriver *driver = pci_driver_find_by_path(path); - if (!driver || !dev || PCI_ACTION_BIND & driver->fail) { + if (!driver || !dev) { /* No driver, no device or failing driver requested */ errno = ENODEV; goto cleanup; @@ -822,7 +812,7 @@ pci_driver_handle_unbind(const char *path) int ret = -1; struct pciDevice *dev = pci_device_find_by_content(path); - if (!dev || !dev->driver || PCI_ACTION_UNBIND & dev->driver->fail) { + if (!dev || !dev->driver) { /* No device, device not binded or failing driver requested */ errno = ENODEV; goto cleanup; @@ -876,11 +866,11 @@ init_env(void) make_file(tmp, "drivers_probe", NULL, -1); # define MAKE_PCI_DRIVER(name, ...) \ - pci_driver_new(name, 0, __VA_ARGS__, -1, -1) + pci_driver_new(name, __VA_ARGS__, -1, -1) MAKE_PCI_DRIVER("iwlwifi", 0x8086, 0x0044); MAKE_PCI_DRIVER("i915", 0x8086, 0x0046, 0x8086, 0x0047); - pci_driver_new("vfio-pci", PCI_ACTION_BIND, -1, -1); + MAKE_PCI_DRIVER("vfio-pci", -1, -1); # define MAKE_PCI_DEVICE(Id, Vendor, Device, ...) \ do { \

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- docs/news.xml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index d63fca3b48..6137ebd943 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -51,6 +51,19 @@ </description> </change> </section> + <section title="Removed features"> + <change> + <summary> + Remove KVM assignment support + </summary> + <description> + The KVM style of PCI device assignment was removed from + kernel in kernel 4.12.0 after being deprecated since 4.2.0.

+ Libvirt defaults to VFIO for a long time. Remove support for + KVM device assignment then.