On 30.05.2019 12:34, Michal Privoznik wrote:
> Soon we will be acquiring job for every virDomainObjAssignDef()
> call. This, however, means that in some cases vm->def might not
> be set just yet. Fortunately, the only thing from domain
> definition used in qemuDomainObjBeginJob()/qemuDomainObjEndJob()
> (and friends) is the domain name and even that is used only for
> debug printings. Therefore, we can safely replace obj->def->name
> with some NULLSTR() magic.
>
> There is one other place where vm->def might be used -
> qemuDomainObjSaveJob() - but this function does something only
> for active domains which already have vm->def pointer set.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/qemu/qemu_domain.c | 27 ++++++++++++++++-----------
> 1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index e50e84a3b2..c61d39b12b 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -7175,6 +7175,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> const char *blocker = NULL;
> const char *agentBlocker = NULL;
> + const char *domName = NULLSTR(obj->def ? obj->def->name : NULL);
> int ret = -1;
> unsigned long long duration = 0;
> unsigned long long agentDuration = 0;
> @@ -7185,7 +7186,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
> qemuDomainJobTypeToString(job),
> qemuDomainAgentJobTypeToString(agentJob),
> qemuDomainAsyncJobTypeToString(asyncJob),
> - obj, obj->def->name,
> + obj, domName,
> qemuDomainJobTypeToString(priv->job.active),
> qemuDomainAgentJobTypeToString(priv->job.agentActive),
> qemuDomainAsyncJobTypeToString(priv->job.asyncJob));
> @@ -7209,7 +7210,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
> if (nowait)
> goto cleanup;
>
> - VIR_DEBUG("Waiting for async job (vm=%p name=%s)", obj, obj->def->name);
> + VIR_DEBUG("Waiting for async job (vm=%p name=%s)", obj, domName);
> if (virCondWaitUntil(&priv->job.asyncCond, &obj->parent.lock, then) < 0)
> goto error;
> }
> @@ -7218,7 +7219,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
> if (nowait)
> goto cleanup;
>
> - VIR_DEBUG("Waiting for job (vm=%p name=%s)", obj, obj->def->name);
> + VIR_DEBUG("Waiting for job (vm=%p name=%s)", obj, domName);
> if (virCondWaitUntil(&priv->job.cond, &obj->parent.lock, then) < 0)
> goto error;
> }
> @@ -7237,7 +7238,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
> VIR_DEBUG("Started job: %s (async=%s vm=%p name=%s)",
> qemuDomainJobTypeToString(job),
> qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
> - obj, obj->def->name);
> + obj, domName);
> priv->job.active = job;
> priv->job.owner = virThreadSelfID();
> priv->job.ownerAPI = virThreadJobGet();
> @@ -7245,7 +7246,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
> } else {
> VIR_DEBUG("Started async job: %s (vm=%p name=%s)",
> qemuDomainAsyncJobTypeToString(asyncJob),
> - obj, obj->def->name);
> + obj, domName);
> qemuDomainObjResetAsyncJob(priv);
> if (VIR_ALLOC(priv->job.current) < 0)
> goto cleanup;
> @@ -7263,7 +7264,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
>
> VIR_DEBUG("Started agent job: %s (vm=%p name=%s job=%s async=%s)",
> qemuDomainAgentJobTypeToString(agentJob),
> - obj, obj->def->name,
> + obj, domName,
> qemuDomainJobTypeToString(priv->job.active),
> qemuDomainAsyncJobTypeToString(priv->job.asyncJob));
> priv->job.agentActive = agentJob;
> @@ -7294,7 +7295,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
> qemuDomainJobTypeToString(job),
> qemuDomainAgentJobTypeToString(agentJob),
> qemuDomainAsyncJobTypeToString(asyncJob),
> - obj->def->name,
> + domName,
> qemuDomainJobTypeToString(priv->job.active),
> qemuDomainAgentJobTypeToString(priv->job.agentActive),
> qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
> @@ -7507,13 +7508,14 @@ qemuDomainObjEndJob(virQEMUDriverPtr driver, virDomainObjPtr obj)
> {
> qemuDomainObjPrivatePtr priv = obj->privateData;
> qemuDomainJob job = priv->job.active;
> + const char *domName = NULLSTR(obj->def ? obj->def->name : NULL);
>
> priv->jobs_queued--;
>
> VIR_DEBUG("Stopping job: %s (async=%s vm=%p name=%s)",
> qemuDomainJobTypeToString(job),
> qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
> - obj, obj->def->name);
> + obj, domName);
>
> qemuDomainObjResetJob(priv);
> if (qemuDomainTrackJob(job))
> @@ -7528,13 +7530,14 @@ qemuDomainObjEndAgentJob(virDomainObjPtr obj)
> {
> qemuDomainObjPrivatePtr priv = obj->privateData;
> qemuDomainAgentJob agentJob = priv->job.agentActive;
> + const char *domName = NULLSTR(obj->def ? obj->def->name : NULL);
>
> 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);
> + obj, domName);
>
> qemuDomainObjResetAgentJob(priv);
> /* We indeed need to wake up ALL threads waiting because
> @@ -7549,6 +7552,7 @@ qemuDomainObjEndJobWithAgent(virQEMUDriverPtr driver,
> qemuDomainObjPrivatePtr priv = obj->privateData;
> qemuDomainJob job = priv->job.active;
> qemuDomainAgentJob agentJob = priv->job.agentActive;
> + const char *domName = NULLSTR(obj->def ? obj->def->name : NULL);
>
> priv->jobs_queued--;
>
> @@ -7556,7 +7560,7 @@ qemuDomainObjEndJobWithAgent(virQEMUDriverPtr driver,
> qemuDomainJobTypeToString(job),
> qemuDomainAgentJobTypeToString(agentJob),
> qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
> - obj, obj->def->name);
> + obj, domName);
>
> qemuDomainObjResetJob(priv);
> qemuDomainObjResetAgentJob(priv);
> @@ -7571,12 +7575,13 @@ void
> qemuDomainObjEndAsyncJob(virQEMUDriverPtr driver, virDomainObjPtr obj)
> {
> qemuDomainObjPrivatePtr priv = obj->privateData;
> + const char *domName = NULLSTR(obj->def ? obj->def->name : NULL);
>
> priv->jobs_queued--;
>
> VIR_DEBUG("Stopping async job: %s (vm=%p name=%s)",
> qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
> - obj, obj->def->name);
> + obj, domName);
>
> qemuDomainObjResetAsyncJob(priv);
> qemuDomainObjSaveJob(driver, obj);
>
Printing NULL instead of name does not look nice to me even this is just debug messages.
AFAIK in case of qemu driver we drop the lock during jobs only for active domains so
on defineXML we can acquire job only for active domains too, then we don't need to print NULLs.
Not sure about other drivers though...
Nikolay
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
