From ff7bc70a5e5d7aaf354ce501653beb82429030fe Mon Sep 17 00:00:00 2001
Message-Id: <ff7bc70a5e5d7aaf354ce501653beb82429030fe@dist-git>
From: Laine Stump <laine@laine.org>
Date: Thu, 11 Apr 2019 15:14:46 -0400
Subject: [PATCH] qemu_hotplug: separate Chr|Lease from other devices in
 DetachDevice switch

The Chr and Lease devices have detach code that is too different from
the other device types to handle with common functionality (which will
soon be added at the end of qemuDomainDetachDeviceLive(). In order to
make this difference obvious, move the cases for those two device
types to the top of the switch statement in
qemuDomainDetachDeviceLive(), have the cases return immediately so the
future common code at the end of the function will be skipped, and
also include some hopefully helpful comments to remind future
maintainers why these two device types are treated differently.

Any attempt to detach an unsupported device type should also skip the
future common code at the end of the function, so the case for
unsupported types is similarly changed from a simple break to a return
-1.

Signed-off-by: Laine Stump <laine@laine.org>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
(cherry picked from commit 2ec6faea798b2a7d8986b7a958e781b54d8a8070)

Partially-Resolves: https://bugzilla.redhat.com/1658198
Signed-off-by: Laine Stump <laine@redhat.com>
Signed-off-by: Laine Stump <laine@laine.org>
Message-Id: <20190411191453.24055-35-laine@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_hotplug.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index a655fc391f..9c0ee1c6a5 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5657,24 +5657,36 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
     int ret = -1;
 
     switch ((virDomainDeviceType)match->type) {
+        /*
+         * lease and chr devices don't follow the standard pattern of
+         * the others, so they must have their own self-contained
+         * Detach functions.
+         */
+    case VIR_DOMAIN_DEVICE_LEASE:
+        return qemuDomainDetachLease(driver, vm, match->data.lease);
+
+    case VIR_DOMAIN_DEVICE_CHR:
+        return qemuDomainDetachChrDevice(driver, vm, match->data.chr, async);
+
+        /*
+         * All the other device types follow a very similar pattern -
+         * First we call type-specific functions to 1) locate the
+         * device we want to detach (based on the prototype device in
+         * match) and 2) do any device-type-specific validation to
+         * assure it is okay to detach the device.
+         */
     case VIR_DOMAIN_DEVICE_DISK:
         ret = qemuDomainDetachDeviceDiskLive(driver, vm, match, async);
         break;
     case VIR_DOMAIN_DEVICE_CONTROLLER:
         ret = qemuDomainDetachControllerDevice(driver, vm, match, async);
         break;
-    case VIR_DOMAIN_DEVICE_LEASE:
-        ret = qemuDomainDetachLease(driver, vm, match->data.lease);
-        break;
     case VIR_DOMAIN_DEVICE_NET:
         ret = qemuDomainDetachNetDevice(driver, vm, match, async);
         break;
     case VIR_DOMAIN_DEVICE_HOSTDEV:
         ret = qemuDomainDetachHostDevice(driver, vm, match, async);
         break;
-    case VIR_DOMAIN_DEVICE_CHR:
-        ret = qemuDomainDetachChrDevice(driver, vm, match->data.chr, async);
-        break;
     case VIR_DOMAIN_DEVICE_RNG:
         ret = qemuDomainDetachRNGDevice(driver, vm, match->data.rng, async);
         break;
@@ -5714,7 +5726,7 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
                        _("live detach of device '%s' is not supported"),
                        virDomainDeviceTypeToString(match->type));
-        break;
+        return -1;
     }
 
     return ret;
-- 
2.21.0