[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v1 1/3] adding unplug_timeout QEMU conf

On Sun, Aug 18, 2019 at 06:45:29PM -0300, Daniel Henrique Barboza wrote:
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 actual operation happened in the guest, causing confusion
for the user.

The commit that introduced this error message:
   commit e3229f6e4461cd1721dc68a32e16ab1718ae716e
       qemu: hotplug: Add support for VCPU unplug

specifically says that we treat this differently than regular device

   As the new code is using device_del all the implications of using it
   are present. Contrary to the device deletion code, the vcpu deletion
   code fails if the unplug request is not executed in time.

Technically, we already did execute the unplug request so we lie to the
user saying "operation failed".

Maybe we can revisit the decision? [cc-ing pkrempa who added this]

This patch adds a new qemu.conf parameter called 'unplug_timeout'
to handle these cases. If left unset, the current default
timeout is used. To avoid user 'experimentation' with small
timeouts, the current timeout is also the minimal value

The reason for this timeout is that we originally promised something
that we cannot deliver - a synchronous device detach API, while the
operation itself is asynchronous. I'm not a fan of exposing it and
making it configurable.

Signed-off-by: Daniel Henrique Barboza <danielhb413 gmail com>
src/qemu/libvirtd_qemu.aug         |  3 +++
src/qemu/qemu.conf                 |  4 ++++
src/qemu/qemu_conf.c               | 26 ++++++++++++++++++++++++++
src/qemu/qemu_conf.h               |  5 +++++
src/qemu/qemu_driver.c             |  2 ++
src/qemu/test_libvirtd_qemu.aug.in |  1 +
6 files changed, 41 insertions(+)


diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 0cbddd7a9c..29824e4e35 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -214,6 +214,8 @@ struct _virQEMUDriverConfig {
    gid_t swtpm_group;

    char **capabilityfilters;
+    unsigned int unplugTimeout;

/* Main driver state */
@@ -294,6 +296,9 @@ struct _virQEMUDriver {

    /* Immutable pointer, self-locking APIs */
    virHashAtomicPtr migrationErrors;
+    /* Immutable value */
+    unsigned int unplugTimeout;

Why store this value twice?


Attachment: signature.asc
Description: PGP signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]