| Message ID | 20120316085453.4947.23068.stgit@amd-6168-8-1.englab.nay.redhat.com |
|---|---|
| State | New |
| Headers | show |
Il 16/03/2012 09:54, Jason Wang ha scritto: > qemu_announce_self() were moved to vm_start(). This is because we may > want to let guest to send the gratuitous packets. After this change, > we need to check the previous run state (RUN_STATE_INMIGRATE) to > decide whether an announcement is needed. > > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > migration.c | 1 - > vl.c | 4 ++++ > 2 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/migration.c b/migration.c > index 00fa1e3..1ce6b5c 100644 > --- a/migration.c > +++ b/migration.c > @@ -88,7 +88,6 @@ void process_incoming_migration(QEMUFile *f) > fprintf(stderr, "load of migration failed\n"); > exit(0); > } > - qemu_announce_self(); > DPRINTF("successfully loaded vm state\n"); > > /* Make sure all file formats flush their mutable metadata */ > diff --git a/vl.c b/vl.c > index 65f11f2..4742b1b 100644 > --- a/vl.c > +++ b/vl.c > @@ -1261,11 +1261,15 @@ void vm_state_notify(int running, RunState state) > void vm_start(void) > { > if (!runstate_is_running()) { > + RunState prev_run_state = current_run_state; > cpu_enable_ticks(); > runstate_set(RUN_STATE_RUNNING); > vm_state_notify(1, RUN_STATE_RUNNING); > resume_all_vcpus(); > monitor_protocol_event(QEVENT_RESUME, NULL); > + if (prev_run_state == RUN_STATE_INMIGRATE) { > + qemu_announce_self(); > + } > } > } > > I tihnk this won't work with -S, did you test it? Perhaps it's possible simply to change if (autostart) { vm_start(); } else { runstate_set(RUN_STATE_PRELAUNCH); } to remain in INMIGRATE state: if (autostart) { vm_start(); } Otherwise looks good. Paolo
On 03/16/2012 05:43 PM, Paolo Bonzini wrote: > Il 16/03/2012 09:54, Jason Wang ha scritto: >> qemu_announce_self() were moved to vm_start(). This is because we may >> want to let guest to send the gratuitous packets. After this change, >> we need to check the previous run state (RUN_STATE_INMIGRATE) to >> decide whether an announcement is needed. >> >> Signed-off-by: Jason Wang<jasowang@redhat.com> >> --- >> migration.c | 1 - >> vl.c | 4 ++++ >> 2 files changed, 4 insertions(+), 1 deletions(-) >> >> diff --git a/migration.c b/migration.c >> index 00fa1e3..1ce6b5c 100644 >> --- a/migration.c >> +++ b/migration.c >> @@ -88,7 +88,6 @@ void process_incoming_migration(QEMUFile *f) >> fprintf(stderr, "load of migration failed\n"); >> exit(0); >> } >> - qemu_announce_self(); >> DPRINTF("successfully loaded vm state\n"); >> >> /* Make sure all file formats flush their mutable metadata */ >> diff --git a/vl.c b/vl.c >> index 65f11f2..4742b1b 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -1261,11 +1261,15 @@ void vm_state_notify(int running, RunState state) >> void vm_start(void) >> { >> if (!runstate_is_running()) { >> + RunState prev_run_state = current_run_state; >> cpu_enable_ticks(); >> runstate_set(RUN_STATE_RUNNING); >> vm_state_notify(1, RUN_STATE_RUNNING); >> resume_all_vcpus(); >> monitor_protocol_event(QEVENT_RESUME, NULL); >> + if (prev_run_state == RUN_STATE_INMIGRATE) { >> + qemu_announce_self(); >> + } >> } >> } >> >> > I tihnk this won't work with -S, did you test it? Perhaps it's possible > simply to change Yes, it does not work. > > if (autostart) { > vm_start(); > } else { > runstate_set(RUN_STATE_PRELAUNCH); > } > > to remain in INMIGRATE state: > > if (autostart) { > vm_start(); > } > > Otherwise looks good. > > Paolo The problem with staying in the INMIGRATE is that we can not figure out when the migration is completed when using '-S', so this kind of transition were forbidden by qmp_cont(). Looks like we need a new state such as RUN_STATE_MIGRATE_PRELAUNCH?
Il 16/03/2012 11:13, Jason Wang ha scritto: > The problem with staying in the INMIGRATE is that we can not figure out > when the migration is completed when using '-S', so this kind of > transition were forbidden by qmp_cont(). > > Looks like we need a new state such as RUN_STATE_MIGRATE_PRELAUNCH? Or just a global need_announce instead of looking at the runstate. Paolo
On 03/16/2012 06:45 PM, Paolo Bonzini wrote: > Il 16/03/2012 11:13, Jason Wang ha scritto: >> > The problem with staying in the INMIGRATE is that we can not figure out >> > when the migration is completed when using '-S', so this kind of >> > transition were forbidden by qmp_cont(). >> > >> > Looks like we need a new state such as RUN_STATE_MIGRATE_PRELAUNCH? > Or just a global need_announce instead of looking at the runstate. > > Paolo > Then I think it's better for us introduce a parameter for vm_start() like what we've done in V4.
Il 16/03/2012 16:23, Jason Wang ha scritto: >>> >> Or just a global need_announce instead of looking at the runstate. >> >> Paolo >> > Then I think it's better for us introduce a parameter for vm_start() > like what we've done in V4. But that didn't work because you ended up changing the "cont" semantics. There are two possibilities. 1) Changing those is okay, in which case you only need to check more runstates; 2) Changing those is not okay, in which case you need something like this in qemu_announce_self() void qemu_announce_self() { if (!runstate_is_running()) { need_announce = true; return; } need_announce = false; ... } and then you just check need_announce in vm_start. Nothing to change in all the invocations of vm_start, you just mark that you need to do work later. Paolo
On 03/17/2012 12:31 AM, Paolo Bonzini wrote: > Il 16/03/2012 16:23, Jason Wang ha scritto: >>> Or just a global need_announce instead of looking at the runstate. >>> >>> Paolo >>> >> Then I think it's better for us introduce a parameter for vm_start() >> like what we've done in V4. > But that didn't work because you ended up changing the "cont" semantics. > > There are two possibilities. > > 1) Changing those is okay, in which case you only need to check more > runstates; > > 2) Changing those is not okay, in which case you need something like > this in qemu_announce_self() > > void qemu_announce_self() > { > if (!runstate_is_running()) { > need_announce = true; > return; > } > > need_announce = false; > ... > } > > and then you just check need_announce in vm_start. Nothing to change in > all the invocations of vm_start, you just mark that you need to do work > later. > > Paolo Right, I would replace the qemu_announce_self() with a "need_announce = true" in process_incoming_migration() and check it later in vm_start(). Thanks
diff --git a/migration.c b/migration.c index 00fa1e3..1ce6b5c 100644 --- a/migration.c +++ b/migration.c @@ -88,7 +88,6 @@ void process_incoming_migration(QEMUFile *f) fprintf(stderr, "load of migration failed\n"); exit(0); } - qemu_announce_self(); DPRINTF("successfully loaded vm state\n"); /* Make sure all file formats flush their mutable metadata */ diff --git a/vl.c b/vl.c index 65f11f2..4742b1b 100644 --- a/vl.c +++ b/vl.c @@ -1261,11 +1261,15 @@ void vm_state_notify(int running, RunState state) void vm_start(void) { if (!runstate_is_running()) { + RunState prev_run_state = current_run_state; cpu_enable_ticks(); runstate_set(RUN_STATE_RUNNING); vm_state_notify(1, RUN_STATE_RUNNING); resume_all_vcpus(); monitor_protocol_event(QEVENT_RESUME, NULL); + if (prev_run_state == RUN_STATE_INMIGRATE) { + qemu_announce_self(); + } } }
qemu_announce_self() were moved to vm_start(). This is because we may want to let guest to send the gratuitous packets. After this change, we need to check the previous run state (RUN_STATE_INMIGRATE) to decide whether an announcement is needed. Signed-off-by: Jason Wang <jasowang@redhat.com> --- migration.c | 1 - vl.c | 4 ++++ 2 files changed, 4 insertions(+), 1 deletions(-)