From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58003) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eWgFc-0000ib-2O for qemu-devel@nongnu.org; Wed, 03 Jan 2018 05:26:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eWgFY-0007Tc-U2 for qemu-devel@nongnu.org; Wed, 03 Jan 2018 05:26:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33768) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eWgFY-0007St-My for qemu-devel@nongnu.org; Wed, 03 Jan 2018 05:26:20 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A1E0BC059B6B for ; Wed, 3 Jan 2018 10:26:19 +0000 (UTC) From: Juan Quintela In-Reply-To: <20180103092025.GF2557@xz-mi> (Peter Xu's message of "Wed, 3 Jan 2018 17:20:25 +0800") References: <20180103054043.25719-1-peterx@redhat.com> <20180103054043.25719-6-peterx@redhat.com> <87shbn2tz0.fsf@secure.laptop> <20180103092025.GF2557@xz-mi> Reply-To: quintela@redhat.com Date: Wed, 03 Jan 2018 11:26:12 +0100 Message-ID: <87o9mb1bnf.fsf@secure.laptop> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 05/11] migration: move vm_old_running into global state List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu Cc: qemu-devel@nongnu.org, Laurent Vivier , "Dr . David Alan Gilbert" Peter Xu wrote: > On Wed, Jan 03, 2018 at 10:05:07AM +0100, Juan Quintela wrote: >> Peter Xu wrote: >> > Firstly, it was passed around. Let's just move it into MigrationState >> > just like many other variables as state of migration. >> > >> > One thing to mention is that for postcopy, we actually don't need this >> > knowledge at all since postcopy can't resume a VM even if it fails (we >> > can see that from the old code too: when we try to resume we also check >> > against "entered_postcopy" variable). So further we do this: >> > >> > - in postcopy_start(), we don't update vm_old_running since useless >> > - in migration_thread(), we don't need to check entered_postcopy when >> > resume, since it's only used for precopy. >> > >> > Comment this out too for that variable definition. >> I think this comment is right for precopy, but not for colo. BTW, I >> think that I would put the postcopy comment on its use, not here. > > Or, how about I just don't mention postcopy at all? Fully agree. >> >> /me tries to improve the comment >> >> Guest was running when we enter the completion stage. If migration don't >> sucess, we need to continue running guest on source. >> >> What do you think? > > I think it's generally good. Maybe a tiny fix like: > > s/Guest was/Whether guest was/ ok. > s/If migration don't sucess/If migration failed/ We also use it in case of migration_cancel. Cancel is not one error, that is why I wrote it that way. What about: Whether guest was running when we enter the completion stage. If migration is interrupted by any reason, we need to continue running the guest on source. What do you think? Later, Juan. > ? Thanks,