From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [QEMU PATCH] kvmclock: advance clock by time window between vm_stop and pre_save Date: Fri, 4 Nov 2016 16:25:23 +0100 Message-ID: <20161104152522.GC5388@potion> References: <20161104094322.GA16930@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, qemu-devel , "Dr. David Alan Gilbert" , Paolo Bonzini , Juan Quintela , Eduardo Habkost To: Marcelo Tosatti Return-path: Received: from mx1.redhat.com ([209.132.183.28]:48784 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934358AbcKDPZ1 (ORCPT ); Fri, 4 Nov 2016 11:25:27 -0400 Content-Disposition: inline In-Reply-To: <20161104094322.GA16930@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: 2016-11-04 07:43-0200, Marcelo Tosatti: > This patch, relative to pre-copy migration codepath, > measures the time between vm_stop() and pre_save(), > which includes copying the remaining RAM to destination, > and advances the clock by that amount. > > In a VM with 5 seconds downtime, this reduces the guest > clock difference on destination from 5s to 0.2s. > > Please do not apply this yet as some codepaths still need > checking, submitting early for comments. The time computation looks ok. We could make it slightly more precise by returning the CLOCK_MONOTONIC at which KVM_GET_CLOCK is read with the IOCTL, but we don't account the migration time anyway, so that precision would be wasted. > Signed-off-by: Marcelo Tosatti > > diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c > @@ -100,6 +106,11 @@ static void kvmclock_vm_state_change(void *opaque, int running, > s->clock = time_at_migration; > } > > + if (s->advance_clock && s->clock + s->advance_clock > s->clock) { > + s->clock += s->advance_clock; > + s->advance_clock = 0; > + } Can't the advance_clock added to the migrated KVMClockState instead of passing it as another parameter? (It is sad that we can't just query KVMClockState in kvmclock_pre_save because of the Linux bug.) > @@ -135,6 +146,18 @@ static void kvmclock_vm_state_change(void *opaque, int running, > abort(); > } > s->clock = data.clock; > + /* > + * Transition from VM-running to VM-stopped via migration? > + * Record when the VM was stopped. > + */ > + > + if (state == RUN_STATE_FINISH_MIGRATE && > + !migration_in_postcopy(migrate_get_current())) { > + clock_gettime(CLOCK_MONOTONIC, &s->t_aftervmstop); How big (more like small) was the clock delta between here and kvmclock_pre_save with postcopy? Thanks.