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

[Juan Quintela <quintela@redhat.com>]

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

Later, Juan.


> ?  Thanks,