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@xxxxxxxxxx>
> Reviewed-by: Xi Xu <xu.xi8@xxxxxxxxxx>
> ---
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
