From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37174) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eWgTU-0000z7-Oc for qemu-devel@nongnu.org; Wed, 03 Jan 2018 05:40:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eWgTP-0007hB-PZ for qemu-devel@nongnu.org; Wed, 03 Jan 2018 05:40:44 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42936) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eWgTP-0007gX-JD for qemu-devel@nongnu.org; Wed, 03 Jan 2018 05:40:39 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 89880C0587FA for ; Wed, 3 Jan 2018 10:40:38 +0000 (UTC) Date: Wed, 3 Jan 2018 18:40:34 +0800 From: Peter Xu Message-ID: <20180103104034.GK2557@xz-mi> References: <20180103054043.25719-1-peterx@redhat.com> <20180103054043.25719-6-peterx@redhat.com> <87shbn2tz0.fsf@secure.laptop> <20180103092025.GF2557@xz-mi> <87o9mb1bnf.fsf@secure.laptop> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <87o9mb1bnf.fsf@secure.laptop> 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: Juan Quintela Cc: qemu-devel@nongnu.org, Laurent Vivier , "Dr . David Alan Gilbert" On Wed, Jan 03, 2018 at 11:26:12AM +0100, Juan Quintela wrote: > 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? Sure. I was mostly trying to fix the typo and grammar issue. Will take your advise. -- Peter Xu