On 08/27/2018 08:04 AM, 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@xxxxxxxxxx>
> Reviewed-by: Xi Xu <xu.xi8@xxxxxxxxxx>
> ---
> src/qemu/libvirtd_qemu.aug | 1 +
> src/qemu/qemu.conf | 6 ++++++
> src/qemu/qemu_conf.c | 3 +++
> src/qemu/qemu_conf.h | 2 ++
> src/qemu/qemu_domain.c | 6 +++++-
> src/qemu/test_libvirtd_qemu.aug.in | 1 +
> 6 files changed, 18 insertions(+), 1 deletion(-)
>
> 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..e88db3f 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -813,3 +813,9 @@
> #
> #swtpm_user = "tss"
> #swtpm_group = "tss"
> +
> +# Override the timeout (in seconds) for acquiring state lock.
> +#
> +# Default is 0
> +#
> +#state_lock_timeout = 60
Firstly, the default value should be set for the variable. So if the
default is really 0 (which it is not) then this line should read:
#state_lock_timeout = 0
Secondly, the default is not zero, because if user sets it to zero your
code uses the hardcoded timeout of 30 seconds. Also, we should probably
reserve 0 for some special future use (no timeouts perhaps?)
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index a4f545e..f452714 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -863,6 +863,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;
> +
> if (virConfGetValueInt(conf, "seccomp_sandbox", &cfg->seccompSandbox) < 0)
> goto cleanup;
>
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index a8d84ef..97cf2e1 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -190,6 +190,8 @@ struct _virQEMUDriverConfig {
> int keepAliveInterval;
> unsigned int keepAliveCount;
>
> + int stateLockTimeout;
> +
> int seccompSandbox;
>
> char *migrateHost;
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 886e3fb..d375a73 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -6714,7 +6714,11 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
> }
>
> priv->jobs_queued++;
> - then = now + QEMU_JOB_WAIT_TIME;
> + if (cfg->stateLockTimeout > 0)
> + then = now + cfg->stateLockTimeout;
> + else
> + then = now + QEMU_JOB_WAIT_TIME;
You can set the default in virQEMUDriverConfigNew() so that it's always
set and simplify this line then.
> +
>
> retry:
> if ((!async && job != QEMU_JOB_DESTROY) &&
> diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
> index f1e8806..dc5de96 100644
> --- a/src/qemu/test_libvirtd_qemu.aug.in
> +++ b/src/qemu/test_libvirtd_qemu.aug.in
> @@ -105,3 +105,4 @@ module Test_libvirtd_qemu =
> { "pr_helper" = "/usr/bin/qemu-pr-helper" }
> { "swtpm_user" = "tss" }
> { "swtpm_group" = "tss" }
> +{ "state_lock_timeout" = "60" }
>
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
