From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [qemu patch V3 2/2] kvmclock: reduce kvmclock difference on migration Date: Mon, 28 Nov 2016 18:31:04 +0100 Message-ID: <3dba7b62-bfd2-6551-d983-89541afbcee4@redhat.com> References: <20161121105002.291554230@redhat.com> <20161121105052.598267440@redhat.com> <20161128141322.GB14328@thinpad.lan.raisama.net> <20161128164522.GB4028@amt.cnet> <20161128171201.GD14328@thinpad.lan.raisama.net> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org, Dr David Alan Gilbert , Juan Quintela , Radim Krcmar To: Eduardo Habkost , Marcelo Tosatti Return-path: Received: from mx1.redhat.com ([209.132.183.28]:60166 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751120AbcK1RbJ (ORCPT ); Mon, 28 Nov 2016 12:31:09 -0500 In-Reply-To: <20161128171201.GD14328@thinpad.lan.raisama.net> Sender: kvm-owner@vger.kernel.org List-ID: On 28/11/2016 18:12, Eduardo Habkost wrote: >>> > > >>> > > Why not use src_use_reliable_get_clock here, too? (Maybe rename >>> > > it to simply clock_is_reliable?) >> > >> > Because you end up mixing two mental ideas (one: whether the source >> > has KVM_GET_CLOCK, second: whether the destination has KVM_GET_CLOCK >> > into one variable). I find it more confusing than not. >> > Maybe its just my limited brain but i find it >> > confusing. > I find it simpler and easier to reason about. Me too---note that it's not "whether X had reliable KVM_GET_CLOCK" but rather "whether the last read came from a reliable KVM_GET_CLOCK". However... >>> > > You can change kvm_get_clock() to kvm_get_clock(KVMClockState*), >>> > > and make it update both s->clock and s->clock_is_reliable. With >>> > > this, s->clock and s->clock_is_reliable will be always in sync, >>> > > and have a single code path for both local restore and migration: >> > >> > There should not be a single path for migration/runtime cases: >> > on migration, we care whether the source used reliable KVM_GET_CLOCK. >> > >> > On runtime, we care whether the destination uses reliable KVM_GET_CLOCK. > The conditions may be different, but both can be translated to a > "should we trust the value in s->clock" flag, to simplify the > code (just like Paolo's suggestion on his reply to v1 today). ... I think I understand now: Marcelo is approaching the problem differently. For him it's not an issue of "should we trust the value" but rather it's "is the value going to mess up the guest with backwards time". Which is fine, it just requires some more comments because it's subtle and it looks a lot like the kernel API is being misused (while it's fine). Paolo