[PATCH v2 1/3] qemu: use a bigger unplug timeout for PPC64 guests

[Daniel Henrique Barboza <danielhb413@xxxxxxxxx>]

For some architectures and setups, device removal can take
longer than the default 5 seconds. This results in commands
such as 'virsh setvcpus' to fire timeout messages even if
the operation were successful in the guest, confusing the
user.

This patch sets a new 10 seconds unplug timeout for PPC64
guests. All other archs will keep the default 5 seconds
timeout.

Instead of putting 'if PPC64' conditionals inside qemu_hotplug.c
to set the new timeout value, a new QEMU driver attribute
'unplugTimeout' was added. The timeout value is set during
qemuStateInitialize only once. All qemu_hotplug.c functions
that uses the timeout have easy access to a qemu_driver object,
thus the change to use unplugTimeout is straightforward.

The now unused 'qemuDomainRemoveDeviceWaitTime' global can't
be simply erased from qemu_hotplug.c though. Next patch will
remove it properly.

Signed-off-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
---
 src/qemu/qemu_conf.h    |  3 +++
 src/qemu/qemu_driver.c  | 11 +++++++++++
 src/qemu/qemu_hotplug.c | 11 ++++++-----
 tests/qemuhotplugtest.c |  2 +-
 4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 8473d6d4ca..edda1421d0 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -296,6 +296,9 @@ struct _virQEMUDriver {
 
     /* Immutable pointer, self-locking APIs */
     virHashAtomicPtr migrationErrors;
+
+    /* Immutable value */
+    unsigned int unplugTimeout;
 };
 
 virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1e041a8bac..2ebe22ee79 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -134,6 +134,13 @@ VIR_LOG_INIT("qemu.qemu_driver");
 
 #define QEMU_NB_BANDWIDTH_PARAM 7
 
+/* Timeout in miliseconds for device removal. PPC64 domains
+ * can experience a bigger delay in unplug operations during
+ * heavy guest activity (vcpu being the most notable case), thus
+ * the timeout for PPC64 is also bigger. */
+#define QEMU_UNPLUG_TIMEOUT 5000
+#define QEMU_UNPLUG_TIMEOUT_PPC64 10000
+
 static void qemuProcessEventHandler(void *data, void *opaque);
 
 static int qemuStateCleanup(void);
@@ -1095,6 +1102,10 @@ qemuStateInitialize(bool privileged,
     if (!qemu_driver->workerPool)
         goto error;
 
+    qemu_driver->unplugTimeout = QEMU_UNPLUG_TIMEOUT;
+    if (ARCH_IS_PPC64(qemu_driver->caps->host.arch))
+        qemu_driver->unplugTimeout = QEMU_UNPLUG_TIMEOUT_PPC64;
+
     qemuProcessReconnectAll(qemu_driver);
 
     qemuAutostartDomains(qemu_driver);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index bd8868b0f7..4088cc5e8e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5237,7 +5237,8 @@ qemuDomainResetDeviceRemoval(virDomainObjPtr vm)
  *      - we failed to reliably wait for the event and thus use fallback behavior
  */
 static int
-qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
+qemuDomainWaitForDeviceRemoval(virQEMUDriverPtr driver,
+                               virDomainObjPtr vm)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
     unsigned long long until;
@@ -5245,7 +5246,7 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
 
     if (virTimeMillisNow(&until) < 0)
         return 1;
-    until += qemuDomainRemoveDeviceWaitTime;
+    until += driver->unplugTimeout;
 
     while (priv->unplug.alias) {
         if ((rc = virDomainObjWaitUntil(vm, until)) == 1)
@@ -5701,7 +5702,7 @@ qemuDomainDetachDeviceChr(virQEMUDriverPtr driver,
     } else if (async) {
         ret = 0;
     } else {
-        if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
+        if ((ret = qemuDomainWaitForDeviceRemoval(driver, vm)) == 1)
             ret = qemuDomainRemoveChrDevice(driver, vm, tmpChr, true);
     }
 
@@ -6001,7 +6002,7 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
     if (async) {
         ret = 0;
     } else {
-        if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
+        if ((ret = qemuDomainWaitForDeviceRemoval(driver, vm)) == 1)
             ret = qemuDomainRemoveDevice(driver, vm, &detach);
     }
 
@@ -6107,7 +6108,7 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver,
         goto cleanup;
     }
 
-    if ((rc = qemuDomainWaitForDeviceRemoval(vm)) <= 0) {
+    if ((rc = qemuDomainWaitForDeviceRemoval(driver, vm)) <= 0) {
         if (rc == 0)
             virReportError(VIR_ERR_OPERATION_FAILED, "%s",
                            _("vcpu unplug request timed out"));
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 3c177c6622..da6258991d 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -645,7 +645,7 @@ mymain(void)
     driver.hostdevMgr = virHostdevManagerGetDefault();
 
     /* wait only 100ms for DEVICE_DELETED event */
-    qemuDomainRemoveDeviceWaitTime = 100;
+    driver.unplugTimeout = 100;
 
 #define DO_TEST(file, ACTION, dev, fial, kep, ...) \
     do { \
-- 
2.21.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list