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@zte.com.cn> Reviewed-by: Xi Xu <xu.xi8@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