[libvirt] [PATCH 08/12] virpci: Drop newid style of PCI device detach

Daniel Henrique Barboza danielhb413 at gmail.com
Tue Aug 20 17:46:47 UTC 2019



On 8/20/19 11:30 AM, Michal Privoznik wrote:
> As stated in 84f9358b18346 all kernels that we are interested in
> have 'drivers_override'. Drop the other, older style of
> overriding PCI device driver - newid.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---


Reviewed-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413 at gmail.com>


>   src/util/virpci.c | 284 +---------------------------------------------
>   1 file changed, 2 insertions(+), 282 deletions(-)
>
> diff --git a/src/util/virpci.c b/src/util/virpci.c
> index ea5be62033..6724a8ad9e 100644
> --- a/src/util/virpci.c
> +++ b/src/util/virpci.c
> @@ -218,16 +218,6 @@ virPCIDriverDir(const char *driver)
>   }
>   
>   
> -static char *
> -virPCIDriverFile(const char *driver, const char *file)
> -{
> -    char *buffer;
> -
> -    ignore_value(virAsprintf(&buffer, PCI_SYSFS "drivers/%s/%s", driver, file));
> -    return buffer;
> -}
> -
> -
>   static char *
>   virPCIFile(const char *device, const char *file)
>   {
> @@ -1145,104 +1135,6 @@ virPCIDeviceBindWithDriverOverride(virPCIDevicePtr dev,
>       return 0;
>   }
>   
> -static int
> -virPCIDeviceUnbindFromStubWithNewid(virPCIDevicePtr dev)
> -{
> -    int result = -1;
> -    VIR_AUTOFREE(char *) drvdir = NULL;
> -    VIR_AUTOFREE(char *) path = NULL;
> -    VIR_AUTOFREE(char *) driver = NULL;
> -
> -    /* If the device is currently bound to one of the "well known"
> -     * stub drivers, then unbind it, otherwise ignore it.
> -     */
> -    if (virPCIDeviceGetDriverPathAndName(dev, &drvdir, &driver) < 0)
> -        goto cleanup;
> -
> -    if (!driver) {
> -        /* The device is not bound to any driver and we are almost done. */
> -        VIR_DEBUG("PCI device %s is not bound to any driver", dev->name);
> -        goto reprobe;
> -    }
> -
> -    if (!dev->unbind_from_stub) {
> -        VIR_DEBUG("Unbind from stub skipped for PCI device %s", dev->name);
> -        goto remove_slot;
> -    }
> -
> -    /* If the device isn't bound to a known stub, skip the unbind. */
> -    if (virPCIStubDriverTypeFromString(driver) < 0 ||
> -        virPCIStubDriverTypeFromString(driver) == VIR_PCI_STUB_DRIVER_NONE) {
> -        VIR_DEBUG("Unbind from stub skipped for PCI device %s because of "
> -                  "unknown stub driver", dev->name);
> -        goto remove_slot;
> -    }
> -
> -    VIR_DEBUG("Unbinding PCI device %s from stub driver %s",
> -              dev->name, driver);
> -
> -    if (virPCIDeviceUnbind(dev) < 0)
> -        goto cleanup;
> -    dev->unbind_from_stub = false;
> -
> - remove_slot:
> -    if (!dev->remove_slot) {
> -        VIR_DEBUG("Slot removal skipped for PCI device %s", dev->name);
> -        goto reprobe;
> -    }
> -
> -    VIR_DEBUG("Removing slot for PCI device %s", dev->name);
> -
> -    /* Xen's pciback.ko wants you to use remove_slot on the specific device */
> -    if (!(path = virPCIDriverFile(driver, "remove_slot")))
> -        goto cleanup;
> -
> -    if (virFileExists(path) && virFileWriteStr(path, dev->name, 0) < 0) {
> -        virReportSystemError(errno,
> -                             _("Failed to remove slot for PCI device '%s' from %s"),
> -                             dev->name, driver);
> -        goto cleanup;
> -    }
> -    dev->remove_slot = false;
> -
> - reprobe:
> -    if (!dev->reprobe) {
> -        VIR_DEBUG("Reprobe skipped for PCI device %s", dev->name);
> -        result = 0;
> -        goto cleanup;
> -    }
> -
> -    VIR_DEBUG("Reprobing for PCI device %s", dev->name);
> -
> -    /* Trigger a re-probe of the device is not in the stub's dynamic
> -     * ID table. If the stub is available, but 'remove_id' isn't
> -     * available, then re-probing would just cause the device to be
> -     * re-bound to the stub.
> -     */
> -    VIR_FREE(path);
> -    if (driver && !(path = virPCIDriverFile(driver, "remove_id")))
> -        goto cleanup;
> -
> -    if (!driver || !virFileExists(drvdir) || virFileExists(path)) {
> -        if (virFileWriteStr(PCI_SYSFS "drivers_probe", dev->name, 0) < 0) {
> -            virReportSystemError(errno,
> -                                 _("Failed to trigger a re-probe for PCI device '%s'"),
> -                                 dev->name);
> -            goto cleanup;
> -        }
> -    }
> -
> -    result = 0;
> -
> - cleanup:
> -    /* do not do it again */
> -    dev->unbind_from_stub = false;
> -    dev->remove_slot = false;
> -    dev->reprobe = false;
> -
> -    return result;
> -}
> -
>   static int
>   virPCIDeviceUnbindFromStubWithOverride(virPCIDevicePtr dev)
>   {
> @@ -1257,167 +1149,7 @@ virPCIDeviceUnbindFromStubWithOverride(virPCIDevicePtr dev)
>   static int
>   virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
>   {
> -    VIR_AUTOFREE(char *) path = NULL;
> -
> -    /*
> -     * Prefer using the device's driver_override interface, falling back
> -     * to the unpleasant new_id interface.
> -     */
> -    if (!(path = virPCIFile(dev->name, "driver_override")))
> -        return -1;
> -
> -    if (virFileExists(path))
> -        return virPCIDeviceUnbindFromStubWithOverride(dev);
> -
> -    return virPCIDeviceUnbindFromStubWithNewid(dev);
> -}
> -
> -static int
> -virPCIDeviceBindToStubWithNewid(virPCIDevicePtr dev)
> -{
> -    int result = -1;
> -    bool reprobe = false;
> -    VIR_AUTOFREE(char *) stubDriverPath = NULL;
> -    VIR_AUTOFREE(char *) driverLink = NULL;
> -    VIR_AUTOFREE(char *) path = NULL; /* reused for different purposes */
> -    VIR_AUTOPTR(virError) err = NULL;
> -    const char *stubDriverName = NULL;
> -
> -    /* Check the device is configured to use one of the known stub drivers */
> -    if (dev->stubDriver == VIR_PCI_STUB_DRIVER_NONE) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("No stub driver configured for PCI device %s"),
> -                       dev->name);
> -        return -1;
> -    } else if (!(stubDriverName = virPCIStubDriverTypeToString(dev->stubDriver))) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("Unknown stub driver configured for PCI device %s"),
> -                       dev->name);
> -        return -1;
> -    }
> -
> -    if (!(stubDriverPath = virPCIDriverDir(stubDriverName))  ||
> -        !(driverLink = virPCIFile(dev->name, "driver")))
> -        goto cleanup;
> -
> -    if (virFileExists(driverLink)) {
> -        if (virFileLinkPointsTo(driverLink, stubDriverPath)) {
> -            /* The device is already bound to the correct driver */
> -            VIR_DEBUG("Device %s is already bound to %s",
> -                      dev->name, stubDriverName);
> -            result = 0;
> -            goto cleanup;
> -        }
> -        reprobe = true;
> -    }
> -
> -    /* Add the PCI device ID to the stub's dynamic ID table;
> -     * this is needed to allow us to bind the device to the stub.
> -     * Note: if the device is not currently bound to any driver,
> -     * stub will immediately be bound to the device. Also, note
> -     * that if a new device with this ID is hotplugged, or if a probe
> -     * is triggered for such a device, it will also be immediately
> -     * bound by the stub.
> -     */
> -    if (!(path = virPCIDriverFile(stubDriverName, "new_id")))
> -        goto cleanup;
> -
> -    if (virFileWriteStr(path, dev->id, 0) < 0) {
> -        virReportSystemError(errno,
> -                             _("Failed to add PCI device ID '%s' to %s"),
> -                             dev->id, stubDriverName);
> -        goto cleanup;
> -    }
> -
> -    /* check whether the device is bound to pci-stub when we write dev->id to
> -     * ${stubDriver}/new_id.
> -     */
> -    if (virFileLinkPointsTo(driverLink, stubDriverPath)) {
> -        dev->unbind_from_stub = true;
> -        dev->remove_slot = true;
> -        result = 0;
> -        goto remove_id;
> -    }
> -
> -    if (virPCIDeviceUnbind(dev) < 0)
> -        goto remove_id;
> -
> -    /* If the device was bound to a driver we'll need to reprobe later */
> -    dev->reprobe = reprobe;
> -
> -    /* If the device isn't already bound to pci-stub, try binding it now.
> -     */
> -    if (!virFileLinkPointsTo(driverLink, stubDriverPath)) {
> -        /* Xen's pciback.ko wants you to use new_slot first */
> -        VIR_FREE(path);
> -        if (!(path = virPCIDriverFile(stubDriverName, "new_slot")))
> -            goto remove_id;
> -
> -        if (virFileExists(path) && virFileWriteStr(path, dev->name, 0) < 0) {
> -            virReportSystemError(errno,
> -                                 _("Failed to add slot for "
> -                                   "PCI device '%s' to %s"),
> -                                 dev->name, stubDriverName);
> -            goto remove_id;
> -        }
> -        dev->remove_slot = true;
> -
> -        VIR_FREE(path);
> -        if (!(path = virPCIDriverFile(stubDriverName, "bind")))
> -            goto remove_id;
> -
> -        if (virFileWriteStr(path, dev->name, 0) < 0) {
> -            virReportSystemError(errno,
> -                                 _("Failed to bind PCI device '%s' to %s"),
> -                                 dev->name, stubDriverName);
> -            goto remove_id;
> -        }
> -        dev->unbind_from_stub = true;
> -    }
> -
> -    result = 0;
> -
> - remove_id:
> -    err = virSaveLastError();
> -
> -    /* If 'remove_id' exists, remove the device id from pci-stub's dynamic
> -     * ID table so that 'drivers_probe' works below.
> -     */
> -    VIR_FREE(path);
> -    if (!(path = virPCIDriverFile(stubDriverName, "remove_id"))) {
> -        /* We do not remove PCI ID from pci-stub, and we cannot reprobe it */
> -        if (dev->reprobe) {
> -            VIR_WARN("Could not remove PCI ID '%s' from %s, and the device "
> -                     "cannot be probed again.", dev->id, stubDriverName);
> -        }
> -        dev->reprobe = false;
> -        result = -1;
> -        goto cleanup;
> -    }
> -
> -    if (virFileExists(path) && virFileWriteStr(path, dev->id, 0) < 0) {
> -        virReportSystemError(errno,
> -                             _("Failed to remove PCI ID '%s' from %s"),
> -                             dev->id, stubDriverName);
> -
> -        /* remove PCI ID from pci-stub failed, and we cannot reprobe it */
> -        if (dev->reprobe) {
> -            VIR_WARN("Failed to remove PCI ID '%s' from %s, and the device "
> -                     "cannot be probed again.", dev->id, stubDriverName);
> -        }
> -        dev->reprobe = false;
> -        result = -1;
> -        goto cleanup;
> -    }
> -
> - cleanup:
> -    if (result < 0)
> -        virPCIDeviceUnbindFromStub(dev);
> -
> -    if (err)
> -        virSetError(err);
> -
> -    return result;
> +    return virPCIDeviceUnbindFromStubWithOverride(dev);
>   }
>   
>   static int
> @@ -1463,19 +1195,7 @@ virPCIDeviceBindToStubWithOverride(virPCIDevicePtr dev)
>   static int
>   virPCIDeviceBindToStub(virPCIDevicePtr dev)
>   {
> -    VIR_AUTOFREE(char *) path = NULL;
> -
> -    /*
> -     * Prefer using the device's driver_override interface, falling back
> -     * to the unpleasant new_id interface.
> -     */
> -    if (!(path = virPCIFile(dev->name, "driver_override")))
> -        return -1;
> -
> -    if (virFileExists(path))
> -        return virPCIDeviceBindToStubWithOverride(dev);
> -
> -    return virPCIDeviceBindToStubWithNewid(dev);
> +    return virPCIDeviceBindToStubWithOverride(dev);
>   }
>   
>   /* virPCIDeviceDetach:




More information about the libvir-list mailing list