Re: [libvirt] [PATCH 5/6] qemu: Recompute downtime and total time when migration completes
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(a)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