[libvirt] [PATCH] fix the failure of nodedev-reattach caused by the variables from nodedev-attach
ghostwcy
ghostwcy at gmail.com
Fri Jul 1 15:30:42 UTC 2011
At 2011-7-1 18:24, Guannan Ren write:
> the "virsh nodedev-reattch" command could return successfully, but the pci device is still
> bound to pci-stub driver. The reason is noddev-reattach trys to use the variables set by
> nodedev-dettach commands. Becuase these variables is not persistent, this is not we expected.
> the patch try to fix it.
I do not agree with this patch. You should read this mail:
https://www.redhat.com/archives/libvir-list/2011-April/msg00315.html
This patch will cause regression about hotpluging host pci device.
I think the problem is that: the init value of these variables is wrong.
We can fix this bug by modifing pciGetDevice() or its caller.
>
> ---
> src/util/pci.c | 74 +++++++++++++++++++------------------------------------
> 1 files changed, 26 insertions(+), 48 deletions(-)
>
> diff --git a/src/util/pci.c b/src/util/pci.c
> index 46a3a83..d345c3e 100644
> --- a/src/util/pci.c
> +++ b/src/util/pci.c
> @@ -67,9 +67,6 @@ struct _pciDevice {
> unsigned managed : 1;
>
> /* used by reattach function */
> - unsigned unbind_from_stub : 1;
> - unsigned remove_slot : 1;
> - unsigned reprobe : 1;
> };
>
> struct _pciDeviceList {
> @@ -882,14 +879,23 @@ pciUnbindDeviceFromStub(pciDevice *dev, const char *driver)
> if (pciDriverDir(&drvdir, driver)< 0)
> goto cleanup;
>
> - if (!dev->unbind_from_stub)
> - goto remove_slot;
> -
> - /* If the device is bound to stub, unbind it.
> - */
> if (pciDeviceFile(&path, dev->name, "driver")< 0)
> goto cleanup;
>
> + /* If the device is bound to a driver instead of pci-stub, do nothing.
> + */
> + if (virFileExists(path)&& ! virFileLinkPointsTo(path, drvdir)){
> + result = 0;
> + goto cleanup;
> + }
> +
> + /* If the device is not bound to any driver, reprobe it.
> + */
> + if (!virFileExists(path))
> + goto reprobe;
> +
> + /* If the device is bound to stub, unbind it.
> + */
> if (virFileExists(drvdir)&& virFileLinkPointsTo(path, drvdir)) {
> if (pciDriverFile(&path, driver, "unbind")< 0) {
> goto cleanup;
> @@ -902,11 +908,6 @@ pciUnbindDeviceFromStub(pciDevice *dev, const char *driver)
> goto cleanup;
> }
> }
> - dev->unbind_from_stub = 0;
> -
> -remove_slot:
> - if (!dev->remove_slot)
> - goto reprobe;
>
> /* Xen's pciback.ko wants you to use remove_slot on the specific device */
> if (pciDriverFile(&path, driver, "remove_slot")< 0) {
> @@ -919,14 +920,8 @@ remove_slot:
> dev->name, driver);
> goto cleanup;
> }
> - dev->remove_slot = 0;
>
> reprobe:
> - if (!dev->reprobe) {
> - result = 0;
> - goto cleanup;
> - }
> -
> /* 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
> @@ -935,6 +930,12 @@ reprobe:
> if (pciDriverFile(&path, driver, "remove_id")< 0) {
> goto cleanup;
> }
> +
> + /* If the device is still in stub's dynamic ID table,remove it,
> + * otherwise, ignore the error.
> + */
> + if (virFileExists(path)&& virFileWriteStr(path, dev->name, 0)< 0){
> + }
>
> if (!virFileExists(drvdir) || virFileExists(path)) {
> if (virFileWriteStr(PCI_SYSFS "drivers_probe", dev->name, 0)< 0) {
> @@ -948,11 +949,6 @@ reprobe:
> result = 0;
>
> cleanup:
> - /* do not do it again */
> - dev->unbind_from_stub = 0;
> - dev->remove_slot = 0;
> - dev->reprobe = 0;
> -
> VIR_FREE(drvdir);
> VIR_FREE(path);
>
> @@ -966,7 +962,6 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver)
> int result = -1;
> char *drvdir = NULL;
> char *path = NULL;
> - int reprobe = 0;
>
> /* check whether the device is already bound to a driver */
> if (pciDriverDir(&drvdir, driver)< 0 ||
> @@ -980,7 +975,6 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver)
> result = 0;
> goto cleanup;
> }
> - reprobe = 1;
> }
>
> /* Add the PCI device ID to the stub's dynamic ID table;
> @@ -1011,8 +1005,7 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver)
> }
>
> if (virFileLinkPointsTo(path, drvdir)) {
> - dev->unbind_from_stub = 1;
> - dev->remove_slot = 1;
> + result = 0;
> goto remove_id;
> }
>
> @@ -1022,7 +1015,7 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver)
> * your root filesystem.
> */
> if (pciDeviceFile(&path, dev->name, "driver/unbind")< 0) {
> - goto cleanup;
> + goto remove_id;
> }
>
> if (virFileExists(path)) {
> @@ -1030,9 +1023,8 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver)
> virReportSystemError(errno,
> _("Failed to unbind PCI device '%s'"),
> dev->name);
> - goto cleanup;
> + goto remove_id;
> }
> - dev->reprobe = reprobe;
> }
>
> /* If the device isn't already bound to pci-stub, try binding it now.
> @@ -1054,7 +1046,6 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver)
> dev->name, driver);
> goto remove_id;
> }
> - dev->remove_slot = 1;
>
> if (pciDriverFile(&path, driver, "bind")< 0) {
> goto remove_id;
> @@ -1066,20 +1057,15 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver)
> dev->name, driver);
> goto remove_id;
> }
> - dev->unbind_from_stub = 1;
> }
>
> + result = 0;
> remove_id:
> /* If 'remove_id' exists, remove the device id from pci-stub's dynamic
> * ID table so that 'drivers_probe' works below.
> */
> if (pciDriverFile(&path, driver, "remove_id")< 0) {
> - /* 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, driver);
> - }
> - dev->reprobe = 0;
> + result = -1;
> goto cleanup;
> }
>
> @@ -1087,18 +1073,10 @@ remove_id:
> virReportSystemError(errno,
> _("Failed to remove PCI ID '%s' from %s"),
> dev->id, driver);
> -
> - /* 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, driver);
> - }
> - dev->reprobe = 0;
> + result = -1;
> goto cleanup;
> }
>
> - result = 0;
> -
> cleanup:
> VIR_FREE(drvdir);
> VIR_FREE(path);
More information about the libvir-list
mailing list