Re: [PATCH] runstate: ignore exit request in finish migrate state

[Markus Armbruster <armbru@redhat.com>]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Paolo Bonzini (pbonzini@redhat.com) wrote:
>> On 17/10/19 12:18, Laurent Vivier wrote:
>> > Trying to reboot a VM while a migration is running can
>> > move to the prelaunch state (because of the reset) while
>> > the runstate is in finish migrate state.
>> > As the logical step after the finish migrate is postmigrate,
>> > this can create an invalid state transition from prelaunch state
>> > to postmigrate state and this raises an error and aborts:
>> > 
>> >     invalid runstate transition: 'prelaunch' -> 'postmigrate'
>> > 
>> > As we are not able to manage reset in finish migrate state the
>> > best we can do is to ignore any changes and delay them until
>> > the next state which should be postmigrate and which should allow
>> > this kind of transition.
>> > 
>> > Reported-by: Lukáš Doktor <ldoktor@redhat.com>
>> > Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> > ---
>> >  vl.c | 3 +++
>> >  1 file changed, 3 insertions(+)
>> > 
>> > diff --git a/vl.c b/vl.c
>> > index 0a295e5d77d6..dc71c822ba24 100644
>> > --- a/vl.c
>> > +++ b/vl.c
>> > @@ -1744,6 +1744,9 @@ static bool main_loop_should_exit(void)
>> >      RunState r;
>> >      ShutdownCause request;
>> >  
>> > +    if (runstate_check(RUN_STATE_FINISH_MIGRATE)) {
>> > +        return false;
>> > +    }
>> >      if (preconfig_exit_requested) {
>> >          if (runstate_check(RUN_STATE_PRECONFIG)) {
>> >              runstate_set(RUN_STATE_PRELAUNCH);
>> > 
>> 
>> Your patch makes sense, but I don't understand this function very much.
>>  In particular I don't understand why it returns true when
>> preconfig_exit_requested is true.  Wouldn't that cause main_loop() and
>> thus QEMU to exit?  Igor, can you help?
>
> It's because we now run main_loop() twice; once in the preconfig state
> and once the main main loop.
>
>
> 4323     /* do monitor/qmp handling at preconfig state if requested */
> 4324     main_loop();
>
> ....
>
> 4452     os_setup_post();
> 4453 
> 4454     main_loop();
> 4455 
> 4456     gdbserver_cleanup();

Commit 047f7038f5 "cli: add --preconfig option"

The whole thing is (in my opinion) a cheap hack we accepted for want of
better ideas we could get done in time.  Sometimes you have to hold you
nose and deliver.

https://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg05584.html