[libvirt] [PATCH 2/7] qemu: hotplug: Refactor semantics of qemuDomainWaitForDeviceRemoval

[Peter Krempa]

Neither of the callers cares whether the DEVICE_DELETED event isn't
supported or the event was received. Simplify the code and callers by
unifying the two values and changing the return value constants so that
a temporary variable can be omitted.
---
 src/qemu/qemu_hotplug.c | 67 +++++++++++++++----------------------------------
 1 file changed, 20 insertions(+), 47 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6c619e9..7317089 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3351,11 +3351,13 @@ qemuDomainResetDeviceRemoval(virDomainObjPtr vm)
 }

 /* Returns:
- *   0 when DEVICE_DELETED event is unsupported, or we failed to reliably wait
- *   for the event
- *   1 when DEVICE_DELETED arrived before the timeout and the caller is
- *     responsible for finishing the removal
- *   2 device removal did not finish in qemuDomainRemoveDeviceWaitTime
+ *   0 DEVICE_DELETED event is supported and removal of the device did not
+ *     finish in qemuDomainRemoveDeviceWaitTime
+ *
+ *   1 when the caller is responsible for finishing the device removal:
+ *      - DEVICE_DELETED event is unsupported
+ *      - DEVICE_DELETED event arrived before the timeout time
+ *      - we failed to reliably wait for the event and thus use fallback behavior
  */
 static int
 qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
@@ -3364,21 +3366,21 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
     unsigned long long until;

     if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT))
-        return 0;
+        return 1;

     if (virTimeMillisNow(&until) < 0)
-        return 0;
+        return 1;
     until += qemuDomainRemoveDeviceWaitTime;

     while (priv->unpluggingDevice) {
         if (virCondWaitUntil(&priv->unplugFinished,
                              &vm->parent.lock, until) < 0) {
             if (errno == ETIMEDOUT) {
-                return 2;
+                return 0;
             } else {
                 VIR_WARN("Failed to wait on unplug condition for domain '%s' "
                          "device '%s'", vm->def->name, priv->unpluggingDevice);
-                return 0;
+                return 1;
             }
         }
     }
@@ -3414,7 +3416,6 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
 {
     int ret = -1;
     qemuDomainObjPrivatePtr priv = vm->privateData;
-    int rc;

     if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) {
         virReportError(VIR_ERR_OPERATION_FAILED,
@@ -3468,11 +3469,8 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
         goto cleanup;

-    rc = qemuDomainWaitForDeviceRemoval(vm);
-    if (rc == 0 || rc == 1)
+    if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
         ret = qemuDomainRemoveDiskDevice(driver, vm, detach);
-    else
-        ret = 0;

  cleanup:
     qemuDomainResetDeviceRemoval(vm);
@@ -3486,7 +3484,6 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver,
 {
     int ret = -1;
     qemuDomainObjPrivatePtr priv = vm->privateData;
-    int rc;

     if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
         virReportError(VIR_ERR_OPERATION_FAILED,
@@ -3514,11 +3511,8 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver,
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
         goto cleanup;

-    rc = qemuDomainWaitForDeviceRemoval(vm);
-    if (rc == 0 || rc == 1)
+    if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
         ret = qemuDomainRemoveDiskDevice(driver, vm, detach);
-    else
-        ret = 0;

  cleanup:
     qemuDomainResetDeviceRemoval(vm);
@@ -3634,7 +3628,6 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
     int idx, ret = -1;
     virDomainControllerDefPtr detach = NULL;
     qemuDomainObjPrivatePtr priv = vm->privateData;
-    int rc;

     if ((idx = virDomainControllerFind(vm->def,
                                        dev->data.controller->type,
@@ -3702,11 +3695,8 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
         goto cleanup;

-    rc = qemuDomainWaitForDeviceRemoval(vm);
-    if (rc == 0 || rc == 1)
+    if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
         ret = qemuDomainRemoveControllerDevice(driver, vm, detach);
-    else
-        ret = 0;

  cleanup:
     qemuDomainResetDeviceRemoval(vm);
@@ -3846,10 +3836,8 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
     if (ret < 0) {
         if (virDomainObjIsActive(vm))
             virDomainAuditHostdev(vm, detach, "detach", false);
-    } else {
-        int rc = qemuDomainWaitForDeviceRemoval(vm);
-        if (rc == 0 || rc == 1)
-            ret = qemuDomainRemoveHostDevice(driver, vm, detach);
+    } else if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) {
+        ret = qemuDomainRemoveHostDevice(driver, vm, detach);
     }

     qemuDomainResetDeviceRemoval(vm);
@@ -3940,7 +3928,6 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
     int detachidx, ret = -1;
     virDomainNetDefPtr detach = NULL;
     qemuDomainObjPrivatePtr priv = vm->privateData;
-    int rc;

     if ((detachidx = virDomainNetFindIdx(vm->def, dev->data.net)) < 0)
         goto cleanup;
@@ -4017,11 +4004,8 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
         goto cleanup;

-    rc = qemuDomainWaitForDeviceRemoval(vm);
-    if (rc == 0 || rc == 1)
+    if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
         ret = qemuDomainRemoveNetDevice(driver, vm, detach);
-    else
-        ret = 0;

  cleanup:
     qemuDomainResetDeviceRemoval(vm);
@@ -4157,7 +4141,6 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
     virDomainDefPtr vmdef = vm->def;
     virDomainChrDefPtr tmpChr;
     char *devstr = NULL;
-    int rc;

     if (!(tmpChr = virDomainChrFind(vmdef, chr))) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -4189,15 +4172,11 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
         goto cleanup;

-    rc = qemuDomainWaitForDeviceRemoval(vm);
-    if (rc == 0 || rc == 1) {
+    if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) {
         qemuDomainReleaseDeviceAddress(vm, &tmpChr->info, NULL);
         ret = qemuDomainRemoveChrDevice(driver, vm, tmpChr);
-    } else {
-        ret = 0;
     }

-
  cleanup:
     qemuDomainResetDeviceRemoval(vm);
     VIR_FREE(devstr);
@@ -4243,11 +4222,8 @@ qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
     if (qemuDomainObjExitMonitor(driver, vm) || rc < 0)
         goto cleanup;

-    rc = qemuDomainWaitForDeviceRemoval(vm);
-    if (rc == 0 || rc == 1)
+    if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
         ret = qemuDomainRemoveRNGDevice(driver, vm, tmpRNG);
-    else
-        ret = 0;

  cleanup:
     qemuDomainResetDeviceRemoval(vm);
@@ -4295,11 +4271,8 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver,
     if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
         goto cleanup;

-    rc = qemuDomainWaitForDeviceRemoval(vm);
-    if (rc == 0 || rc == 1)
+    if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
         ret = qemuDomainRemoveMemoryDevice(driver, vm, mem);
-    else
-        ret = 0;

  cleanup:
     qemuDomainResetDeviceRemoval(vm);
-- 
2.8.0