From c5a1547cc5aa9fed95f2bb48d9dcb4c659e640d1 Mon Sep 17 00:00:00 2001
Message-Id: <c5a1547cc5aa9fed95f2bb48d9dcb4c659e640d1.1390394206.git.jdenemar@redhat.com>
From: Jiri Denemark <jdenemar@redhat.com>
Date: Mon, 20 Jan 2014 14:41:12 +0100
Subject: [PATCH] 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.

(cherry picked from commit d8ab981bdd137f15675ee0d101abeabf42680cc1)

Signed-off-by: Jiri Denemark <jdenemar@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 b48e433..f689961 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1091,16 +1091,17 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev,
 {
     int result = -1;
     int reprobe = false;
-
     char *stubDriverPath = NULL;
     char *driverLink = NULL;
     char *oldDriverPath = NULL;
     char *oldDriverName = 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)) {
@@ -1108,7 +1109,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;
         }
@@ -1140,6 +1140,7 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev,
     if (virFileLinkPointsTo(driverLink, stubDriverPath)) {
         dev->unbind_from_stub = true;
         dev->remove_slot = true;
+        result = 0;
         goto remove_id;
     }
 
@@ -1148,16 +1149,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;
     }
@@ -1192,7 +1192,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.
      */
@@ -1203,6 +1207,7 @@ remove_id:
                      "cannot be probed again.", dev->id, stubDriverName);
         }
         dev->reprobe = false;
+        result = -1;
         goto cleanup;
     }
 
@@ -1217,12 +1222,10 @@ 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);
@@ -1230,13 +1233,17 @@ cleanup:
     VIR_FREE(oldDriverName);
     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