From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59480) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c2hJ5-00065u-G5 for qemu-devel@nongnu.org; Fri, 04 Nov 2016 12:25:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c2hJ1-0008VD-HL for qemu-devel@nongnu.org; Fri, 04 Nov 2016 12:25:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58422) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c2hJ1-0008UC-AY for qemu-devel@nongnu.org; Fri, 04 Nov 2016 12:25:27 -0400 Date: Fri, 4 Nov 2016 14:04:37 -0200 From: Marcelo Tosatti Message-ID: <20161104160437.GB22546@amt.cnet> References: <20161104094322.GA16930@amt.cnet> <20161104152522.GC5388@potion> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20161104152522.GC5388@potion> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [QEMU PATCH] kvmclock: advance clock by time window between vm_stop and pre_save List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Radim =?utf-8?B?S3LEjW3DocWZ?= Cc: kvm@vger.kernel.org, qemu-devel , "Dr. David Alan Gilbert" , Paolo Bonzini , Juan Quintela , Eduardo Habkost On Fri, Nov 04, 2016 at 04:25:23PM +0100, Radim Kr=C4=8Dm=C3=A1=C5=99 wro= te: > 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(),=20 > > which includes copying the remaining RAM to destination, > > and advances the clock by that amount. > >=20 > > In a VM with 5 seconds downtime, this reduces the guest=20 > > clock difference on destination from 5s to 0.2s. > >=20 > > Please do not apply this yet as some codepaths still need > > checking, submitting early for comments. >=20 > The time computation looks ok. >=20 > 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. Yes, can be enhanced later... That should be about 1us? (return to userspace). > > Signed-off-by: Marcelo Tosatti > >=20 > > 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 *opaqu= e, int running, > > s->clock =3D time_at_migration; > > } > > =20 > > + if (s->advance_clock && s->clock + s->advance_clock > s->clo= ck) { > > + s->clock +=3D s->advance_clock; > > + s->advance_clock =3D 0; > > + } >=20 > Can't the advance_clock added to the migrated KVMClockState instead of > passing it as another parameter? Don't want to be fragile to ->ns being overwritten by pre_save executing on the destination and overwriting the value passed in from migration...=20 > (It is sad that we can't just query KVMClockState in kvmclock_pre_save > because of the Linux bug.) >=20 > > @@ -135,6 +146,18 @@ static void kvmclock_vm_state_change(void *opaqu= e, int running, > > abort(); > > } > > s->clock =3D data.clock; > > + /* > > + * Transition from VM-running to VM-stopped via migration? > > + * Record when the VM was stopped. > > + */ > > + > > + if (state =3D=3D RUN_STATE_FINISH_MIGRATE && > > + !migration_in_postcopy(migrate_get_current())) { > > + clock_gettime(CLOCK_MONOTONIC, &s->t_aftervmstop); >=20 > How big (more like small) was the clock delta between here and > kvmclock_pre_save with postcopy? Haven't checked, but the clock delta is now 0.2s.=20 So i assume postcopy (stopping the VM, saving the device states) should be about 0.2s. I'll run and let you know.