On Mon, Aug 17, 2020 at 10:37:18AM +0530, Prathamesh Chavan wrote:
Reference to `maxQueuedJobs` required us to access config of the qemu-driver. And creating its copy in the `qemuDomainJob` helped us access the variable without referencing the driver's config.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com> --- src/qemu/qemu_domain.c | 5 ++++- src/qemu/qemu_domainjob.c | 13 +++++++------ src/qemu/qemu_domainjob.h | 4 +++- 3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1ae44ae39f..677fa7ea91 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2085,11 +2085,14 @@ static void * qemuDomainObjPrivateAlloc(void *opaque) { qemuDomainObjPrivatePtr priv; + virQEMUDriverPtr driver = opaque; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
if (VIR_ALLOC(priv) < 0) return NULL;
- if (qemuDomainObjInitJob(&priv->job, &qemuPrivateJobCallbacks) < 0) { + if (qemuDomainObjInitJob(&priv->job, &qemuPrivateJobCallbacks, + cfg->maxQueuedJobs) < 0) { virReportSystemError(errno, "%s", _("Unable to init qemu driver mutexes")); goto error; diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index 7cd1aabd9e..eebc144747 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -117,10 +117,12 @@ qemuDomainAsyncJobPhaseFromString(qemuDomainAsyncJob job,
int qemuDomainObjInitJob(qemuDomainJobObjPtr job, - qemuDomainObjPrivateJobCallbacksPtr cb) + qemuDomainObjPrivateJobCallbacksPtr cb, + unsigned int maxQueuedJobs) { memset(job, 0, sizeof(*job)); job->cb = cb; + job->maxQueuedJobs = maxQueuedJobs;
if (!(job->privateData = job->cb->allocJobPrivate())) return -1; @@ -344,7 +346,6 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, unsigned long long then; bool nested = job == QEMU_JOB_ASYNC_NESTED; bool async = job == QEMU_JOB_ASYNC; - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); const char *blocker = NULL; const char *agentBlocker = NULL; int ret = -1; @@ -370,8 +371,8 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
retry: if ((!async && job != QEMU_JOB_DESTROY) && - cfg->maxQueuedJobs && - priv->job.jobs_queued > cfg->maxQueuedJobs) { + priv->job.maxQueuedJobs && + priv->job.jobs_queued > priv->job.maxQueuedJobs) { goto error; }
@@ -501,8 +502,8 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, _("cannot acquire state change lock")); } ret = -2; - } else if (cfg->maxQueuedJobs && - priv->job.jobs_queued > cfg->maxQueuedJobs) { + } else if (priv->job.maxQueuedJobs && + priv->job.jobs_queued > priv->job.maxQueuedJobs) { if (blocker && agentBlocker) { virReportError(VIR_ERR_OPERATION_FAILED, _("cannot acquire state change " diff --git a/src/qemu/qemu_domainjob.h b/src/qemu/qemu_domainjob.h index 0696b79fe3..11e7f2f432 100644 --- a/src/qemu/qemu_domainjob.h +++ b/src/qemu/qemu_domainjob.h @@ -153,6 +153,7 @@ struct _qemuDomainJobObj { unsigned long apiFlags; /* flags passed to the API which started the async job */
int jobs_queued; + unsigned int maxQueuedJobs;
void *privateData; /* job specific collection of data */ qemuDomainObjPrivateJobCallbacksPtr cb;
The comment below applies to both 2/6 and 3/6. Looking at this again, I don't think we want to move any of the jobs_queued or maxQueuedJobs attributes to the job structure, I'm sorry I didn't spot this right away in the previous revision of the series, I take responsibility for it. The job structure represents a single job, so from the design POV, referencing both jobs_queued (which is really tied to a domain) and maxQueuedJobs (which is a global driver setting) from within the job structure simply doesn't feel right. Instead, I think qemuDomainObjBeginJobInternal will simply have to be split in parts specific to QEMU that have access to the driver and hypervisor-agnostic bits that can be moved to src/hypervisor. Erik