| Message ID | 1436533106-17011-3-git-send-email-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
* Juan Quintela (quintela@redhat.com) wrote: > We use global state in both savevm & migration. The easiest way is to > put the setup in a single place. > > Signed-off-by: Juan Quintela <quintela@redhat.com> I don't think this works; I think pre-save is called after the migration code has changed the runstate, so you end up saving the wrong runstate, and that explains the error Christian sees. Dave > --- > migration/migration.c | 30 ++++++++++++------------------ > 1 file changed, 12 insertions(+), 18 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index ba82ff6..d1421fe 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -110,17 +110,6 @@ typedef struct { > > static GlobalState global_state; > > -static int global_state_store(void) > -{ > - if (!runstate_store((char *)global_state.runstate, > - sizeof(global_state.runstate))) { > - error_report("runstate name too big: %s", global_state.runstate); > - trace_migrate_state_too_big(); > - return -EINVAL; > - } > - return 0; > -} > - > static bool global_state_received(void) > { > return global_state.received; > @@ -187,6 +176,14 @@ static void global_state_pre_save(void *opaque) > GlobalState *s = opaque; > > trace_migrate_global_state_pre_save((char *)s->runstate); > + > + if (!runstate_store((char *)global_state.runstate, > + sizeof(global_state.runstate))) { > + error_report("runstate name too big: %s", global_state.runstate); > + trace_migrate_state_too_big(); > + exit(EXIT_FAILURE); > + } > + > s->size = strlen((char *)s->runstate) + 1; > } > > @@ -940,13 +937,10 @@ static void *migration_thread(void *opaque) > qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER); > old_vm_running = runstate_is_running(); > > - ret = global_state_store(); > - if (!ret) { > - ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); > - if (ret >= 0) { > - qemu_file_set_rate_limit(s->file, INT64_MAX); > - qemu_savevm_state_complete(s->file); > - } > + ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); > + if (ret >= 0) { > + qemu_file_set_rate_limit(s->file, INT64_MAX); > + qemu_savevm_state_complete(s->file); > } > qemu_mutex_unlock_iothread(); > > -- > 2.4.3 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Juan Quintela (quintela@redhat.com) wrote: >> We use global state in both savevm & migration. The easiest way is to >> put the setup in a single place. >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> > > I don't think this works; I think pre-save is called after the migration > code has changed the runstate, so you end up saving the wrong runstate, > and that explains the error Christian sees. Grrrr, it works for savevm. Just to make clear how things went completely wrong: I add this code to global_state_pre_save with a guard of: if (strcmp(global_State_runstate, "") != 0) { } And it worked for savevm. So, if it works for savevm, it will also work for migration, right? Code paths are almost identical. And have the code in a single place. It appears that answer was not. Sorry, Juan. Adding back to both places. > > Dave > >> --- >> migration/migration.c | 30 ++++++++++++------------------ >> 1 file changed, 12 insertions(+), 18 deletions(-) >> >> diff --git a/migration/migration.c b/migration/migration.c >> index ba82ff6..d1421fe 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -110,17 +110,6 @@ typedef struct { >> >> static GlobalState global_state; >> >> -static int global_state_store(void) >> -{ >> - if (!runstate_store((char *)global_state.runstate, >> - sizeof(global_state.runstate))) { >> - error_report("runstate name too big: %s", global_state.runstate); >> - trace_migrate_state_too_big(); >> - return -EINVAL; >> - } >> - return 0; >> -} >> - >> static bool global_state_received(void) >> { >> return global_state.received; >> @@ -187,6 +176,14 @@ static void global_state_pre_save(void *opaque) >> GlobalState *s = opaque; >> >> trace_migrate_global_state_pre_save((char *)s->runstate); >> + >> + if (!runstate_store((char *)global_state.runstate, >> + sizeof(global_state.runstate))) { >> + error_report("runstate name too big: %s", global_state.runstate); >> + trace_migrate_state_too_big(); >> + exit(EXIT_FAILURE); >> + } >> + >> s->size = strlen((char *)s->runstate) + 1; >> } >> >> @@ -940,13 +937,10 @@ static void *migration_thread(void *opaque) >> qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER); >> old_vm_running = runstate_is_running(); >> >> - ret = global_state_store(); >> - if (!ret) { >> - ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); >> - if (ret >= 0) { >> - qemu_file_set_rate_limit(s->file, INT64_MAX); >> - qemu_savevm_state_complete(s->file); >> - } >> + ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); >> + if (ret >= 0) { >> + qemu_file_set_rate_limit(s->file, INT64_MAX); >> + qemu_savevm_state_complete(s->file); >> } >> qemu_mutex_unlock_iothread(); >> >> -- >> 2.4.3 >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/migration/migration.c b/migration/migration.c index ba82ff6..d1421fe 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -110,17 +110,6 @@ typedef struct { static GlobalState global_state; -static int global_state_store(void) -{ - if (!runstate_store((char *)global_state.runstate, - sizeof(global_state.runstate))) { - error_report("runstate name too big: %s", global_state.runstate); - trace_migrate_state_too_big(); - return -EINVAL; - } - return 0; -} - static bool global_state_received(void) { return global_state.received; @@ -187,6 +176,14 @@ static void global_state_pre_save(void *opaque) GlobalState *s = opaque; trace_migrate_global_state_pre_save((char *)s->runstate); + + if (!runstate_store((char *)global_state.runstate, + sizeof(global_state.runstate))) { + error_report("runstate name too big: %s", global_state.runstate); + trace_migrate_state_too_big(); + exit(EXIT_FAILURE); + } + s->size = strlen((char *)s->runstate) + 1; } @@ -940,13 +937,10 @@ static void *migration_thread(void *opaque) qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER); old_vm_running = runstate_is_running(); - ret = global_state_store(); - if (!ret) { - ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); - if (ret >= 0) { - qemu_file_set_rate_limit(s->file, INT64_MAX); - qemu_savevm_state_complete(s->file); - } + ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); + if (ret >= 0) { + qemu_file_set_rate_limit(s->file, INT64_MAX); + qemu_savevm_state_complete(s->file); } qemu_mutex_unlock_iothread();
We use global state in both savevm & migration. The easiest way is to put the setup in a single place. Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/migration.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-)