On 08/27/2018 12:57 PM, Yi Wang wrote:
When doing some job holding state lock for a long time,
we may come across error:
"Timed out during operation: cannot acquire state change lock"
Well, sometimes it's not a problem and users wanner continue
to wait, and this patch allow users decide how long time they
can wait the state lock.
Signed-off-by: Yi Wang <wang.yi59(a)zte.com.cn>
Reviewed-by: Xi Xu <xu.xi8(a)zte.com.cn>
---
changes in v2:
- change default value to 30 in qemu.conf
- set the default value in virQEMUDriverConfigNew()
---
src/qemu/libvirtd_qemu.aug | 1 +
src/qemu/qemu.conf | 6 ++++++
src/qemu/qemu_conf.c | 8 ++++++++
src/qemu/qemu_conf.h | 2 ++
src/qemu/qemu_domain.c | 5 +----
src/qemu/test_libvirtd_qemu.aug.in | 1 +
6 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index ddc4bbf..f7287ae 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -93,6 +93,7 @@ module Libvirtd_qemu =
| limits_entry "max_core"
| bool_entry "dump_guest_core"
| str_entry "stdio_handler"
+ | int_entry "state_lock_timeout"
let device_entry = bool_entry "mac_filter"
| bool_entry "relaxed_acs_check"
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index cd57b3c..4284786 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -813,3 +813,9 @@
#
#swtpm_user = "tss"
#swtpm_group = "tss"
+
+# The timeout (in seconds) waiting for acquiring state lock.
This is rather sparse description. I know that state change lock is,
because I'm a libvirt devel. However, I don't expect our users to know
that. How about adding the following description:
When two or more threads want to work with the same domain they use a
job lock to mutually exclude each other. However, waiting for the lock
is limited up to state_lock_timeout seconds.
Also, could you move this close to max_queued variable since they both
refer to the same area?
+#
+# Default is 30
+#
+#state_lock_timeout = 60
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index a4f545e..31e0013 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -129,6 +129,9 @@ void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def)
#endif
+/* Give up waiting for mutex after 30 seconds */
+#define QEMU_JOB_WAIT_TIME (1000ull * 30)
+
virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
{
virQEMUDriverConfigPtr cfg;
@@ -346,6 +349,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
cfg->glusterDebugLevel = 4;
cfg->stdioLogD = true;
+ cfg->stateLockTimeout = QEMU_JOB_WAIT_TIME;
+
if (!(cfg->namespaces = virBitmapNew(QEMU_DOMAIN_NS_LAST)))
goto error;
@@ -863,6 +868,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
if (virConfGetValueUInt(conf, "keepalive_count",
&cfg->keepAliveCount) < 0)
goto cleanup;
+ if (virConfGetValueInt(conf, "state_lock_timeout",
&cfg->stateLockTimeout) < 0)
+ goto cleanup;
+
Almost, you need to check if cfg->stateLockTimeout is not zero. Such
code could go into virQEMUDriverConfigValidate().
Otherwise looking good.
Michal