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 want to 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 v6:
- modify the description in qemu.conf
- move the multiplication to BeginJobInternal

changes in v5:
- adjust position of state lock in aug file
- fix state lock time got from conf file from milliseconds to
  seconds

changes in v4:
- fix syntax-check error

changes in v3:
- add user-friendly description and nb of state lock
- check validity of stateLockTimeout

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                 |  7 +++++++
 src/qemu/qemu_conf.c               | 14 ++++++++++++++
 src/qemu/qemu_conf.h               |  2 ++
 src/qemu/qemu_domain.c             |  7 +++----
 src/qemu/test_libvirtd_qemu.aug.in |  1 +
 6 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index ddc4bbf..a5601e1 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -100,6 +100,7 @@ module Libvirtd_qemu =
                  | str_entry "lock_manager"
 
    let rpc_entry = int_entry "max_queued"
+                 | int_entry "state_lock_timeout"
                  | int_entry "keepalive_interval"
                  | int_entry "keepalive_count"
 
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index cd57b3c..f5e34f1 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -667,6 +667,13 @@
 #
 #max_queued = 0
 
+
+# It is strongly recommended to not touch this setting
+#
+# Default is 30
+#
+#state_lock_timeout = 60
+
 ###################################################################
 # Keepalive protocol:
 # This allows qemu driver to detect broken connections to remote
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index a4f545e..5be37dc 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 (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;
+
     if (virConfGetValueInt(conf, "seccomp_sandbox", &cfg->seccompSandbox) < 0)
         goto cleanup;
 
@@ -1055,6 +1063,12 @@ virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg)
         return -1;
     }
 
+    if (cfg->stateLockTimeout <= 0) {
+        virReportError(VIR_ERR_CONF_SYNTAX, "%s",
+                       _("state_lock_timeout must be larger than zero"));
+        return -1;
+    }
+
     return 0;
 }
 
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..306772a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6652,9 +6652,6 @@ qemuDomainObjCanSetJob(qemuDomainObjPrivatePtr priv,
              priv->job.agentActive == QEMU_AGENT_JOB_NONE));
 }
 
-/* Give up waiting for mutex after 30 seconds */
-#define QEMU_JOB_WAIT_TIME (1000ull * 30)
-
 /**
  * qemuDomainObjBeginJobInternal:
  * @driver: qemu driver
@@ -6714,7 +6711,9 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
     }
 
     priv->jobs_queued++;
-    then = now + QEMU_JOB_WAIT_TIME;
+
+    cfg->stateLockTimeout *= 1000;
+    then = now + cfg->stateLockTimeout;
 
  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..8e072d0 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -82,6 +82,7 @@ module Test_libvirtd_qemu =
 { "relaxed_acs_check" = "1" }
 { "lock_manager" = "lockd" }
 { "max_queued" = "0" }
+{ "state_lock_timeout" = "60" }
 { "keepalive_interval" = "5" }
 { "keepalive_count" = "5" }
 { "seccomp_sandbox" = "1" }
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Yi Wang <wang.yi59@zte.com.cn> [2018-09-13, 10:39AM +0800]:
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 886e3fb..306772a 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -6652,9 +6652,6 @@ qemuDomainObjCanSetJob(qemuDomainObjPrivatePtr priv,
>               priv->job.agentActive == QEMU_AGENT_JOB_NONE));
>  }
>  
> -/* Give up waiting for mutex after 30 seconds */
> -#define QEMU_JOB_WAIT_TIME (1000ull * 30)
> -
>  /**
>   * qemuDomainObjBeginJobInternal:
>   * @driver: qemu driver
> @@ -6714,7 +6711,9 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
>      }
>  
>      priv->jobs_queued++;
> -    then = now + QEMU_JOB_WAIT_TIME;
> +
> +    cfg->stateLockTimeout *= 1000;

This doesn't look right. Each time qemuDomainObjBeginJobInternal is
called, the global config value gets multiplied.

> +    then = now + cfg->stateLockTimeout;

Why not just

    then = now + cfg->stateLockTimeout * 1000;

?

-- 
IBM Systems
Linux on Z & Virtualization Development
------------------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819
------------------------------------------------------------------------
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294 
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

> Yi Wang <wang.yi59@zte.com.cn> [2018-09-13, 10:39AM +0800]:
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index 886e3fb..306772a 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -6652,9 +6652,6 @@ qemuDomainObjCanSetJob(qemuDomainObjPrivatePtr priv,
> >               priv->job.agentActive == QEMU_AGENT_JOB_NONE));
> >  }
> >
> > -/* Give up waiting for mutex after 30 seconds */
> > -#define QEMU_JOB_WAIT_TIME (1000ull * 30)
> > -
> >  /**
> >   * qemuDomainObjBeginJobInternal:
> >   * @driver: qemu driver
> > @@ -6714,7 +6711,9 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
> >      }
> >
> >      priv->jobs_queued++;
> > -    then = now + QEMU_JOB_WAIT_TIME;
> > +
> > +    cfg->stateLockTimeout *= 1000;
>
> This doesn't look right. Each time qemuDomainObjBeginJobInternal is
> called, the global config value gets multiplied.
>
> > +    then = now + cfg->stateLockTimeout;
>
> Why not just
>
>     then = now + cfg->stateLockTimeout * 1000;
>
> ?
Quite right. :-(
Many thanks.


---
Best wishes
Yi Wang--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list