1:36 p.m.
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 function called
qemuDomainGetUnplugTimeout was added. The timeout value is then
retrieved when needed, by passing the correspondent DomainDef
object. This approach allows for different guest architectures
to have distint unplug timeout intervals, regardless of the
host architecture. This design also makes it easier to
modify/enhance the unplug timeout logic in the future
(allow for special timeouts for TCG domains, for example).
A new mock file was created to work with qemuhotplugtest.c,
given that the test timeout is significantly shorter than
the actual timeout value in qemu_hotplug.c.
The now unused 'qemuDomainRemoveDeviceWaitTime' global can't
be simply erased from qemu_hotplug.c though. Next patch will
remove it properly.
Suggested-by: Cole Robinson <crobinso(a)redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
---
src/qemu/qemu_hotplug.c | 20 +++++++++++++++++++-
src/qemu/qemu_hotplug.h | 2 ++
tests/Makefile.am | 13 ++++++++++++-
tests/qemuhotplugmock.c | 33 +++++++++++++++++++++++++++++++++
tests/qemuhotplugtest.c | 3 ++-
5 files changed, 68 insertions(+), 3 deletions(-)
create mode 100644 tests/qemuhotplugmock.c
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index bf301919cc..a7955e8062 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -63,6 +63,13 @@ VIR_LOG_INIT("qemu.qemu_hotplug");
#define CHANGE_MEDIA_TIMEOUT 5000
+/* 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 1000ull * 5
+#define QEMU_UNPLUG_TIMEOUT_PPC64 1000ull * 10
+
/* Wait up to 5 seconds for device removal to finish. */
unsigned long long qemuDomainRemoveDeviceWaitTime = 1000ull * 5;
@@ -5112,6 +5119,17 @@ qemuDomainResetDeviceRemoval(virDomainObjPtr vm)
priv->unplug.eventSeen = false;
}
+
+unsigned long long
+qemuDomainGetUnplugTimeout(virDomainObjPtr vm)
+{
+ if (qemuDomainIsPSeries(vm->def))
+ return QEMU_UNPLUG_TIMEOUT_PPC64;
+
+ return QEMU_UNPLUG_TIMEOUT;
+}
+
+
/* Returns:
* -1 Unplug of the device failed
*
@@ -5130,7 +5148,7 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
if (virTimeMillisNow(&until) < 0)
return 1;
- until += qemuDomainRemoveDeviceWaitTime;
+ until += qemuDomainGetUnplugTimeout(vm);
while (priv->unplug.alias) {
if ((rc = virDomainObjWaitUntil(vm, until)) == 1)
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 6d2cd34dbc..1dfc601110 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -161,3 +161,5 @@ int qemuDomainDetachDBusVMState(virQEMUDriverPtr driver,
virDomainObjPtr vm,
const char *id,
qemuDomainAsyncJob asyncJob);
+
+unsigned long long qemuDomainGetUnplugTimeout(virDomainObjPtr vm);
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a9acd88670..8674b1f9da 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -223,6 +223,7 @@ test_libraries = libshunload.la \
libvirhostcpumock.la \
libdomaincapsmock.la \
libvirfilecachemock.la \
+ libqemuhotplugmock.la \
$(NULL)
if WITH_REMOTE
@@ -565,6 +566,11 @@ libqemucpumock_la_SOURCES = \
libqemucpumock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
libqemucpumock_la_LIBADD = $(MOCKLIBS_LIBS)
+libqemuhotplugmock_la_SOURCES = \
+ qemuhotplugmock.c
+libqemuhotplugmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
+libqemuhotplugmock_la_LIBADD = $(MOCKLIBS_LIBS)
+
qemuxml2argvtest_SOURCES = \
qemuxml2argvtest.c testutilsqemu.c testutilsqemu.h \
testutils.c testutils.h \
@@ -642,7 +648,11 @@ qemuhotplugtest_SOURCES = \
testutils.c testutils.h \
testutilsqemu.c testutilsqemu.h \
$(NULL)
-qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS)
+qemuhotplugtest_LDADD = \
+ libqemutestdriver.la \
+ libqemumonitortestutils.la \
+ $(qemu_LDADDS) \
+ $(NULL)
qemublocktest_SOURCES = \
qemublocktest.c \
@@ -716,6 +726,7 @@ EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c \
qemusecuritymock.c \
qemufirmwaretest.c \
qemuvhostusertest.c \
+ qemuhotplugmock.c \
$(QEMUMONITORTESTUTILS_SOURCES)
endif ! WITH_QEMU
diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c
new file mode 100644
index 0000000000..43a9d79051
--- /dev/null
+++ b/tests/qemuhotplugmock.c
@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2019 IBM Corporation
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ */
+
+#include <config.h>
+
+#include "qemu/qemu_hotplug.h"
+#include "conf/domain_conf.h"
+
+unsigned long long
+qemuDomainGetUnplugTimeout(virDomainObjPtr vm G_GNUC_UNUSED)
+{
+ /* Wait only 100ms for DEVICE_DELETED event. Give a greater
+ * timeout in case of PSeries guest to be consistent with the
+ * original logic. */
+ if (qemuDomainIsPSeries(vm->def))
+ return 200;
+ return 100;
+}
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index d3da08875a..5f2fc6a598 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -888,4 +888,5 @@ mymain(void)
VIR_TEST_MAIN_PRELOAD(mymain,
VIR_TEST_MOCK("virpci"),
- VIR_TEST_MOCK("virprocess"));
+ VIR_TEST_MOCK("virprocess"),
+ VIR_TEST_MOCK("qemuhotplug"));
--
2.21.0