From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [QEMU PATCH] kvmclock: advance clock by time window between vm_stop and pre_save Date: Fri, 4 Nov 2016 17:29:36 -0400 (EDT) Message-ID: <595276440.10962400.1478294976364.JavaMail.zimbra@redhat.com> References: <20161104094322.GA16930@amt.cnet> <20161104152522.GC5388@potion> <1c69a083-eef0-8fa0-0e74-5a4e25a066a0@redhat.com> <20161104154827.GD5388@potion> <1217f6ce-8143-8d0e-97f6-470926438992@redhat.com> <20161104171605.GE5388@potion> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm@vger.kernel.org, qemu-devel , "Dr. David Alan Gilbert" , Juan Quintela , Eduardo Habkost , Roman Kagan To: Radim =?utf-8?B?S3LEjW3DocWZ?= Return-path: Received: from mx6-phx2.redhat.com ([209.132.183.39]:34857 "EHLO mx6-phx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753131AbcKDV3l (ORCPT ); Fri, 4 Nov 2016 17:29:41 -0400 In-Reply-To: <20161104171605.GE5388@potion> Sender: kvm-owner@vger.kernel.org List-ID: > >> No, the one that forced Marcelo to add the 10 minute limit to the > >> advance_clock. We wouldn't need this advance_clock hack if we could > >> just call KVM_GET_CLOCK like we did before 00f4d64ee76e ("kvmclock: > >> clock should count only if vm is running"). > > > > There are two cases: > > > > - migrating a paused guest > > > > - pausing at the end of migration > > > > In the first case, kvmclock_vm_state_change's !running branch will see > > state == RUN_STATE_FINISH_MIGRATE && s->clock_valid. In the second > > case, it will see state == RUN_STATE_FINISH_MIGRATE && !s->clock_valid. > > I lift my case, marcelo's said that stopping the time is a feature ... > (*kittens die*) But that's why separating the two cases brings us the best of both worlds. If migrating a paused guest, there's no need for any adjustment, so no advance_clock hack. If pausing at the end of migration, there's no need to pause kvmclock (this patch is effectively working around 00f4d64ee76e) and if we don't do that we can just call KVM_GET_CLOCK at pre_save time. > Oh, and this does introduce a minor bug to this patch -- the time > counted by KVM_GET_CLOCK is has different frequency CLOCK_MONOTONIC. > Not accounting for that is bearable. Not really, I was going to point that out when I got to replying with a review. :) Paolo From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46368) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c2m3T-0001xC-Ny for qemu-devel@nongnu.org; Fri, 04 Nov 2016 17:29:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c2m3Q-00025C-Ia for qemu-devel@nongnu.org; Fri, 04 Nov 2016 17:29:43 -0400 Received: from mx6-phx2.redhat.com ([209.132.183.39]:39606) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c2m3Q-00022Q-Az for qemu-devel@nongnu.org; Fri, 04 Nov 2016 17:29:40 -0400 Date: Fri, 4 Nov 2016 17:29:36 -0400 (EDT) From: Paolo Bonzini Message-ID: <595276440.10962400.1478294976364.JavaMail.zimbra@redhat.com> In-Reply-To: <20161104171605.GE5388@potion> References: <20161104094322.GA16930@amt.cnet> <20161104152522.GC5388@potion> <1c69a083-eef0-8fa0-0e74-5a4e25a066a0@redhat.com> <20161104154827.GD5388@potion> <1217f6ce-8143-8d0e-97f6-470926438992@redhat.com> <20161104171605.GE5388@potion> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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: Marcelo Tosatti , kvm@vger.kernel.org, qemu-devel , "Dr. David Alan Gilbert" , Juan Quintela , Eduardo Habkost , Roman Kagan > >> No, the one that forced Marcelo to add the 10 minute limit to the > >> advance_clock. We wouldn't need this advance_clock hack if we could > >> just call KVM_GET_CLOCK like we did before 00f4d64ee76e ("kvmclock: > >> clock should count only if vm is running"). > > > > There are two cases: > > > > - migrating a paused guest > > > > - pausing at the end of migration > > > > In the first case, kvmclock_vm_state_change's !running branch will see > > state == RUN_STATE_FINISH_MIGRATE && s->clock_valid. In the second > > case, it will see state == RUN_STATE_FINISH_MIGRATE && !s->clock_valid. > > I lift my case, marcelo's said that stopping the time is a feature ... > (*kittens die*) But that's why separating the two cases brings us the best of both worlds. If migrating a paused guest, there's no need for any adjustment, so no advance_clock hack. If pausing at the end of migration, there's no need to pause kvmclock (this patch is effectively working around 00f4d64ee76e) and if we don't do that we can just call KVM_GET_CLOCK at pre_save time. > Oh, and this does introduce a minor bug to this patch -- the time > counted by KVM_GET_CLOCK is has different frequency CLOCK_MONOTONIC. > Not accounting for that is bearable. Not really, I was going to point that out when I got to replying with a review. :) Paolo