[PATCH] qemu: Avoid excessive calls to qemuDomainObjSaveJob()

[Jiri Denemark <jdenemar@xxxxxxxxxx>]

As reported by Daniel Berrangé, we have a huge performance regression
for virDomainGetInfo() due to the change which makes virDomainEndJob()
save the XML status file every time it is called. Previous to that
change, 2000 calls to virDomainGetInfo() took ~2.5 seconds. After that
change, 2000 calls to virDomainGetInfo() take 2 *minutes* 45 secs.

We made the change to be able to recover from libvirtd restart in the
middle of a job. However, only destroy and async jobs are taken care of.
Thus it makes more sense to only save domain state XML when these jobs
are started/stopped.
---
 src/qemu/qemu_domain.c  |   21 ++++++++++++++++++---
 src/qemu/qemu_domain.h  |    5 +++++
 src/qemu/qemu_process.c |    4 ++++
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 69d9e6e..da1f57f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -183,6 +183,12 @@ qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv)
     ignore_value(virCondDestroy(&priv->job.asyncCond));
 }
 
+static bool
+qemuDomainTrackJob(enum qemuDomainJob job)
+{
+    return (QEMU_DOMAIN_TRACK_JOBS & JOB_MASK(job)) != 0;
+}
+
 
 static void *qemuDomainObjPrivateAlloc(void)
 {
@@ -239,6 +245,7 @@ static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data)
 {
     qemuDomainObjPrivatePtr priv = data;
     const char *monitorpath;
+    enum qemuDomainJob job;
 
     /* priv->monitor_chr is set only for qemu */
     if (priv->monConfig) {
@@ -284,6 +291,10 @@ static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data)
     if (priv->lockState)
         virBufferAsprintf(buf, "  <lockstate>%s</lockstate>\n", priv->lockState);
 
+    job = priv->job.active;
+    if (!qemuDomainTrackJob(job))
+        priv->job.active = QEMU_JOB_NONE;
+
     if (priv->job.active || priv->job.asyncJob) {
         virBufferAsprintf(buf, "  <job type='%s' async='%s'",
                           qemuDomainJobTypeToString(priv->job.active),
@@ -295,6 +306,7 @@ static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data)
         }
         virBufferAddLit(buf, "/>\n");
     }
+    priv->job.active = job;
 
     if (priv->fakeReboot)
         virBufferAsprintf(buf, "  <fakereboot/>\n");
@@ -766,7 +778,8 @@ retry:
         virDomainObjLock(obj);
     }
 
-    qemuDomainObjSaveJob(driver, obj);
+    if (qemuDomainTrackJob(job))
+        qemuDomainObjSaveJob(driver, obj);
 
     return 0;
 
@@ -862,15 +875,17 @@ int qemuDomainObjBeginAsyncJobWithDriver(struct qemud_driver *driver,
 int qemuDomainObjEndJob(struct qemud_driver *driver, virDomainObjPtr obj)
 {
     qemuDomainObjPrivatePtr priv = obj->privateData;
+    enum qemuDomainJob job = priv->job.active;
 
     priv->jobs_queued--;
 
     VIR_DEBUG("Stopping job: %s (async=%s)",
-              qemuDomainJobTypeToString(priv->job.active),
+              qemuDomainJobTypeToString(job),
               qemuDomainAsyncJobTypeToString(priv->job.asyncJob));
 
     qemuDomainObjResetJob(priv);
-    qemuDomainObjSaveJob(driver, obj);
+    if (qemuDomainTrackJob(job))
+        qemuDomainObjSaveJob(driver, obj);
     virCondSignal(&priv->job.cond);
 
     return virDomainObjUnref(obj);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index adccfed..b3eecd3 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -53,6 +53,11 @@
      JOB_MASK(QEMU_JOB_DESTROY) |       \
      JOB_MASK(QEMU_JOB_ABORT))
 
+/* Jobs which have to be tracked in domain state XML. */
+# define QEMU_DOMAIN_TRACK_JOBS         \
+    (JOB_MASK(QEMU_JOB_DESTROY) |       \
+     JOB_MASK(QEMU_JOB_ASYNC))
+
 /* Only 1 job is allowed at any time
  * A job includes *all* monitor commands, even those just querying
  * information, not merely actions */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 481b4f3..96f39e8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2963,6 +2963,10 @@ qemuProcessRecoverJob(struct qemud_driver *driver,
     if (!virDomainObjIsActive(vm))
         return -1;
 
+    /* In case any special handling is added for job type that has been ignored
+     * before, QEMU_DOMAIN_TRACK_JOBS (from qemu_domain.h) needs to be updated
+     * for the job to be properly tracked in domain state XML.
+     */
     switch (job->active) {
     case QEMU_JOB_QUERY:
         /* harmless */
-- 
1.7.8.5

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