[libvirt] [PATCH 2/9] pci: Fix failure paths in detach
https://bugzilla.redhat.com/show_bug.cgi?id=1046919
Since commit v0.9.0-47-g4e8969e (released in 0.9.1) some failures during
device detach were reported to callers of virPCIDeviceBindToStub as
success. For example, even though a device seemed to be detached
virsh # nodedev-detach pci_0000_07_05_0 --driver vfio
Device pci_0000_07_05_0 detached
one could find similar message in libvirt logs:
Failed to bind PCI device '0000:07:05.0' to vfio-pci: No such device
This patch fixes these paths and also avoids overwriting real errors
with errors encountered during a cleanup phase.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/util/virpci.c | 39 +++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 8577fd4..f6ab794 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1123,14 +1123,15 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev,
{
int result = -1;
int reprobe = false;
-
char *stubDriverPath = NULL;
char *driverLink = NULL;
char *path = NULL; /* reused for different purposes */
- const char *newDriverName = NULL;
+ char *newDriverName = NULL;
+ virErrorPtr err = NULL;
if (virPCIDriverDir(&stubDriverPath, stubDriverName) < 0 ||
- virPCIFile(&driverLink, dev->name, "driver") < 0)
+ virPCIFile(&driverLink, dev->name, "driver") < 0 ||
+ VIR_STRDUP(newDriverName, stubDriverName) < 0)
goto cleanup;
if (virFileExists(driverLink)) {
@@ -1138,7 +1139,6 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev,
/* The device is already bound to the correct driver */
VIR_DEBUG("Device %s is already bound to %s",
dev->name, stubDriverName);
- newDriverName = stubDriverName;
result = 0;
goto cleanup;
}
@@ -1170,6 +1170,7 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev,
if (virFileLinkPointsTo(driverLink, stubDriverPath)) {
dev->unbind_from_stub = true;
dev->remove_slot = true;
+ result = 0;
goto remove_id;
}
@@ -1178,16 +1179,15 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev,
* PCI device happens to be IDE controller for the disk hosting
* your root filesystem.
*/
- if (virPCIFile(&path, dev->name, "driver/unbind") < 0) {
- goto cleanup;
- }
+ if (virPCIFile(&path, dev->name, "driver/unbind") < 0)
+ goto remove_id;
if (virFileExists(path)) {
if (virFileWriteStr(path, dev->name, 0) < 0) {
virReportSystemError(errno,
_("Failed to unbind PCI device
'%s'"),
dev->name);
- goto cleanup;
+ goto remove_id;
}
dev->reprobe = reprobe;
}
@@ -1222,7 +1222,11 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev,
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.
*/
@@ -1233,6 +1237,7 @@ remove_id:
"cannot be probed again.", dev->id, stubDriverName);
}
dev->reprobe = false;
+ result = -1;
goto cleanup;
}
@@ -1247,24 +1252,26 @@ remove_id:
"cannot be probed again.", dev->id, stubDriverName);
}
dev->reprobe = false;
+ result = -1;
goto cleanup;
}
- newDriverName = stubDriverName;
- result = 0;
-
cleanup:
VIR_FREE(stubDriverPath);
VIR_FREE(driverLink);
VIR_FREE(path);
- if (newDriverName &&
- STRNEQ_NULLABLE(dev->stubDriver, newDriverName)) {
+ if (result < 0) {
+ VIR_FREE(newDriverName);
+ virPCIDeviceUnbindFromStub(dev);
+ } else {
VIR_FREE(dev->stubDriver);
- result = VIR_STRDUP(dev->stubDriver, newDriverName);
+ dev->stubDriver = newDriverName;
}
- if (result < 0)
- virPCIDeviceUnbindFromStub(dev);
+
+ if (err)
+ virSetError(err);
+ virFreeError(err);
return result;
}
--
1.8.5.3