Re: [libvirt] [v2 RESEND 2/3] qemu: Introduce APIs for manipulating qemuDomainAgentJob

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&gt; --- src/qemu/THREADS.txt | 112 ++++++++++++++++++++++++++--- src/qemu/qemu_domain.c | 187 +++++++++++++++++++++++++++++++++++++++---------- src/qemu/qemu_domain.h | 12 ++++ 3 files changed, 264 insertions(+), 47 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 91f3c6d236..30a06a1575 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c

@@ -6356,6 +6369,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)

@@ -6434,10 +6456,9 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, goto error; } - while (priv->job.active) { + while (!qemuDomainObjCanSetJob(priv, job, agentJob)) {

if (nowait) goto cleanup; -

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; @@ -6448,32 +6469,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)) @@ -6554,12 +6591,46 @@ int qemuDomainObjBeginJob(virQEMUDriverPtr driver, qemuDomainJob job) { if (qemuDomainObjBeginJobInternal(driver, obj, job, + QEMU_AGENT_JOB_NONE, QEMU_ASYNC_JOB_NONE, false) < 0) return -1; else return 0; } +/** + * qemuDomainObjBeginAgentJob: + * + * Grabs agent type of job. Use if caller talks to guest agent only. + * + * To end job call qemuDomainObjEndAgentJob. + */ +int qemuDomainObjBeginAgentJob(virQEMUDriverPtr driver, + virDomainObjPtr obj, + qemuDomainAgentJob agentJob)

+{ + return qemuDomainObjBeginJobInternal(driver, obj, QEMU_JOB_NONE, + agentJob, + QEMU_ASYNC_JOB_NONE, false); +} + +/** + * qemuDomainObjBeginJobWithAgent: + * + * Grabs both monitor and agent types of job. Use if caller talks to both + * monitor and guest agent. + * + * To end job call qemuDomainObjEndJobWithAgent. + */ +int qemuDomainObjBeginJobWithAgent(virQEMUDriverPtr driver, + virDomainObjPtr obj, + qemuDomainJob job, + qemuDomainAgentJob agentJob)

+{ + return qemuDomainObjBeginJobInternal(driver, obj, job, + agentJob, QEMU_ASYNC_JOB_NONE, false);

+} + int qemuDomainObjBeginAsyncJob(virQEMUDriverPtr driver, virDomainObjPtr obj, qemuDomainAsyncJob asyncJob, @@ -6569,6 +6640,7 @@ int qemuDomainObjBeginAsyncJob(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv; if (qemuDomainObjBeginJobInternal(driver, obj, QEMU_JOB_ASYNC, + QEMU_AGENT_JOB_NONE, asyncJob, false) < 0) return -1; @@ -6598,6 +6670,7 @@ qemuDomainObjBeginNestedJob(virQEMUDriverPtr driver, return qemuDomainObjBeginJobInternal(driver, obj, QEMU_JOB_ASYNC_NESTED, + QEMU_AGENT_JOB_NONE, QEMU_ASYNC_JOB_NONE, false); } @@ -6621,6 +6694,7 @@ qemuDomainObjBeginJobNowait(virQEMUDriverPtr driver, qemuDomainJob job) { return qemuDomainObjBeginJobInternal(driver, obj, job, + QEMU_AGENT_JOB_NONE, QEMU_ASYNC_JOB_NONE, true); } @@ -6646,7 +6720,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 @@ -6800,8 +6914,9 @@ qemuDomainObjEnterMonitorAsync(virQEMUDriverPtr driver, * obj must be locked before calling * * To be called immediately before any QEMU agent API call. - * Must have already called qemuDomainObjBeginJob() and checked - * that the VM is still active. + * Must have already called qemuDomainObjBeginAgentJob() or + * qemuDomainObjBeginJobWithAgent() and checked that the VM is + * still active.

* * To be followed with qemuDomainObjExitAgent() once complete */