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