8:45 a.m.
The point is to break QEMU_JOB_* into smaller pieces which
enables us to achieve higher throughput. For instance, if there
are two threads, one is trying to query something on qemu
monitor while the other is trying to query something on agent
monitor these two threads would serialize. There is not much
reason for that.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/THREADS.txt | 62 +++++++++++++++++--
src/qemu/qemu_domain.c | 163 +++++++++++++++++++++++++++++++++++++++----------
src/qemu/qemu_domain.h | 12 ++++
3 files changed, 200 insertions(+), 37 deletions(-)
diff --git a/src/qemu/THREADS.txt b/src/qemu/THREADS.txt
index 7243161fe3..6219f46a81 100644
--- a/src/qemu/THREADS.txt
+++ b/src/qemu/THREADS.txt
@@ -51,8 +51,8 @@ There are a number of locks on various objects
Since virDomainObjPtr lock must not be held during sleeps, the job
conditions provide additional protection for code making updates.
- QEMU driver uses two kinds of job conditions: asynchronous and
- normal.
+ QEMU driver uses three kinds of job conditions: asynchronous, agent
+ and normal.
Asynchronous job condition is used for long running jobs (such as
migration) that consist of several monitor commands and it is
@@ -69,9 +69,15 @@ There are a number of locks on various objects
it needs to wait until the asynchronous job ends and try to acquire
the job again.
+ Agent job condition is then used when thread wishes to talk to qemu
+ agent monitor. It is possible to acquire just agent job
+ (qemuDomainObjBeginAgentJob), or only normal job
+ (qemuDomainObjBeginJob) or both at the same time
+ (qemuDomainObjBeginJobWithAgent).
+
Immediately after acquiring the virDomainObjPtr lock, any method
- which intends to update state must acquire either asynchronous or
- normal job condition. The virDomainObjPtr lock is released while
+ which intends to update state must acquire asynchronous, normal or
+ agent job condition. The virDomainObjPtr lock is released while
blocking on these condition variables. Once the job condition is
acquired, a method can safely release the virDomainObjPtr lock
whenever it hits a piece of code which may sleep/wait, and
@@ -132,6 +138,30 @@ To acquire the normal job condition
+To acquire the agent job condition
+
+ qemuDomainObjBeginAgentJob()
+ - Waits until there is no other agent job set
+ - Sets job.agentActive tp the job type
+
+ qemuDomainObjEndAgentJob()
+ - Sets job.agentActive to 0
+ - Signals on job.cond condition
+
+
+
+To acquire both normal and agent job condition
+
+ qemuDomainObjBeginJobWithAgent()
+ - Waits until there is no normal and no agent job set
+ - Sets both job.active and job.agentActive with required job types
+
+ qemuDomainObjEndJobWithAgent()
+ - Sets both job.active and job.agentActive to 0
+ - Signals on job.cond condition
+
+
+
To acquire the asynchronous job condition
qemuDomainObjBeginAsyncJob()
@@ -247,6 +277,30 @@ Design patterns
virDomainObjEndAPI(&obj);
+ * Invoking an agent command on a virDomainObjPtr
+
+ virDomainObjPtr obj;
+ qemuAgentPtr agent;
+
+ obj = qemuDomObjFromDomain(dom);
+
+ qemuDomainObjBeginAgentJob(obj, QEMU_JOB_TYPE);
+
+ ...do prep work...
+
+ if (!qemuDomainAgentAvailable(obj, true))
+ goto cleanup;
+
+ agent = qemuDomainObjEnterAgent(obj);
+ qemuAgentXXXX(agent, ..);
+ qemuDomainObjExitAgent(obj, agent);
+
+ ...do final work...
+
+ qemuDomainObjEndAgentJob(obj);
+ virDomainObjEndAPI(&obj);
+
+
* Running asynchronous job
virDomainObjPtr obj;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b8e34c1c2c..09404f6569 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -319,6 +319,17 @@ qemuDomainObjResetJob(qemuDomainObjPrivatePtr priv)
job->started = 0;
}
+static void
+qemuDomainObjResetAgentJob(qemuDomainObjPrivatePtr priv)
+{
+ qemuDomainJobObjPtr job = &priv->job;
+
+ job->agentActive = QEMU_AGENT_JOB_NONE;
+ job->agentOwner = 0;
+ job->agentOwnerAPI = NULL;
+ job->agentStarted = 0;
+}
+
static void
qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
{
@@ -6361,6 +6372,15 @@ qemuDomainJobAllowed(qemuDomainObjPrivatePtr priv, qemuDomainJob
job)
return !priv->job.active && qemuDomainNestedJobAllowed(priv, job);
}
+static bool
+qemuDomainObjCanSetJob(qemuDomainObjPrivatePtr priv,
+ qemuDomainJob job,
+ qemuDomainAgentJob agentJob)
+{
+ return (job == QEMU_JOB_NONE || !priv->job.active) &&
+ (agentJob == QEMU_AGENT_JOB_NONE || !priv->job.agentActive);
+}
+
/* Give up waiting for mutex after 30 seconds */
#define QEMU_JOB_WAIT_TIME (1000ull * 30)
@@ -6371,6 +6391,7 @@ static int ATTRIBUTE_NONNULL(1)
qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
virDomainObjPtr obj,
qemuDomainJob job,
+ qemuDomainAgentJob agentJob,
qemuDomainAsyncJob asyncJob)
{
qemuDomainObjPrivatePtr priv = obj->privateData;
@@ -6383,16 +6404,15 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
int ret = -1;
unsigned long long duration = 0;
unsigned long long asyncDuration = 0;
- const char *jobStr;
- if (async)
- jobStr = qemuDomainAsyncJobTypeToString(asyncJob);
- else
- jobStr = qemuDomainJobTypeToString(job);
-
- VIR_DEBUG("Starting %s: %s (vm=%p name=%s, current job=%s async=%s)",
- async ? "async job" : "job", jobStr, obj,
obj->def->name,
+ VIR_DEBUG("Starting job: job=%s agentJob=%s asyncJob=%s "
+ "(vm=%p name=%s, current job=%s agentJob=%s async=%s)",
+ qemuDomainJobTypeToString(job),
+ qemuDomainAgentJobTypeToString(agentJob),
+ qemuDomainAsyncJobTypeToString(asyncJob),
+ obj, obj->def->name,
qemuDomainJobTypeToString(priv->job.active),
+ qemuDomainAgentJobTypeToString(priv->job.agentActive),
qemuDomainAsyncJobTypeToString(priv->job.asyncJob));
if (virTimeMillisNow(&now) < 0) {
@@ -6416,7 +6436,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
goto error;
}
- while (priv->job.active) {
+ while (!qemuDomainObjCanSetJob(priv, job, agentJob)) {
VIR_DEBUG("Waiting for job (vm=%p name=%s)", obj,
obj->def->name);
if (virCondWaitUntil(&priv->job.cond, &obj->parent.lock, then) <
0)
goto error;
@@ -6427,32 +6447,48 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
if (!nested && !qemuDomainNestedJobAllowed(priv, job))
goto retry;
- qemuDomainObjResetJob(priv);
-
ignore_value(virTimeMillisNow(&now));
- if (job != QEMU_JOB_ASYNC) {
- VIR_DEBUG("Started job: %s (async=%s vm=%p name=%s)",
- qemuDomainJobTypeToString(job),
- qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
- obj, obj->def->name);
- priv->job.active = job;
- priv->job.owner = virThreadSelfID();
- priv->job.ownerAPI = virThreadJobGet();
+ if (job) {
+ qemuDomainObjResetJob(priv);
+
+ if (job != QEMU_JOB_ASYNC) {
+ VIR_DEBUG("Started job: %s (async=%s vm=%p name=%s)",
+ qemuDomainJobTypeToString(job),
+ qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
+ obj, obj->def->name);
+ priv->job.active = job;
+ priv->job.owner = virThreadSelfID();
+ priv->job.ownerAPI = virThreadJobGet();
+ priv->job.started = now;
+ } else {
+ VIR_DEBUG("Started async job: %s (vm=%p name=%s)",
+ qemuDomainAsyncJobTypeToString(asyncJob),
+ obj, obj->def->name);
+ qemuDomainObjResetAsyncJob(priv);
+ if (VIR_ALLOC(priv->job.current) < 0)
+ goto cleanup;
+ priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
+ priv->job.asyncJob = asyncJob;
+ priv->job.asyncOwner = virThreadSelfID();
+ priv->job.asyncOwnerAPI = virThreadJobGet();
+ priv->job.asyncStarted = now;
+ priv->job.current->started = now;
+ }
+ }
+
+ if (agentJob) {
+ qemuDomainObjResetAgentJob(priv);
+
+ VIR_DEBUG("Started agent job: %s (vm=%p name=%s job=%s async=%s)",
+ qemuDomainAgentJobTypeToString(agentJob),
+ obj, obj->def->name,
+ qemuDomainJobTypeToString(priv->job.active),
+ qemuDomainAsyncJobTypeToString(priv->job.asyncJob));
+ priv->job.agentActive = agentJob;
+ priv->job.agentOwner = virThreadSelfID();
+ priv->job.agentOwnerAPI = virThreadJobGet();
priv->job.started = now;
- } else {
- VIR_DEBUG("Started async job: %s (vm=%p name=%s)",
- qemuDomainAsyncJobTypeToString(asyncJob),
- obj, obj->def->name);
- qemuDomainObjResetAsyncJob(priv);
- if (VIR_ALLOC(priv->job.current) < 0)
- goto cleanup;
- priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
- priv->job.asyncJob = asyncJob;
- priv->job.asyncOwner = virThreadSelfID();
- priv->job.asyncOwnerAPI = virThreadJobGet();
- priv->job.asyncStarted = now;
- priv->job.current->started = now;
}
if (qemuDomainTrackJob(job))
@@ -6534,12 +6570,31 @@ int qemuDomainObjBeginJob(virQEMUDriverPtr driver,
qemuDomainJob job)
{
if (qemuDomainObjBeginJobInternal(driver, obj, job,
+ QEMU_AGENT_JOB_NONE,
QEMU_ASYNC_JOB_NONE) < 0)
return -1;
else
return 0;
}
+int qemuDomainObjBeginAgentJob(virQEMUDriverPtr driver,
+ virDomainObjPtr obj,
+ qemuDomainAgentJob agentJob)
+{
+ return qemuDomainObjBeginJobInternal(driver, obj, QEMU_JOB_NONE,
+ agentJob,
+ QEMU_ASYNC_JOB_NONE);
+}
+
+int qemuDomainObjBeginJobWithAgent(virQEMUDriverPtr driver,
+ virDomainObjPtr obj,
+ qemuDomainJob job,
+ qemuDomainAgentJob agentJob)
+{
+ return qemuDomainObjBeginJobInternal(driver, obj, job,
+ agentJob, QEMU_ASYNC_JOB_NONE);
+}
+
int qemuDomainObjBeginAsyncJob(virQEMUDriverPtr driver,
virDomainObjPtr obj,
qemuDomainAsyncJob asyncJob,
@@ -6549,6 +6604,7 @@ int qemuDomainObjBeginAsyncJob(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv;
if (qemuDomainObjBeginJobInternal(driver, obj, QEMU_JOB_ASYNC,
+ QEMU_AGENT_JOB_NONE,
asyncJob) < 0)
return -1;
@@ -6578,6 +6634,7 @@ qemuDomainObjBeginNestedJob(virQEMUDriverPtr driver,
return qemuDomainObjBeginJobInternal(driver, obj,
QEMU_JOB_ASYNC_NESTED,
+ QEMU_AGENT_JOB_NONE,
QEMU_ASYNC_JOB_NONE);
}
@@ -6604,7 +6661,47 @@ qemuDomainObjEndJob(virQEMUDriverPtr driver, virDomainObjPtr obj)
qemuDomainObjResetJob(priv);
if (qemuDomainTrackJob(job))
qemuDomainObjSaveJob(driver, obj);
- virCondSignal(&priv->job.cond);
+ virCondBroadcast(&priv->job.cond);
+}
+
+void
+qemuDomainObjEndAgentJob(virDomainObjPtr obj)
+{
+ qemuDomainObjPrivatePtr priv = obj->privateData;
+ qemuDomainAgentJob agentJob = priv->job.agentActive;
+
+ priv->jobs_queued--;
+
+ VIR_DEBUG("Stopping agent job: %s (async=%s vm=%p name=%s)",
+ qemuDomainAgentJobTypeToString(agentJob),
+ qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
+ obj, obj->def->name);
+
+ qemuDomainObjResetAgentJob(priv);
+ virCondBroadcast(&priv->job.cond);
+}
+
+void
+qemuDomainObjEndJobWithAgent(virQEMUDriverPtr driver,
+ virDomainObjPtr obj)
+{
+ qemuDomainObjPrivatePtr priv = obj->privateData;
+ qemuDomainJob job = priv->job.active;
+ qemuDomainAgentJob agentJob = priv->job.agentActive;
+
+ priv->jobs_queued--;
+
+ VIR_DEBUG("Stopping both jobs: %s %s (async=%s vm=%p name=%s)",
+ qemuDomainJobTypeToString(job),
+ qemuDomainAgentJobTypeToString(agentJob),
+ qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
+ obj, obj->def->name);
+
+ qemuDomainObjResetJob(priv);
+ qemuDomainObjResetAgentJob(priv);
+ if (qemuDomainTrackJob(job))
+ qemuDomainObjSaveJob(driver, obj);
+ virCondBroadcast(&priv->job.cond);
}
void
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 709b42e6fd..6ada26ca99 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -517,6 +517,15 @@ int qemuDomainObjBeginJob(virQEMUDriverPtr driver,
virDomainObjPtr obj,
qemuDomainJob job)
ATTRIBUTE_RETURN_CHECK;
+int qemuDomainObjBeginAgentJob(virQEMUDriverPtr driver,
+ virDomainObjPtr obj,
+ qemuDomainAgentJob agentJob)
+ ATTRIBUTE_RETURN_CHECK;
+int qemuDomainObjBeginJobWithAgent(virQEMUDriverPtr driver,
+ virDomainObjPtr obj,
+ qemuDomainJob job,
+ qemuDomainAgentJob agentJob)
+ ATTRIBUTE_RETURN_CHECK;
int qemuDomainObjBeginAsyncJob(virQEMUDriverPtr driver,
virDomainObjPtr obj,
qemuDomainAsyncJob asyncJob,
@@ -530,6 +539,9 @@ int qemuDomainObjBeginNestedJob(virQEMUDriverPtr driver,
void qemuDomainObjEndJob(virQEMUDriverPtr driver,
virDomainObjPtr obj);
+void qemuDomainObjEndAgentJob(virDomainObjPtr obj);
+void qemuDomainObjEndJobWithAgent(virQEMUDriverPtr driver,
+ virDomainObjPtr obj);
void qemuDomainObjEndAsyncJob(virQEMUDriverPtr driver,
virDomainObjPtr obj);
void qemuDomainObjAbortAsyncJob(virDomainObjPtr obj);
--
2.16.4