| Message ID | 20120313085622.10958.46277.stgit@amd-6168-8-1.englab.nay.redhat.com |
|---|---|
| State | New |
| Headers | show |
Il 13/03/2012 09:56, Jason Wang ha scritto: > This patch moves qemu_announce_self() to vm_start() and add a new parameters to > control whether sending gratuitous packet is needed. There are several reasons > to do this: > > - Gratuitous packet is also needed when we resume a stopped vm or successfully > load a state. > - Sending gratuitous packets may be done through co-operation with guest, so > this work should be done after vm is started. > > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > gdbstub.c | 2 +- > migration.c | 5 ++--- > monitor.c | 2 +- > qmp.c | 2 +- > savevm.c | 2 +- > sysemu.h | 2 +- > vl.c | 7 +++++-- > 7 files changed, 12 insertions(+), 10 deletions(-) > > diff --git a/gdbstub.c b/gdbstub.c > index ef95ac2..f4d22e5 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -371,7 +371,7 @@ static inline void gdb_continue(GDBState *s) > #ifdef CONFIG_USER_ONLY > s->running_state = 1; > #else > - vm_start(); > + vm_start(false); > #endif Here we're switching from RUNSTATE_DEBUG. > } > > diff --git a/migration.c b/migration.c > index 00fa1e3..40332d2 100644 > --- a/migration.c > +++ b/migration.c > @@ -88,14 +88,13 @@ 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 */ > bdrv_invalidate_cache_all(); > > if (autostart) { > - vm_start(); > + vm_start(true); Here from RUN_STATE_INMIGRATE. > } else { > runstate_set(RUN_STATE_PRELAUNCH); > } > @@ -274,7 +273,7 @@ static void migrate_fd_put_ready(void *opaque) > } > if (s->state != MIG_STATE_COMPLETED) { > if (old_vm_running) { > - vm_start(); > + vm_start(false); Here from RUN_STATE_FINISH_MIGRATE. > } > } > } > diff --git a/monitor.c b/monitor.c > index cbdfbad..0b63c11 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -2260,7 +2260,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) > vm_stop(RUN_STATE_RESTORE_VM); > > if (load_vmstate(name) == 0 && saved_vm_running) { > - vm_start(); > + vm_start(true); Here from RUN_STATE_RESTORE_VM. > } > } > > diff --git a/qmp.c b/qmp.c > index a182b51..252c842 100644 > --- a/qmp.c > +++ b/qmp.c > @@ -160,7 +160,7 @@ void qmp_cont(Error **errp) > return; > } > > - vm_start(); > + vm_start(true); Here from RUN_STATE_PAUSED or RUN_STATE_PRELAUNCH. This introduces a difference here with "qemu -S" + cont, and "qemu". The former will send a gratuitous ARP, the latter won't. Is this desired/harmless/...? > } > > void qmp_system_wakeup(Error **errp) > diff --git a/savevm.c b/savevm.c > index 5b59826..82b9d3a 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -2107,7 +2107,7 @@ void do_savevm(Monitor *mon, const QDict *qdict) > > the_end: > if (saved_vm_running) > - vm_start(); > + vm_start(false); > } This is RUN_STATE_SAVE_VM. > > int load_vmstate(const char *name) > diff --git a/sysemu.h b/sysemu.h > index 98118cc..787edd4 100644 > --- a/sysemu.h > +++ b/sysemu.h > @@ -34,7 +34,7 @@ void vm_state_notify(int running, RunState state); > #define VMRESET_SILENT false > #define VMRESET_REPORT true > > -void vm_start(void); > +void vm_start(bool announce); > void vm_stop(RunState state); > void vm_stop_force_state(RunState state); > > diff --git a/vl.c b/vl.c > index 65f11f2..4e04f82 100644 > --- a/vl.c > +++ b/vl.c > @@ -1258,7 +1258,7 @@ void vm_state_notify(int running, RunState state) > } > } > > -void vm_start(void) > +void vm_start(bool announce) > { > if (!runstate_is_running()) { > cpu_enable_ticks(); > @@ -1266,6 +1266,9 @@ void vm_start(void) > vm_state_notify(1, RUN_STATE_RUNNING); > resume_all_vcpus(); > monitor_protocol_event(QEVENT_RESUME, NULL); > + if (announce) { > + qemu_announce_self(); > + } > } > } > > @@ -3619,7 +3622,7 @@ int main(int argc, char **argv, char **envp) > exit(ret); > } > } else if (autostart) { > - vm_start(); > + vm_start(false); > } This is RUN_STATE_PRELAUNCH. To some up, it seems like whether to send an announcement depends on the previous runstate: it should be sent only for RUN_STATE_INMIGRATE, RUN_STATE_RESTORE_VM and (new with your patch) RUN_STATE_PAUSED. So perhaps the new argument to vm_start is not needed. Paolo > > os_setup_post(); > > >
On Tue, Mar 13, 2012 at 04:56:22PM +0800, Jason Wang wrote: > This patch moves qemu_announce_self() to vm_start() and add a new parameters to > control whether sending gratuitous packet is needed. There are several reasons > to do this: > > - Gratuitous packet is also needed when we resume a stopped vm or successfully > load a state. Why is it needed when we continue a stopped vm? > - Sending gratuitous packets may be done through co-operation with guest, so > this work should be done after vm is started. > > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > gdbstub.c | 2 +- > migration.c | 5 ++--- > monitor.c | 2 +- > qmp.c | 2 +- > savevm.c | 2 +- > sysemu.h | 2 +- > vl.c | 7 +++++-- > 7 files changed, 12 insertions(+), 10 deletions(-) > > diff --git a/gdbstub.c b/gdbstub.c > index ef95ac2..f4d22e5 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -371,7 +371,7 @@ static inline void gdb_continue(GDBState *s) > #ifdef CONFIG_USER_ONLY > s->running_state = 1; > #else > - vm_start(); > + vm_start(false); > #endif > } > > diff --git a/migration.c b/migration.c > index 00fa1e3..40332d2 100644 > --- a/migration.c > +++ b/migration.c > @@ -88,14 +88,13 @@ 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 */ > bdrv_invalidate_cache_all(); > > if (autostart) { > - vm_start(); > + vm_start(true); > } else { > runstate_set(RUN_STATE_PRELAUNCH); > } > @@ -274,7 +273,7 @@ static void migrate_fd_put_ready(void *opaque) > } > if (s->state != MIG_STATE_COMPLETED) { > if (old_vm_running) { > - vm_start(); > + vm_start(false); > } > } > } > diff --git a/monitor.c b/monitor.c > index cbdfbad..0b63c11 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -2260,7 +2260,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) > vm_stop(RUN_STATE_RESTORE_VM); > > if (load_vmstate(name) == 0 && saved_vm_running) { > - vm_start(); > + vm_start(true); > } > } > > diff --git a/qmp.c b/qmp.c > index a182b51..252c842 100644 > --- a/qmp.c > +++ b/qmp.c > @@ -160,7 +160,7 @@ void qmp_cont(Error **errp) > return; > } > > - vm_start(); > + vm_start(true); > } > > void qmp_system_wakeup(Error **errp) > diff --git a/savevm.c b/savevm.c > index 5b59826..82b9d3a 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -2107,7 +2107,7 @@ void do_savevm(Monitor *mon, const QDict *qdict) > > the_end: > if (saved_vm_running) > - vm_start(); > + vm_start(false); > } > > int load_vmstate(const char *name) > diff --git a/sysemu.h b/sysemu.h > index 98118cc..787edd4 100644 > --- a/sysemu.h > +++ b/sysemu.h > @@ -34,7 +34,7 @@ void vm_state_notify(int running, RunState state); > #define VMRESET_SILENT false > #define VMRESET_REPORT true > > -void vm_start(void); > +void vm_start(bool announce); > void vm_stop(RunState state); > void vm_stop_force_state(RunState state); > > diff --git a/vl.c b/vl.c > index 65f11f2..4e04f82 100644 > --- a/vl.c > +++ b/vl.c > @@ -1258,7 +1258,7 @@ void vm_state_notify(int running, RunState state) > } > } > > -void vm_start(void) > +void vm_start(bool announce) > { > if (!runstate_is_running()) { > cpu_enable_ticks(); > @@ -1266,6 +1266,9 @@ void vm_start(void) > vm_state_notify(1, RUN_STATE_RUNNING); > resume_all_vcpus(); > monitor_protocol_event(QEVENT_RESUME, NULL); > + if (announce) { > + qemu_announce_self(); > + } > } > } > > @@ -3619,7 +3622,7 @@ int main(int argc, char **argv, char **envp) > exit(ret); > } > } else if (autostart) { > - vm_start(); > + vm_start(false); > } > > os_setup_post();
On 03/13/2012 05:18 PM, Paolo Bonzini wrote: > Il 13/03/2012 09:56, Jason Wang ha scritto: >> This patch moves qemu_announce_self() to vm_start() and add a new parameters to >> control whether sending gratuitous packet is needed. There are several reasons >> to do this: >> >> - Gratuitous packet is also needed when we resume a stopped vm or successfully >> load a state. >> - Sending gratuitous packets may be done through co-operation with guest, so >> this work should be done after vm is started. >> >> Signed-off-by: Jason Wang<jasowang@redhat.com> >> --- >> gdbstub.c | 2 +- >> migration.c | 5 ++--- >> monitor.c | 2 +- >> qmp.c | 2 +- >> savevm.c | 2 +- >> sysemu.h | 2 +- >> vl.c | 7 +++++-- >> 7 files changed, 12 insertions(+), 10 deletions(-) >> >> diff --git a/gdbstub.c b/gdbstub.c >> index ef95ac2..f4d22e5 100644 >> --- a/gdbstub.c >> +++ b/gdbstub.c >> @@ -371,7 +371,7 @@ static inline void gdb_continue(GDBState *s) >> #ifdef CONFIG_USER_ONLY >> s->running_state = 1; >> #else >> - vm_start(); >> + vm_start(false); >> #endif > Here we're switching from RUNSTATE_DEBUG. > >> } >> >> diff --git a/migration.c b/migration.c >> index 00fa1e3..40332d2 100644 >> --- a/migration.c >> +++ b/migration.c >> @@ -88,14 +88,13 @@ 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 */ >> bdrv_invalidate_cache_all(); >> >> if (autostart) { >> - vm_start(); >> + vm_start(true); > Here from RUN_STATE_INMIGRATE. > >> } else { >> runstate_set(RUN_STATE_PRELAUNCH); >> } >> @@ -274,7 +273,7 @@ static void migrate_fd_put_ready(void *opaque) >> } >> if (s->state != MIG_STATE_COMPLETED) { >> if (old_vm_running) { >> - vm_start(); >> + vm_start(false); > Here from RUN_STATE_FINISH_MIGRATE. > >> } >> } >> } >> diff --git a/monitor.c b/monitor.c >> index cbdfbad..0b63c11 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -2260,7 +2260,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) >> vm_stop(RUN_STATE_RESTORE_VM); >> >> if (load_vmstate(name) == 0&& saved_vm_running) { >> - vm_start(); >> + vm_start(true); > Here from RUN_STATE_RESTORE_VM. > >> } >> } >> >> diff --git a/qmp.c b/qmp.c >> index a182b51..252c842 100644 >> --- a/qmp.c >> +++ b/qmp.c >> @@ -160,7 +160,7 @@ void qmp_cont(Error **errp) >> return; >> } >> >> - vm_start(); >> + vm_start(true); > Here from RUN_STATE_PAUSED or RUN_STATE_PRELAUNCH. > > This introduces a difference here with "qemu -S" + cont, and "qemu". > The former will send a gratuitous ARP, the latter won't. Is this > desired/harmless/...? Not desired but harmless I think. And indeed there're two possibilities: - start a fresh vm with -S, and then continue the vm. - migrate a guest with -S option used in destination, and then continue the vm. It's a little hard to differentiate one from another just in vm_stop(). > >> } >> >> void qmp_system_wakeup(Error **errp) >> diff --git a/savevm.c b/savevm.c >> index 5b59826..82b9d3a 100644 >> --- a/savevm.c >> +++ b/savevm.c >> @@ -2107,7 +2107,7 @@ void do_savevm(Monitor *mon, const QDict *qdict) >> >> the_end: >> if (saved_vm_running) >> - vm_start(); >> + vm_start(false); >> } > This is RUN_STATE_SAVE_VM. > >> >> int load_vmstate(const char *name) >> diff --git a/sysemu.h b/sysemu.h >> index 98118cc..787edd4 100644 >> --- a/sysemu.h >> +++ b/sysemu.h >> @@ -34,7 +34,7 @@ void vm_state_notify(int running, RunState state); >> #define VMRESET_SILENT false >> #define VMRESET_REPORT true >> >> -void vm_start(void); >> +void vm_start(bool announce); >> void vm_stop(RunState state); >> void vm_stop_force_state(RunState state); >> >> diff --git a/vl.c b/vl.c >> index 65f11f2..4e04f82 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -1258,7 +1258,7 @@ void vm_state_notify(int running, RunState state) >> } >> } >> >> -void vm_start(void) >> +void vm_start(bool announce) >> { >> if (!runstate_is_running()) { >> cpu_enable_ticks(); >> @@ -1266,6 +1266,9 @@ void vm_start(void) >> vm_state_notify(1, RUN_STATE_RUNNING); >> resume_all_vcpus(); >> monitor_protocol_event(QEVENT_RESUME, NULL); >> + if (announce) { >> + qemu_announce_self(); >> + } >> } >> } >> >> @@ -3619,7 +3622,7 @@ int main(int argc, char **argv, char **envp) >> exit(ret); >> } >> } else if (autostart) { >> - vm_start(); >> + vm_start(false); >> } > This is RUN_STATE_PRELAUNCH. > > To some up, it seems like whether to send an announcement depends on the > previous runstate: it should be sent only for RUN_STATE_INMIGRATE, > RUN_STATE_RESTORE_VM and (new with your patch) RUN_STATE_PAUSED. So > perhaps the new argument to vm_start is not needed. > > Paolo Make sense, thanks. >> >> os_setup_post(); >> >> >>
On 03/13/2012 10:23 PM, Michael S. Tsirkin wrote: > On Tue, Mar 13, 2012 at 04:56:22PM +0800, Jason Wang wrote: >> This patch moves qemu_announce_self() to vm_start() and add a new parameters to >> control whether sending gratuitous packet is needed. There are several reasons >> to do this: >> >> - Gratuitous packet is also needed when we resume a stopped vm or successfully >> load a state. > Why is it needed when we continue a stopped vm? If we stop a vm for a little long time, the mac table entry in the switch would also be expired. So we need to announce again. > >> - Sending gratuitous packets may be done through co-operation with guest, so >> this work should be done after vm is started. >> >> Signed-off-by: Jason Wang<jasowang@redhat.com> >> --- >> gdbstub.c | 2 +- >> migration.c | 5 ++--- >> monitor.c | 2 +- >> qmp.c | 2 +- >> savevm.c | 2 +- >> sysemu.h | 2 +- >> vl.c | 7 +++++-- >> 7 files changed, 12 insertions(+), 10 deletions(-) >> >> diff --git a/gdbstub.c b/gdbstub.c >> index ef95ac2..f4d22e5 100644 >> --- a/gdbstub.c >> +++ b/gdbstub.c >> @@ -371,7 +371,7 @@ static inline void gdb_continue(GDBState *s) >> #ifdef CONFIG_USER_ONLY >> s->running_state = 1; >> #else >> - vm_start(); >> + vm_start(false); >> #endif >> } >> >> diff --git a/migration.c b/migration.c >> index 00fa1e3..40332d2 100644 >> --- a/migration.c >> +++ b/migration.c >> @@ -88,14 +88,13 @@ 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 */ >> bdrv_invalidate_cache_all(); >> >> if (autostart) { >> - vm_start(); >> + vm_start(true); >> } else { >> runstate_set(RUN_STATE_PRELAUNCH); >> } >> @@ -274,7 +273,7 @@ static void migrate_fd_put_ready(void *opaque) >> } >> if (s->state != MIG_STATE_COMPLETED) { >> if (old_vm_running) { >> - vm_start(); >> + vm_start(false); >> } >> } >> } >> diff --git a/monitor.c b/monitor.c >> index cbdfbad..0b63c11 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -2260,7 +2260,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) >> vm_stop(RUN_STATE_RESTORE_VM); >> >> if (load_vmstate(name) == 0&& saved_vm_running) { >> - vm_start(); >> + vm_start(true); >> } >> } >> >> diff --git a/qmp.c b/qmp.c >> index a182b51..252c842 100644 >> --- a/qmp.c >> +++ b/qmp.c >> @@ -160,7 +160,7 @@ void qmp_cont(Error **errp) >> return; >> } >> >> - vm_start(); >> + vm_start(true); >> } >> >> void qmp_system_wakeup(Error **errp) >> diff --git a/savevm.c b/savevm.c >> index 5b59826..82b9d3a 100644 >> --- a/savevm.c >> +++ b/savevm.c >> @@ -2107,7 +2107,7 @@ void do_savevm(Monitor *mon, const QDict *qdict) >> >> the_end: >> if (saved_vm_running) >> - vm_start(); >> + vm_start(false); >> } >> >> int load_vmstate(const char *name) >> diff --git a/sysemu.h b/sysemu.h >> index 98118cc..787edd4 100644 >> --- a/sysemu.h >> +++ b/sysemu.h >> @@ -34,7 +34,7 @@ void vm_state_notify(int running, RunState state); >> #define VMRESET_SILENT false >> #define VMRESET_REPORT true >> >> -void vm_start(void); >> +void vm_start(bool announce); >> void vm_stop(RunState state); >> void vm_stop_force_state(RunState state); >> >> diff --git a/vl.c b/vl.c >> index 65f11f2..4e04f82 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -1258,7 +1258,7 @@ void vm_state_notify(int running, RunState state) >> } >> } >> >> -void vm_start(void) >> +void vm_start(bool announce) >> { >> if (!runstate_is_running()) { >> cpu_enable_ticks(); >> @@ -1266,6 +1266,9 @@ void vm_start(void) >> vm_state_notify(1, RUN_STATE_RUNNING); >> resume_all_vcpus(); >> monitor_protocol_event(QEVENT_RESUME, NULL); >> + if (announce) { >> + qemu_announce_self(); >> + } >> } >> } >> >> @@ -3619,7 +3622,7 @@ int main(int argc, char **argv, char **envp) >> exit(ret); >> } >> } else if (autostart) { >> - vm_start(); >> + vm_start(false); >> } >> >> os_setup_post();
diff --git a/gdbstub.c b/gdbstub.c index ef95ac2..f4d22e5 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -371,7 +371,7 @@ static inline void gdb_continue(GDBState *s) #ifdef CONFIG_USER_ONLY s->running_state = 1; #else - vm_start(); + vm_start(false); #endif } diff --git a/migration.c b/migration.c index 00fa1e3..40332d2 100644 --- a/migration.c +++ b/migration.c @@ -88,14 +88,13 @@ 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 */ bdrv_invalidate_cache_all(); if (autostart) { - vm_start(); + vm_start(true); } else { runstate_set(RUN_STATE_PRELAUNCH); } @@ -274,7 +273,7 @@ static void migrate_fd_put_ready(void *opaque) } if (s->state != MIG_STATE_COMPLETED) { if (old_vm_running) { - vm_start(); + vm_start(false); } } } diff --git a/monitor.c b/monitor.c index cbdfbad..0b63c11 100644 --- a/monitor.c +++ b/monitor.c @@ -2260,7 +2260,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) vm_stop(RUN_STATE_RESTORE_VM); if (load_vmstate(name) == 0 && saved_vm_running) { - vm_start(); + vm_start(true); } } diff --git a/qmp.c b/qmp.c index a182b51..252c842 100644 --- a/qmp.c +++ b/qmp.c @@ -160,7 +160,7 @@ void qmp_cont(Error **errp) return; } - vm_start(); + vm_start(true); } void qmp_system_wakeup(Error **errp) diff --git a/savevm.c b/savevm.c index 5b59826..82b9d3a 100644 --- a/savevm.c +++ b/savevm.c @@ -2107,7 +2107,7 @@ void do_savevm(Monitor *mon, const QDict *qdict) the_end: if (saved_vm_running) - vm_start(); + vm_start(false); } int load_vmstate(const char *name) diff --git a/sysemu.h b/sysemu.h index 98118cc..787edd4 100644 --- a/sysemu.h +++ b/sysemu.h @@ -34,7 +34,7 @@ void vm_state_notify(int running, RunState state); #define VMRESET_SILENT false #define VMRESET_REPORT true -void vm_start(void); +void vm_start(bool announce); void vm_stop(RunState state); void vm_stop_force_state(RunState state); diff --git a/vl.c b/vl.c index 65f11f2..4e04f82 100644 --- a/vl.c +++ b/vl.c @@ -1258,7 +1258,7 @@ void vm_state_notify(int running, RunState state) } } -void vm_start(void) +void vm_start(bool announce) { if (!runstate_is_running()) { cpu_enable_ticks(); @@ -1266,6 +1266,9 @@ void vm_start(void) vm_state_notify(1, RUN_STATE_RUNNING); resume_all_vcpus(); monitor_protocol_event(QEVENT_RESUME, NULL); + if (announce) { + qemu_announce_self(); + } } } @@ -3619,7 +3622,7 @@ int main(int argc, char **argv, char **envp) exit(ret); } } else if (autostart) { - vm_start(); + vm_start(false); } os_setup_post();
This patch moves qemu_announce_self() to vm_start() and add a new parameters to control whether sending gratuitous packet is needed. There are several reasons to do this: - Gratuitous packet is also needed when we resume a stopped vm or successfully load a state. - Sending gratuitous packets may be done through co-operation with guest, so this work should be done after vm is started. Signed-off-by: Jason Wang <jasowang@redhat.com> --- gdbstub.c | 2 +- migration.c | 5 ++--- monitor.c | 2 +- qmp.c | 2 +- savevm.c | 2 +- sysemu.h | 2 +- vl.c | 7 +++++-- 7 files changed, 12 insertions(+), 10 deletions(-)