[libvirt] [PATCH v4] qemu: Introduce state_lock_timeout toqemu.conf

[wang.yi59 at zte.com.cn]

Hi Jano,
thanks for your reply.

> On Tue, Aug 28, 2018 at 04:40:16PM +0800, 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
>
> s/wanner/want to/

Ok, I will pay attention to this later.

> >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"
> >
>
> here you add the option at the end of the 'process_entry' group
>
> > 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..8920a1a 100644
> >--- a/src/qemu/qemu.conf
> >+++ b/src/qemu/qemu.conf
> >@@ -667,6 +667,16 @@
> > #
> > #max_queued = 0
> >
> >+
> >+# 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.
> >+# NB, strong recommendation to set the timeout longer than 30 seconds.
> >+#
> >+# Default is 30
> >+#
> >+#state_lock_timeout = 60
>
> But here in qemu.conf, you add it between the rpc entries.
>
> It seems we did not follow the structure with 'stdio_handler',
> but adding it either right after 'dump_guest_core' or at the end of file
> would be better than squeezing it between rpc entries.

As Michal suggested in:
https://www.redhat.com/archives/libvir-list/2018-August/msg01693.html
max_queued and state_lock_timeout both refer to the same area, so I put it
here :)

> >@@ -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;
> >+
>
> But the parsed value is passed directly here, so '60' in the config file
> would result in 60 ms.

Oho, I will fix this.

> >
> > 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" }
>
> This needs to be ordered properly - install the 'augeas' tool to see
> where 'make check' fails.

My fault, sorry for the mistake, and next patch will fix this.
Thanks again, Jano.


---
Best wishes
Yi Wang