| Message ID | 20191017101806.3644-1-lvivier@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | runstate: ignore exit request in finish migrate state | expand |
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? Paolo
* 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(); > Paolo Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
"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
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> Queued, thanks. Paolo > --- > 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); >
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);
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(+)