On Tue, May 10, 2022 at 17:20:38 +0200, Jiri Denemark wrote:
> Jobs that are supposed to remain active even when libvirt daemon
> restarts were reported as started at the time the daemon was restarted.
> This is not very helpful, we should restore the original timestamp.
>
> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> ---
> src/qemu/qemu_domainjob.c | 20 +++++++++++++------
> src/qemu/qemu_domainjob.h | 1 +
> src/qemu/qemu_process.c | 4 +++-
> .../migration-in-params-in.xml | 2 +-
> .../migration-out-nbd-bitmaps-in.xml | 2 +-
> .../migration-out-nbd-out.xml | 2 +-
> .../migration-out-nbd-tls-out.xml | 2 +-
> .../migration-out-params-in.xml | 2 +-
> 8 files changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c
> index 1f82457bd4..8e8d229afe 100644
> --- a/src/qemu/qemu_domainjob.c
> +++ b/src/qemu/qemu_domainjob.c
[...]
> @@ -261,18 +263,15 @@ qemuDomainObjRestoreAsyncJob(virDomainObj *vm,
> {
> qemuDomainObjPrivate *priv = vm->privateData;
> qemuDomainJobObj *job = &priv->job;
> - unsigned long long now;
>
> VIR_DEBUG("Restoring %s async job for domain %s",
> virDomainAsyncJobTypeToString(asyncJob), vm->def->name);
>
> - ignore_value(virTimeMillisNow(&now));
> -
> priv->job.jobsQueued++;
> job->asyncJob = asyncJob;
> job->phase = phase;
> job->asyncOwnerAPI = g_strdup(virThreadJobGet());
> - job->asyncStarted = now;
> + job->asyncStarted = started;
>
> qemuDomainObjSetAsyncJobMask(vm, allowedJobs);
>
> @@ -280,7 +279,7 @@ qemuDomainObjRestoreAsyncJob(virDomainObj *vm,
> qemuDomainJobSetStatsType(priv->job.current, statsType);
> job->current->operation = operation;
> job->current->status = status;
> - job->current->started = now;
> + job->current->started = started;
> }
You don't seem to have any fallback when reading back older status XML
where ...
>
>
> @@ -1250,8 +1249,10 @@ qemuDomainObjPrivateXMLFormatJob(virBuffer *buf,
> priv->job.phase));
> }
>
> - if (priv->job.asyncJob != VIR_ASYNC_JOB_NONE)
> + if (priv->job.asyncJob != VIR_ASYNC_JOB_NONE) {
> virBufferAsprintf(&attrBuf, " flags='0x%lx'", priv->job.apiFlags);
> + virBufferAsprintf(&attrBuf, " asyncStarted='%llu'", priv->job.asyncStarted);
... the start timestamp is not printed, so ...
> + }
>
> if (priv->job.cb &&
> priv->job.cb->formatJob(&childBuf, &priv->job, vm) < 0)
> @@ -1307,6 +1308,13 @@ qemuDomainObjPrivateXMLParseJob(virDomainObj *vm,
> }
> VIR_FREE(tmp);
> }
> +
> + if (virXPathULongLong("string(@asyncStarted)", ctxt,
> + &priv->job.asyncStarted) == -2) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Invalid async job start"));
... this will keep it initialized to 0 ...
> + return -1;
> + }
> }
>
> if (virXPathULongHex("string(@flags)", ctxt, &priv->job.apiFlags) == -2) {
[...]
> diff --git a/tests/qemustatusxml2xmldata/migration-in-params-in.xml b/tests/qemustatusxml2xmldata/migration-in-params-in.xml
> index f4bc5753c4..9e9c2deac6 100644
> --- a/tests/qemustatusxml2xmldata/migration-in-params-in.xml
> +++ b/tests/qemustatusxml2xmldata/migration-in-params-in.xml
> @@ -238,7 +238,7 @@
> <flag name='dump-completed'/>
> <flag name='hda-output'/>
> </qemuCaps>
> - <job type='none' async='migration in' phase='prepare' flags='0x900'>
... so existing backup jobs will seem to be started at the beginning of
epoch:
> + <job type='none' async='migration in' phase='prepare' flags='0x900' asyncStarted='0'>
> <migParams>
> <param name='compress-level' value='1'/>
> <param name='compress-threads' value='8'/>
Probably not a big problem but we IMO should initialize it to current
time. That will break the test though unless you mock the time getting
function.
