Message ID | 20120328054012.34135.77090.stgit@amd-6168-8-1.englab.nay.redhat.com |
---|---|
State | New |
Headers | show |
Il 28/03/2012 07:40, 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. A global variable > need_announce were introduced to record the pending announcement, and > vm_start() would send gratuitous packet depends on this value. > > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > migration.c | 2 +- > migration.h | 2 ++ > vl.c | 5 +++++ > 3 files changed, 8 insertions(+), 1 deletions(-) > > diff --git a/migration.c b/migration.c > index 00fa1e3..861cce9 100644 > --- a/migration.c > +++ b/migration.c > @@ -88,7 +88,7 @@ void process_incoming_migration(QEMUFile *f) > fprintf(stderr, "load of migration failed\n"); > exit(0); > } > - qemu_announce_self(); > + need_announce = true; > DPRINTF("successfully loaded vm state\n"); > > /* Make sure all file formats flush their mutable metadata */ > diff --git a/migration.h b/migration.h > index 372b066..0a31463 100644 > --- a/migration.h > +++ b/migration.h > @@ -95,4 +95,6 @@ void migrate_add_blocker(Error *reason); > */ > void migrate_del_blocker(Error *reason); > > +extern bool need_announce; > + > #endif > diff --git a/vl.c b/vl.c > index 65f11f2..05ebf57 100644 > --- a/vl.c > +++ b/vl.c > @@ -231,6 +231,7 @@ int boot_menu; > uint8_t *boot_splash_filedata; > int boot_splash_filedata_size; > uint8_t qemu_extra_params_fw[2]; > +bool need_announce = false; > > typedef struct FWBootEntry FWBootEntry; > > @@ -1266,6 +1267,10 @@ void vm_start(void) > vm_state_notify(1, RUN_STATE_RUNNING); > resume_all_vcpus(); > monitor_protocol_event(QEVENT_RESUME, NULL); > + if (need_announce) { > + need_announce = false; > + qemu_announce_self(); > + } > } > } > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Paolo
On 03/28/2012 07:40 AM, Jason Wang wrote: > qemu_announce_self() were moved to vm_start(). This is because we may > want to let guest to send the gratuitous packets. A global variable > need_announce were introduced to record the pending announcement, and > vm_start() would send gratuitous packet depends on this value. > > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > migration.c | 2 +- > migration.h | 2 ++ > vl.c | 5 +++++ > 3 files changed, 8 insertions(+), 1 deletions(-) > > diff --git a/migration.c b/migration.c > index 00fa1e3..861cce9 100644 > --- a/migration.c > +++ b/migration.c > @@ -88,7 +88,7 @@ void process_incoming_migration(QEMUFile *f) > fprintf(stderr, "load of migration failed\n"); > exit(0); > } > - qemu_announce_self(); > + need_announce = true; > DPRINTF("successfully loaded vm state\n"); > > /* Make sure all file formats flush their mutable metadata */ > diff --git a/migration.h b/migration.h > index 372b066..0a31463 100644 > --- a/migration.h > +++ b/migration.h > @@ -95,4 +95,6 @@ void migrate_add_blocker(Error *reason); > */ > void migrate_del_blocker(Error *reason); > > +extern bool need_announce; > + Hi Jason, I don't like this external flag. As this is only related to migration I think we can add a new state RUN_STATE_MIG_PRELAUNCH. In vm_start call qemu_announce_self only if the state was RUN_STATE_MIG_PRELAUNCH. This will we useful if we will need to do something else when resuming a migrated guest. Regards, Orit > #endif > diff --git a/vl.c b/vl.c > index 65f11f2..05ebf57 100644 > --- a/vl.c > +++ b/vl.c > @@ -231,6 +231,7 @@ int boot_menu; > uint8_t *boot_splash_filedata; > int boot_splash_filedata_size; > uint8_t qemu_extra_params_fw[2]; > +bool need_announce = false; > > typedef struct FWBootEntry FWBootEntry; > > @@ -1266,6 +1267,10 @@ void vm_start(void) > vm_state_notify(1, RUN_STATE_RUNNING); > resume_all_vcpus(); > monitor_protocol_event(QEVENT_RESUME, NULL); > + if (need_announce) { > + need_announce = false; > + qemu_announce_self(); > + } > } > } > > >
On 05/02/2012 03:49 PM, Orit Wasserman wrote: > On 03/28/2012 07:40 AM, Jason Wang wrote: >> qemu_announce_self() were moved to vm_start(). This is because we may >> want to let guest to send the gratuitous packets. A global variable >> need_announce were introduced to record the pending announcement, and >> vm_start() would send gratuitous packet depends on this value. >> >> Signed-off-by: Jason Wang<jasowang@redhat.com> >> --- >> migration.c | 2 +- >> migration.h | 2 ++ >> vl.c | 5 +++++ >> 3 files changed, 8 insertions(+), 1 deletions(-) >> >> diff --git a/migration.c b/migration.c >> index 00fa1e3..861cce9 100644 >> --- a/migration.c >> +++ b/migration.c >> @@ -88,7 +88,7 @@ void process_incoming_migration(QEMUFile *f) >> fprintf(stderr, "load of migration failed\n"); >> exit(0); >> } >> - qemu_announce_self(); >> + need_announce = true; >> DPRINTF("successfully loaded vm state\n"); >> >> /* Make sure all file formats flush their mutable metadata */ >> diff --git a/migration.h b/migration.h >> index 372b066..0a31463 100644 >> --- a/migration.h >> +++ b/migration.h >> @@ -95,4 +95,6 @@ void migrate_add_blocker(Error *reason); >> */ >> void migrate_del_blocker(Error *reason); >> >> +extern bool need_announce; >> + > Hi Jason, > I don't like this external flag. > As this is only related to migration I think we can add a new state RUN_STATE_MIG_PRELAUNCH. > In vm_start call qemu_announce_self only if the state was RUN_STATE_MIG_PRELAUNCH. > This will we useful if we will need to do something else when resuming a migrated guest. > > Regards, > Orit Hi Orit: No problem, the only reason of using global variable is simplicity, we thought to add one new run state in the past. I would add one like what you suggested. Thanks >> #endif >> diff --git a/vl.c b/vl.c >> index 65f11f2..05ebf57 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -231,6 +231,7 @@ int boot_menu; >> uint8_t *boot_splash_filedata; >> int boot_splash_filedata_size; >> uint8_t qemu_extra_params_fw[2]; >> +bool need_announce = false; >> >> typedef struct FWBootEntry FWBootEntry; >> >> @@ -1266,6 +1267,10 @@ void vm_start(void) >> vm_state_notify(1, RUN_STATE_RUNNING); >> resume_all_vcpus(); >> monitor_protocol_event(QEVENT_RESUME, NULL); >> + if (need_announce) { >> + need_announce = false; >> + qemu_announce_self(); >> + } >> } >> } >> >> >> >
Il 02/05/2012 09:59, Jason Wang ha scritto: > I don't like this external flag. As this is only related to migration > I think we can add a new state RUN_STATE_MIG_PRELAUNCH. In vm_start > call qemu_announce_self only if the state was > RUN_STATE_MIG_PRELAUNCH. This will we useful if we will need to do > something else when resuming a migrated guest. I don't think this global is replacing a runstate. It is simply saying that some work has been delayed to the next time the CPU runs. Perhaps we can instead use a runstate-change notifier and keep the global hidden to vl.c. Paolo
diff --git a/migration.c b/migration.c index 00fa1e3..861cce9 100644 --- a/migration.c +++ b/migration.c @@ -88,7 +88,7 @@ void process_incoming_migration(QEMUFile *f) fprintf(stderr, "load of migration failed\n"); exit(0); } - qemu_announce_self(); + need_announce = true; DPRINTF("successfully loaded vm state\n"); /* Make sure all file formats flush their mutable metadata */ diff --git a/migration.h b/migration.h index 372b066..0a31463 100644 --- a/migration.h +++ b/migration.h @@ -95,4 +95,6 @@ void migrate_add_blocker(Error *reason); */ void migrate_del_blocker(Error *reason); +extern bool need_announce; + #endif diff --git a/vl.c b/vl.c index 65f11f2..05ebf57 100644 --- a/vl.c +++ b/vl.c @@ -231,6 +231,7 @@ int boot_menu; uint8_t *boot_splash_filedata; int boot_splash_filedata_size; uint8_t qemu_extra_params_fw[2]; +bool need_announce = false; typedef struct FWBootEntry FWBootEntry; @@ -1266,6 +1267,10 @@ void vm_start(void) vm_state_notify(1, RUN_STATE_RUNNING); resume_all_vcpus(); monitor_protocol_event(QEVENT_RESUME, NULL); + if (need_announce) { + need_announce = false; + 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. A global variable need_announce were introduced to record the pending announcement, and vm_start() would send gratuitous packet depends on this value. Signed-off-by: Jason Wang <jasowang@redhat.com> --- migration.c | 2 +- migration.h | 2 ++ vl.c | 5 +++++ 3 files changed, 8 insertions(+), 1 deletions(-)