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