From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43697) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cBMHJ-00011F-Kb for qemu-devel@nongnu.org; Mon, 28 Nov 2016 08:47:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cBMHF-0003n3-OE for qemu-devel@nongnu.org; Mon, 28 Nov 2016 08:47:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58696) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cBMHF-0003mq-GM for qemu-devel@nongnu.org; Mon, 28 Nov 2016 08:47:25 -0500 References: <20161114140028.GA25935@amt.cnet> <62d634ab-70ad-4be7-1622-f2e3a9d865fe@redhat.com> <20161114145054.GA28663@amt.cnet> <67bffd95-2e4e-7273-c154-a3fdfe622387@redhat.com> <20161114154015.GA30048@amt.cnet> <20161114171318.GA6336@amt.cnet> <14044cda-054d-94eb-8d91-7ad3a1e0869e@redhat.com> <20161114181518.GA14076@amt.cnet> <20161117121637.GA13404@amt.cnet> From: Paolo Bonzini Message-ID: <1023a283-77a7-45e5-8877-6264e08d0658@redhat.com> Date: Mon, 28 Nov 2016 14:47:18 +0100 MIME-Version: 1.0 In-Reply-To: <20161117121637.GA13404@amt.cnet> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [qemu patch 2/2] kvmclock: reduce kvmclock difference on migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcelo Tosatti Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org, "Dr. David Alan Gilbert" , Juan Quintela , Radim Krcmar , Eduardo Habkost On 17/11/2016 13:16, Marcelo Tosatti wrote: > What QEMU wants is to use KVM_GET_CLOCK at pre_save independently > of whether masterclock is enabled or not... it just depends > on KVM_GET_CLOCK being correct for the masterclock case > (108b249c453dd7132599ab6dc7e435a7036c193f). >=20 > So a "reliable KVM_GET_CLOCK" (that does not timebackward > when masterclock is enabled) is much simpler to userspace > than "whether masterclock is enabled or not". >=20 > If you have a reason why that should not be the case, > let me know. I still cannot understand this case. If the source has masterclock _disabled_, shouldn't it read kvmclock=20 from memory? But that's not what your patch does. To be clear, what I mean is: diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c index 653b0b4..6f6e2dc 100644 --- a/hw/i386/kvm/clock.c +++ b/hw/i386/kvm/clock.c @@ -97,6 +97,7 @@ static uint64_t kvm_get_clock(void) fprintf(stderr, "KVM_GET_CLOCK failed: %s\n", strerror(ret)); abort(); } + s->src_use_reliable_get_clock =3D data.flags & KVM_CLOCK_TSC_STABLE; return data.clock; } =20 @@ -110,34 +111,19 @@ static void kvmclock_vm_state_change(void *opaque, = int running, =20 if (running) { struct kvm_clock_data data =3D {}; - uint64_t pvclock_via_mem =3D 0; =20 - /* local (running VM) restore */ - if (s->clock_valid) { - /* - * if host does not support reliable KVM_GET_CLOCK, - * read kvmclock value from memory - */ - if (!kvm_has_adjust_clock_stable()) { - pvclock_via_mem =3D kvmclock_current_nsec(s); - } - /* migration/savevm/init restore */ - } else { - /* - * use s->clock in case machine uses reliable - * get clock and source host supported - * reliable get clock - */ - if (!s->src_use_reliable_get_clock) { - pvclock_via_mem =3D kvmclock_current_nsec(s); + /* + * if last KVM_GET_CLOCK did not retrieve a reliable value, + * we can't rely on the saved clock value. Just discard it and + * read kvmclock value from memory + */ + if (!s->src_use_reliable_get_clock) { + uint64_t pvclock_via_mem =3D kvmclock_current_nsec(s); + if (pvclock_via_mem) { + s->clock =3D pvclock_via_mem; } } =20 - /* We can't rely on the saved clock value, just discard it */ - if (pvclock_via_mem) { - s->clock =3D pvclock_via_mem; - } - s->clock_valid =3D false; =20 data.clock =3D s->clock; @@ -181,16 +168,6 @@ static void kvmclock_realize(DeviceState *dev, Error= **errp) { KVMClockState *s =3D KVM_CLOCK(dev); =20 - /* - * On machine types that support reliable KVM_GET_CLOCK, - * if host kernel does provide reliable KVM_GET_CLOCK, - * set src_use_reliable_get_clock=3Dtrue so that destination - * avoids reading kvmclock from memory. - */ - if (s->mach_use_reliable_get_clock && kvm_has_adjust_clock_stable())= { - s->src_use_reliable_get_clock =3D true; - } - qemu_add_vm_change_state_handler(kvmclock_vm_state_change, s); } =20