On Wed, Jan 03, 2018 at 11:26:12AM +0100, Juan Quintela wrote: > Peter Xu <pet...@redhat.com> wrote: > > On Wed, Jan 03, 2018 at 10:05:07AM +0100, Juan Quintela wrote: > >> Peter Xu <pet...@redhat.com> 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
