Re: [Qemu-devel] [PATCH 2/2] migration: store globalstate in pre_safe

[Juan Quintela]

"Dr. David Alan Gilbert" <address@hidden> wrote:
> * Juan Quintela (address@hidden) 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 <address@hidden>
>
> 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 / address@hidden / Manchester, UK