Re: [libvirt PATCH 17/80] qemu: Restore async job start timestamp on reconnect

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 11, 2022 at 15:04:53 +0200, Peter Krempa wrote:
> 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.

We can keep this fallback in qemuDomainObjRestoreAsyncJob instead of
putting it in the parser to avoid test issues.

Jirka




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux