diff --git a/src/qemu/THREADS.txt b/src/qemu/THREADS.txt index 7243161fe315ad0510162d115d4f15723ac13760..d17f3f4e0cce9439bf4f4232c889bac1641b1438 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,16 +69,23 @@ 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). Which type of job to grab depends + whether caller wishes to communicate only with agent socket, or only + with qemu monitor socket or both, respectively. + 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 - 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 - re-acquire it after the sleep/wait. Whenever an asynchronous job - wants to talk to the monitor, it needs to acquire nested job (a - special kind of normal job) to obtain exclusive access to the - monitor. + which intends to update state must acquire asynchronous, normal or + agent job . 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 re-acquire it after the + sleep/wait. Whenever an asynchronous job wants to talk to the + monitor, it needs to acquire nested job (a special kind of normal + job) to obtain exclusive access to the monitor. Since the virDomainObjPtr lock was dropped while waiting for the job condition, it is possible that the domain is no longer active @@ -132,6 +139,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 +278,65 @@ Design patterns virDomainObjEndAPI(&obj); + * Invoking an agent command on a virDomainObjPtr + + virDomainObjPtr obj; + qemuAgentPtr agent; + + obj = qemuDomObjFromDomain(dom); + + qemuDomainObjBeginAgentJob(obj, QEMU_AGENT_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); + + + * Invoking both monitor and agent commands on a virDomainObjPtr + + virDomainObjPtr obj; + qemuAgentPtr agent; + + obj = qemuDomObjFromDomain(dom); + + qemuDomainObjBeginJobWithAgent(obj, QEMU_JOB_TYPE, QEMU_AGENT_JOB_TYPE); + + if (!virDomainObjIsActive(dom)) + goto cleanup; + + ...do prep work... + + if (!qemuDomainAgentAvailable(obj, true)) + goto cleanup; + + agent = qemuDomainObjEnterAgent(obj); + qemuAgentXXXX(agent, ..); + qemuDomainObjExitAgent(obj, agent); + + ... + + qemuDomainObjEnterMonitor(obj); + qemuMonitorXXXX(priv->mon); + qemuDomainObjExitMonitor(obj); + + /* Alternatively, talk to the monitor first and then talk to the agent. */ + + ...do final work... + + qemuDomainObjEndJobWithAgent(obj); + virDomainObjEndAPI(&obj); + + * Running asynchronous job virDomainObjPtr obj; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 91f3c6d236c12442feec10996708faf672e9d3cf..d7c0598ceedffdab7ce614e7fc1348e36dee7aee 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -313,6 +313,19 @@ 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) { @@ -6356,6 +6369,17 @@ 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 == QEMU_JOB_NONE) && + (agentJob == QEMU_AGENT_JOB_NONE || + priv->job.agentActive == QEMU_AGENT_JOB_NONE)); +} + /* Give up waiting for mutex after 30 seconds */ #define QEMU_JOB_WAIT_TIME (1000ull * 30) @@ -6385,6 +6409,7 @@ static int ATTRIBUTE_NONNULL(1) qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, virDomainObjPtr obj, qemuDomainJob job, + qemuDomainAgentJob agentJob, qemuDomainAsyncJob asyncJob, bool nowait) { @@ -6398,16 +6423,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) { @@ -6434,7 +6458,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, goto error; } - while (priv->job.active) { + while (!qemuDomainObjCanSetJob(priv, job, agentJob)) { if (nowait) goto cleanup; @@ -6448,32 +6472,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(); - 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 (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.agentStarted = now; } if (qemuDomainTrackJob(job)) @@ -6554,12 +6594,50 @@ 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. However, if @job (or @agentJob) is + * QEMU_JOB_NONE (or QEMU_AGENT_JOB_NONE) only agent job is acquired (or + * monitor job). + * + * 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 +6647,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 +6677,7 @@ qemuDomainObjBeginNestedJob(virQEMUDriverPtr driver, return qemuDomainObjBeginJobInternal(driver, obj, QEMU_JOB_ASYNC_NESTED, + QEMU_AGENT_JOB_NONE, QEMU_ASYNC_JOB_NONE, false); } @@ -6621,6 +6701,7 @@ qemuDomainObjBeginJobNowait(virQEMUDriverPtr driver, qemuDomainJob job) { return qemuDomainObjBeginJobInternal(driver, obj, job, + QEMU_AGENT_JOB_NONE, QEMU_ASYNC_JOB_NONE, true); } @@ -6646,7 +6727,53 @@ qemuDomainObjEndJob(virQEMUDriverPtr driver, virDomainObjPtr obj) qemuDomainObjResetJob(priv); if (qemuDomainTrackJob(job)) qemuDomainObjSaveJob(driver, obj); - virCondSignal(&priv->job.cond); + /* We indeed need to wake up ALL threads waiting because + * grabbing a job requires checking more variables. */ + 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); + /* We indeed need to wake up ALL threads waiting because + * grabbing a job requires checking more variables. */ + 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); + /* We indeed need to wake up ALL threads waiting because + * grabbing a job requires checking more variables. */ + virCondBroadcast(&priv->job.cond); } void @@ -6682,8 +6809,9 @@ qemuDomainObjAbortAsyncJob(virDomainObjPtr obj) * obj must be locked before calling * * To be called immediately before any QEMU monitor API call - * Must have already either called qemuDomainObjBeginJob() and checked - * that the VM is still active; may not be used for nested async jobs. + * Must have already either called qemuDomainObjBeginJob() or + * qemuDomainObjBeginJobWithAgent() and checked that the VM is + * still active; may not be used for nested async jobs. * * To be followed with qemuDomainObjExitMonitor() once complete */ @@ -6800,8 +6928,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 */ diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 12573e5f86a6b7dacf3e943ae107ce904289abee..0a9af756b8235a40b93421432d6a9ec82c87d4b6 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -518,6 +518,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, @@ -535,6 +544,9 @@ int qemuDomainObjBeginJobNowait(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);