* [Qemu-devel] [PATCH f0r 2.11] runstate/migrate: Two more transitions
@ 2017-08-04 17:50 Dr. David Alan Gilbert (git)
2017-08-07 7:21 ` Peter Xu
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-08-04 17:50 UTC (permalink / raw)
To: qemu-devel, quintela, peterx, lvivier, pbonzini
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
There's a race if someone does a 'stop' near the end of migrate;
the migration process goes through two runstates:
'finish migrate'
'postmigrate'
If the user issues a 'stop' between the two we end up with invalid
state transitions.
Add the transitions as valid.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
vl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/vl.c b/vl.c
index 99fcfa0442..bacb03f49d 100644
--- a/vl.c
+++ b/vl.c
@@ -621,6 +621,7 @@ static const RunStateTransition runstate_transitions_def[] = {
{ RUN_STATE_PAUSED, RUN_STATE_RUNNING },
{ RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
+ { RUN_STATE_PAUSED, RUN_STATE_POSTMIGRATE },
{ RUN_STATE_PAUSED, RUN_STATE_PRELAUNCH },
{ RUN_STATE_PAUSED, RUN_STATE_COLO},
@@ -633,6 +634,7 @@ static const RunStateTransition runstate_transitions_def[] = {
{ RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
{ RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
+ { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PAUSED },
{ RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
{ RUN_STATE_FINISH_MIGRATE, RUN_STATE_PRELAUNCH },
{ RUN_STATE_FINISH_MIGRATE, RUN_STATE_COLO},
--
2.13.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH f0r 2.11] runstate/migrate: Two more transitions
2017-08-04 17:50 [Qemu-devel] [PATCH f0r 2.11] runstate/migrate: Two more transitions Dr. David Alan Gilbert (git)
@ 2017-08-07 7:21 ` Peter Xu
2017-08-07 12:25 ` Dr. David Alan Gilbert
2017-08-08 7:02 ` Juan Quintela
2017-09-06 13:40 ` Dr. David Alan Gilbert
2 siblings, 1 reply; 8+ messages in thread
From: Peter Xu @ 2017-08-07 7:21 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, quintela, lvivier, pbonzini
On Fri, Aug 04, 2017 at 06:50:11PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> There's a race if someone does a 'stop' near the end of migrate;
> the migration process goes through two runstates:
> 'finish migrate'
> 'postmigrate'
>
> If the user issues a 'stop' between the two we end up with invalid
> state transitions.
> Add the transitions as valid.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> vl.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/vl.c b/vl.c
> index 99fcfa0442..bacb03f49d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -621,6 +621,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>
> { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
> { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
> + { RUN_STATE_PAUSED, RUN_STATE_POSTMIGRATE },
> { RUN_STATE_PAUSED, RUN_STATE_PRELAUNCH },
> { RUN_STATE_PAUSED, RUN_STATE_COLO},
>
> @@ -633,6 +634,7 @@ static const RunStateTransition runstate_transitions_def[] = {
> { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
>
> { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
> + { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PAUSED },
> { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
> { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PRELAUNCH },
> { RUN_STATE_FINISH_MIGRATE, RUN_STATE_COLO},
Do we need this as well?
{ RUN_STATE_POSTMIGRATE, RUN_STATE_PAUSED },
--
Peter Xu
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH f0r 2.11] runstate/migrate: Two more transitions
2017-08-07 7:21 ` Peter Xu
@ 2017-08-07 12:25 ` Dr. David Alan Gilbert
2017-08-08 1:59 ` Peter Xu
0 siblings, 1 reply; 8+ messages in thread
From: Dr. David Alan Gilbert @ 2017-08-07 12:25 UTC (permalink / raw)
To: Peter Xu; +Cc: qemu-devel, quintela, lvivier, pbonzini
* Peter Xu (peterx@redhat.com) wrote:
> On Fri, Aug 04, 2017 at 06:50:11PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > There's a race if someone does a 'stop' near the end of migrate;
> > the migration process goes through two runstates:
> > 'finish migrate'
> > 'postmigrate'
> >
> > If the user issues a 'stop' between the two we end up with invalid
> > state transitions.
> > Add the transitions as valid.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> > vl.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/vl.c b/vl.c
> > index 99fcfa0442..bacb03f49d 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -621,6 +621,7 @@ static const RunStateTransition runstate_transitions_def[] = {
> >
> > { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
> > { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
> > + { RUN_STATE_PAUSED, RUN_STATE_POSTMIGRATE },
> > { RUN_STATE_PAUSED, RUN_STATE_PRELAUNCH },
> > { RUN_STATE_PAUSED, RUN_STATE_COLO},
> >
> > @@ -633,6 +634,7 @@ static const RunStateTransition runstate_transitions_def[] = {
> > { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
> >
> > { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
> > + { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PAUSED },
> > { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
> > { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PRELAUNCH },
> > { RUN_STATE_FINISH_MIGRATE, RUN_STATE_COLO},
>
> Do we need this as well?
>
> { RUN_STATE_POSTMIGRATE, RUN_STATE_PAUSED },
Apparently not:
(qemu) migrate "exec:cat > /dev/null"
(qemu) infomigrate
unknown command: 'infomigrate'
(qemu) info status
VM status: paused (postmigrate)
(qemu) stop
(qemu) info status
VM status: paused (postmigrate)
(qemu)
so doing a stop at that point doesn't seem to cause any problem.
Dave
> --
> Peter Xu
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH f0r 2.11] runstate/migrate: Two more transitions
2017-08-07 12:25 ` Dr. David Alan Gilbert
@ 2017-08-08 1:59 ` Peter Xu
0 siblings, 0 replies; 8+ messages in thread
From: Peter Xu @ 2017-08-08 1:59 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: qemu-devel, quintela, lvivier, pbonzini
On Mon, Aug 07, 2017 at 01:25:29PM +0100, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
> > On Fri, Aug 04, 2017 at 06:50:11PM +0100, Dr. David Alan Gilbert (git) wrote:
> > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > >
> > > There's a race if someone does a 'stop' near the end of migrate;
> > > the migration process goes through two runstates:
> > > 'finish migrate'
> > > 'postmigrate'
> > >
> > > If the user issues a 'stop' between the two we end up with invalid
> > > state transitions.
> > > Add the transitions as valid.
> > >
> > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > ---
> > > vl.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/vl.c b/vl.c
> > > index 99fcfa0442..bacb03f49d 100644
> > > --- a/vl.c
> > > +++ b/vl.c
> > > @@ -621,6 +621,7 @@ static const RunStateTransition runstate_transitions_def[] = {
> > >
> > > { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
> > > { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
> > > + { RUN_STATE_PAUSED, RUN_STATE_POSTMIGRATE },
> > > { RUN_STATE_PAUSED, RUN_STATE_PRELAUNCH },
> > > { RUN_STATE_PAUSED, RUN_STATE_COLO},
> > >
> > > @@ -633,6 +634,7 @@ static const RunStateTransition runstate_transitions_def[] = {
> > > { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
> > >
> > > { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
> > > + { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PAUSED },
> > > { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
> > > { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PRELAUNCH },
> > > { RUN_STATE_FINISH_MIGRATE, RUN_STATE_COLO},
> >
> > Do we need this as well?
> >
> > { RUN_STATE_POSTMIGRATE, RUN_STATE_PAUSED },
>
> Apparently not:
>
> (qemu) migrate "exec:cat > /dev/null"
> (qemu) infomigrate
> unknown command: 'infomigrate'
> (qemu) info status
> VM status: paused (postmigrate)
> (qemu) stop
> (qemu) info status
> VM status: paused (postmigrate)
> (qemu)
>
>
> so doing a stop at that point doesn't seem to cause any problem.
Yes. Thanks.
Reviewed-by: Peter Xu <peterx@redhat.com>
--
Peter Xu
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH f0r 2.11] runstate/migrate: Two more transitions
2017-08-04 17:50 [Qemu-devel] [PATCH f0r 2.11] runstate/migrate: Two more transitions Dr. David Alan Gilbert (git)
2017-08-07 7:21 ` Peter Xu
@ 2017-08-08 7:02 ` Juan Quintela
2017-08-08 7:13 ` Peter Xu
2017-09-06 13:40 ` Dr. David Alan Gilbert
2 siblings, 1 reply; 8+ messages in thread
From: Juan Quintela @ 2017-08-08 7:02 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, peterx, lvivier, pbonzini
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> There's a race if someone does a 'stop' near the end of migrate;
> the migration process goes through two runstates:
> 'finish migrate'
> 'postmigrate'
>
> If the user issues a 'stop' between the two we end up with invalid
> state transitions.
> Add the transitions as valid.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
To answer Peter question:
int vm_stop(RunState state)
{
.... we don't care about this case ....
return do_vm_stop(state);
}
static int do_vm_stop(RunState state)
{
int ret = 0;
if (runstate_is_running()) {
cpu_disable_ticks();
pause_all_vcpus();
runstate_set(state);
vm_state_notify(0, state);
qapi_event_send_stop(&error_abort);
}
bdrv_drain_all();
replay_disable_events();
ret = bdrv_flush_all();
return ret;
}
int runstate_is_running(void)
{
return runstate_check(RUN_STATE_RUNNING);
}
So, "stop" only changes states when we are in RUNNING state.
The idea was that after migration, the only valid command (as in the
film "it should do something")is "run".
Later, Juan.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH f0r 2.11] runstate/migrate: Two more transitions
2017-08-08 7:02 ` Juan Quintela
@ 2017-08-08 7:13 ` Peter Xu
0 siblings, 0 replies; 8+ messages in thread
From: Peter Xu @ 2017-08-08 7:13 UTC (permalink / raw)
To: Juan Quintela; +Cc: Dr. David Alan Gilbert (git), qemu-devel, lvivier, pbonzini
On Tue, Aug 08, 2017 at 09:02:54AM +0200, Juan Quintela wrote:
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > There's a race if someone does a 'stop' near the end of migrate;
> > the migration process goes through two runstates:
> > 'finish migrate'
> > 'postmigrate'
> >
> > If the user issues a 'stop' between the two we end up with invalid
> > state transitions.
> > Add the transitions as valid.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
>
> To answer Peter question:
>
> int vm_stop(RunState state)
> {
> .... we don't care about this case ....
>
> return do_vm_stop(state);
> }
>
> static int do_vm_stop(RunState state)
> {
> int ret = 0;
>
> if (runstate_is_running()) {
> cpu_disable_ticks();
> pause_all_vcpus();
> runstate_set(state);
> vm_state_notify(0, state);
> qapi_event_send_stop(&error_abort);
> }
>
> bdrv_drain_all();
> replay_disable_events();
> ret = bdrv_flush_all();
>
> return ret;
> }
>
>
> int runstate_is_running(void)
> {
> return runstate_check(RUN_STATE_RUNNING);
> }
>
>
> So, "stop" only changes states when we are in RUNNING state.
> The idea was that after migration, the only valid command (as in the
> film "it should do something")is "run".
Thanks for the details. :)
--
Peter Xu
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH f0r 2.11] runstate/migrate: Two more transitions
2017-08-04 17:50 [Qemu-devel] [PATCH f0r 2.11] runstate/migrate: Two more transitions Dr. David Alan Gilbert (git)
2017-08-07 7:21 ` Peter Xu
2017-08-08 7:02 ` Juan Quintela
@ 2017-09-06 13:40 ` Dr. David Alan Gilbert
2017-09-09 13:57 ` Paolo Bonzini
2 siblings, 1 reply; 8+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-06 13:40 UTC (permalink / raw)
To: qemu-devel, quintela, peterx, lvivier, pbonzini
* Dr. David Alan Gilbert (git) (dgilbert@redhat.com) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> There's a race if someone does a 'stop' near the end of migrate;
> the migration process goes through two runstates:
> 'finish migrate'
> 'postmigrate'
>
> If the user issues a 'stop' between the two we end up with invalid
> state transitions.
> Add the transitions as valid.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Queued
> ---
> vl.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/vl.c b/vl.c
> index 99fcfa0442..bacb03f49d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -621,6 +621,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>
> { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
> { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
> + { RUN_STATE_PAUSED, RUN_STATE_POSTMIGRATE },
> { RUN_STATE_PAUSED, RUN_STATE_PRELAUNCH },
> { RUN_STATE_PAUSED, RUN_STATE_COLO},
>
> @@ -633,6 +634,7 @@ static const RunStateTransition runstate_transitions_def[] = {
> { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
>
> { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
> + { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PAUSED },
> { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
> { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PRELAUNCH },
> { RUN_STATE_FINISH_MIGRATE, RUN_STATE_COLO},
> --
> 2.13.3
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH f0r 2.11] runstate/migrate: Two more transitions
2017-09-06 13:40 ` Dr. David Alan Gilbert
@ 2017-09-09 13:57 ` Paolo Bonzini
0 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2017-09-09 13:57 UTC (permalink / raw)
To: Dr. David Alan Gilbert, qemu-devel, quintela, peterx, lvivier
On 06/09/2017 15:40, Dr. David Alan Gilbert wrote:
>> There's a race if someone does a 'stop' near the end of migrate;
>> the migration process goes through two runstates:
>> 'finish migrate'
>> 'postmigrate'
>>
>> If the user issues a 'stop' between the two we end up with invalid
>> state transitions.
>> Add the transitions as valid.
>>
>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Queued
>
>> ---
>> vl.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/vl.c b/vl.c
>> index 99fcfa0442..bacb03f49d 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -621,6 +621,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>>
>> { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
>> { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
>> + { RUN_STATE_PAUSED, RUN_STATE_POSTMIGRATE },
There is:
if (s->state == MIGRATION_STATUS_COMPLETED) {
...
runstate_set(RUN_STATE_POSTMIGRATE);
} else {
...
if (old_vm_running && !entered_postcopy) {
vm_start();
} else {
if (runstate_check(RUN_STATE_FINISH_MIGRATE)) {
runstate_set(RUN_STATE_POSTMIGRATE);
}
}
}
Maybe the runstate_check should be in the "then" branch too, instead
of adding this transition?
Paolo
>> { RUN_STATE_PAUSED, RUN_STATE_PRELAUNCH },
>> { RUN_STATE_PAUSED, RUN_STATE_COLO},
>>
>> @@ -633,6 +634,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>> { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
>>
>> { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
>> + { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PAUSED },
>> { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
>> { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PRELAUNCH },
>> { RUN_STATE_FINISH_MIGRATE, RUN_STATE_COLO},
>> --
>> 2.13.3
>>
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-09-09 13:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-04 17:50 [Qemu-devel] [PATCH f0r 2.11] runstate/migrate: Two more transitions Dr. David Alan Gilbert (git)
2017-08-07 7:21 ` Peter Xu
2017-08-07 12:25 ` Dr. David Alan Gilbert
2017-08-08 1:59 ` Peter Xu
2017-08-08 7:02 ` Juan Quintela
2017-08-08 7:13 ` Peter Xu
2017-09-06 13:40 ` Dr. David Alan Gilbert
2017-09-09 13:57 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.