From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eduardo Habkost Subject: Re: [qemu patch V2 2/2] kvmclock: reduce kvmclock difference on migration Date: Thu, 17 Nov 2016 15:41:22 -0200 Message-ID: <20161117174122.GY5057@thinpad.lan.raisama.net> References: <20161117132411.125108658@redhat.com> <20161117132432.367332082@redhat.com> <20161117141126.GW5057@thinpad.lan.raisama.net> <20161117163051.GA27675@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Paolo Bonzini , kvm@vger.kernel.org, qemu-devel@nongnu.org, "Dr. David Alan Gilbert" , Juan Quintela , Radim Krcmar To: Marcelo Tosatti Return-path: Received: from mx1.redhat.com ([209.132.183.28]:59660 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934311AbcKQRlZ (ORCPT ); Thu, 17 Nov 2016 12:41:25 -0500 Content-Disposition: inline In-Reply-To: <20161117163051.GA27675@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Nov 17, 2016 at 02:30:52PM -0200, Marcelo Tosatti wrote: > On Thu, Nov 17, 2016 at 03:15:03PM +0100, Paolo Bonzini wrote: > > On 17/11/2016 15:11, Eduardo Habkost wrote: > > > On Thu, Nov 17, 2016 at 11:24:13AM -0200, Marcelo Tosatti wrote: [...] > > > What exactly ensures src_use_reliable_get_clock is correctly > > > initialized on the migration destination as well? > > > > > >> + > > >> + return s->mach_use_reliable_get_clock; > > > > > > If if kvm_has_adjust_clock_stable() is false, isn't it simpler to > > > simply skip the section? > > > > This is what I asked for. :) > > > > However, I was proposing a different way to initialize > > src_use_reliable_get_clock. I still have to understand exactly how > > Marcelo's algorithm works because (based on the kvmclock code) it's more > > trick than it seems. > > > > Paolo > > You asked me to return s->mach_use_reliable_get_clock: > > >>> + * avoids reading kvmclock from memory. > >>> + */ > >>> + if (s->mach_use_reliable_get_clock && > >>> kvm_has_adjust_clock_stable()) { > >>> + s->src_use_reliable_get_clock = true; > >>> + } > >>> + > >>> + return s->src_use_reliable_get_clock; > >>> +} > >> > >> Here you can just return s->mach_use_reliable_get_clock. > > > > mach_use_reliable_get_clock can be true but host might not support it. > > Yes, but the "needed" function is only required to avoid breaking > pc-i440fx-2.7 and earlier. If you return true here, you can still > migrate a "false" value for src_use_reliable_get_clock. > > =================== > > I don't see why avoid the subsection, since new machine type is > incompatible anyway. So Eduardo on your suggestion to > skip sending the subsection, what is the advantage? I believe the main (only?) advantage is that it can make the code simpler: if you simply skip the section if the field is false, you don't even need two separate fields. -- Eduardo From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37153) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c7Qgj-0004eC-HH for qemu-devel@nongnu.org; Thu, 17 Nov 2016 12:41:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c7Qgf-00015Y-Jp for qemu-devel@nongnu.org; Thu, 17 Nov 2016 12:41:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55078) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c7Qgf-00015G-Do for qemu-devel@nongnu.org; Thu, 17 Nov 2016 12:41:25 -0500 Date: Thu, 17 Nov 2016 15:41:22 -0200 From: Eduardo Habkost Message-ID: <20161117174122.GY5057@thinpad.lan.raisama.net> References: <20161117132411.125108658@redhat.com> <20161117132432.367332082@redhat.com> <20161117141126.GW5057@thinpad.lan.raisama.net> <20161117163051.GA27675@amt.cnet> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161117163051.GA27675@amt.cnet> Subject: Re: [Qemu-devel] [qemu patch V2 2/2] kvmclock: reduce kvmclock difference on migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcelo Tosatti Cc: Paolo Bonzini , kvm@vger.kernel.org, qemu-devel@nongnu.org, "Dr. David Alan Gilbert" , Juan Quintela , Radim Krcmar On Thu, Nov 17, 2016 at 02:30:52PM -0200, Marcelo Tosatti wrote: > On Thu, Nov 17, 2016 at 03:15:03PM +0100, Paolo Bonzini wrote: > > On 17/11/2016 15:11, Eduardo Habkost wrote: > > > On Thu, Nov 17, 2016 at 11:24:13AM -0200, Marcelo Tosatti wrote: [...] > > > What exactly ensures src_use_reliable_get_clock is correctly > > > initialized on the migration destination as well? > > > > > >> + > > >> + return s->mach_use_reliable_get_clock; > > > > > > If if kvm_has_adjust_clock_stable() is false, isn't it simpler to > > > simply skip the section? > > > > This is what I asked for. :) > > > > However, I was proposing a different way to initialize > > src_use_reliable_get_clock. I still have to understand exactly how > > Marcelo's algorithm works because (based on the kvmclock code) it's more > > trick than it seems. > > > > Paolo > > You asked me to return s->mach_use_reliable_get_clock: > > >>> + * avoids reading kvmclock from memory. > >>> + */ > >>> + if (s->mach_use_reliable_get_clock && > >>> kvm_has_adjust_clock_stable()) { > >>> + s->src_use_reliable_get_clock = true; > >>> + } > >>> + > >>> + return s->src_use_reliable_get_clock; > >>> +} > >> > >> Here you can just return s->mach_use_reliable_get_clock. > > > > mach_use_reliable_get_clock can be true but host might not support it. > > Yes, but the "needed" function is only required to avoid breaking > pc-i440fx-2.7 and earlier. If you return true here, you can still > migrate a "false" value for src_use_reliable_get_clock. > > =================== > > I don't see why avoid the subsection, since new machine type is > incompatible anyway. So Eduardo on your suggestion to > skip sending the subsection, what is the advantage? I believe the main (only?) advantage is that it can make the code simpler: if you simply skip the section if the field is false, you don't even need two separate fields. -- Eduardo