[libvirt] [PATCH 5/6] qemu: Recompute downtime and total time when migration completes

[Jiri Denemark]

On Fri, Sep 05, 2014 at 14:47:35 -0400, John Ferlan wrote:
> 
> 
> On 09/01/2014 11:05 AM, Jiri Denemark wrote:
> > Total time of a migration and total downtime transfered from a source to
> > a destination host do not count with the transfer time to the
> > destination host and with the time elapsed before guest CPUs are
> > resumed. Thus, source libvirtd remembers when migration started and when
> > guest CPUs were paused. Both timestamps are transferred to destination
> > libvirtd which uses them to compute total migration time and total
> > downtime. This, obviously, requires clock to be synchronized between the
> 
> s/This, obviously,/Obviously this/
> 
> "requires clock" reads funny... "requires the time" seems closer
> 
> > two hosts. The reported times are useless otherwise but they would be
> > equally useless if we didn't do this recomputation so don't lose
> > anything by doing it.
> > 
> 
> Say nothing of inter-timezone migrations right?

There are no timezones in timestamps. It's all UTC until you try to
convert it into something humans understand, which we don't need to do
:-)

> > Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> > ---
> >  src/libvirt.c             |  5 ++++-
> >  src/qemu/qemu_domain.c    | 28 ++++++++++++++++++++++++++++
> >  src/qemu/qemu_domain.h    |  2 ++
> >  src/qemu/qemu_migration.c | 15 ++++++++++++++-
> >  src/qemu/qemu_process.c   |  9 ++++++++-
> >  tools/virsh.pod           |  5 ++++-
> >  6 files changed, 60 insertions(+), 4 deletions(-)
> > 
...
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index 18a3761..cec7828 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -222,11 +222,39 @@ qemuDomainJobInfoUpdateTime(qemuDomainJobInfoPtr jobInfo)
> >      if (virTimeMillisNow(&now) < 0)
> >          return -1;
> >  
> > +    if (now < jobInfo->started) {
> > +        VIR_WARN("Async job starts in the future");
> > +        jobInfo->started = 0;
> > +        return 0;
> > +    }
> > +
> >      jobInfo->timeElapsed = now - jobInfo->started;
> >      return 0;
> >  }
> >  
> >  int
> > +qemuDomainJobInfoUpdateDowntime(qemuDomainJobInfoPtr jobInfo)
> > +{
> > +    unsigned long long now;
> > +
> 
> Can jobInfo == NULL? - It's the qemuMigrationWaitForCompletion() path
> timing concern from patch 1.

No, I added ATTRIBUTE_NONNULL to this API too.

> 
> > +    if (!jobInfo->stopped)
> > +        return 0;
> > +
> > +    if (virTimeMillisNow(&now) < 0)
> > +        return -1;
> > +
> > +    if (now < jobInfo->stopped) {
> > +        VIR_WARN("Guest's CPUs stopped in the future");
> > +        jobInfo->stopped = 0;
> > +        return 0;
> > +    }
> > +
> > +    jobInfo->status.downtime = now - jobInfo->stopped;
> > +    jobInfo->status.downtime_set = true;
> > +    return 0;
> > +}
> > +
> > +int
> >  qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo,
> >                          virDomainJobInfoPtr info)
> >  {
> > diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> > index 365238b..435a22b 100644
> > --- a/src/qemu/qemu_domain.h
> > +++ b/src/qemu/qemu_domain.h
> > @@ -105,6 +105,7 @@ typedef qemuDomainJobInfo *qemuDomainJobInfoPtr;
> >  struct _qemuDomainJobInfo {
> >      virDomainJobType type;
> >      unsigned long long started; /* When the async job started */
> > +    unsigned long long stopped; /* When the domain's CPUs were stopped */
> >      /* Computed values */
> >      unsigned long long timeElapsed;
> >      unsigned long long timeRemaining;
> > @@ -390,6 +391,7 @@ bool qemuDomainAgentAvailable(qemuDomainObjPrivatePtr priv,
> >                                bool reportError);
> >  
> >  int qemuDomainJobInfoUpdateTime(qemuDomainJobInfoPtr jobInfo);
> > +int qemuDomainJobInfoUpdateDowntime(qemuDomainJobInfoPtr jobInfo);
> 
> Does this also need some sort of ATTRIBUTE_NONNULL(1)?

Done.

Jirka