Re: [Qemu-devel] [PATCH 05/11] migration: move vm_old_running into global state

[Peter Xu]

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