Re: [Qemu-devel] [Qemu-devel [RFC] [WIP] v1] Keeping the Source side alive incase of network failure (Migration recover from network failure)

[haris iqbal]

On Wed, Jun 1, 2016 at 9:45 PM, Dr. David Alan Gilbert
<address@hidden> wrote:
> * Md Haris Iqbal (address@hidden) wrote:
>
> Remember to add a more detailed comment about what the patch is doing.

Sure. I will do that in for the upcoming patches.

> (And possibly split it up a bit more)

Done, I will split it according to files.

>
>> ---
>>  include/migration/migration.h |  1 +
>>  migration/migration.c         | 41 ++++++++++++++++++++++++++++++++++++-----
>>  vl.c                          |  4 ++++
>>  3 files changed, 41 insertions(+), 5 deletions(-)
>
>> diff --git a/include/migration/migration.h b/include/migration/migration.h
>> index ac2c12c..33da695 100644
>> --- a/include/migration/migration.h
>> +++ b/include/migration/migration.h
>> @@ -325,6 +325,7 @@ void global_state_store_running(void);
>>  void flush_page_queue(MigrationState *ms);
>>  int ram_save_queue_pages(MigrationState *ms, const char *rbname,
>>                           ram_addr_t start, ram_addr_t len);
>> +int qemu_migrate_postcopy_outgoing_recovery(MigrationState *ms);
>>
>>  PostcopyState postcopy_state_get(void);
>>  /* Set the state and return the old state */
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 991313a..ee0c2a8 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -539,6 +539,7 @@ static bool migration_is_setup_or_active(int state)
>>      case MIGRATION_STATUS_ACTIVE:
>>      case MIGRATION_STATUS_POSTCOPY_ACTIVE:
>>      case MIGRATION_STATUS_SETUP:
>> +    case MIGRATION_STATUS_POSTCOPY_RECOVERY:
>>          return true;
>>
>>      default:
>> @@ -1634,6 +1635,8 @@ static void *migration_thread(void *opaque)
>>      /* The active state we expect to be in; ACTIVE or POSTCOPY_ACTIVE */
>>      enum MigrationStatus current_active_state = MIGRATION_STATUS_ACTIVE;
>>
>> +    int32_t ret;
>
> The return type of qemu_file_get_error is int not int32_t.

But, isn't signed int typedef'd to int32_t? And signed int is the same as int.

>
>>      rcu_register_thread();
>>
>>      qemu_savevm_state_header(s->to_dst_file);
>> @@ -1700,11 +1703,26 @@ static void *migration_thread(void *opaque)
>>              }
>>          }
>>
>> -        if (qemu_file_get_error(s->to_dst_file)) {
>> -            migrate_set_state(&s->state, current_active_state,
>> -                              MIGRATION_STATUS_FAILED);
>> -            trace_migration_thread_file_err();
>> -            break;
>> +        if ((ret = qemu_file_get_error(s->to_dst_file))) {
>> +            fprintf(stderr, "1 : Error %s %d\n", strerror(-ret), -ret);
>
> Remember to clean those fprintf's out at the end;
Yes, I will do that while preparing the final patch. These are good
for now, because I can see things happening when I run.

> although it can be useful
> to leave some trace_ calls in; so for example modify
> the trace_migrate_thread_file_err to take an error number.

Yes, I will add trace_ calls.

>
>> +            if(ret != -EIO && s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE) 
>> {
>> +                /* Network Failure during postcopy */
>
> That probably needs commenting as to why it's not -EIO - since that's probably
> what people might expect from a network error;  we'll have to keep an eye out
> to see if that's realyl a reliable check.
> (Also qemu normally puts a space after the 'if')

Done.

>
>> +                current_active_state = MIGRATION_STATUS_POSTCOPY_RECOVERY;
>
> That probably needs a migrate_set_state .
>
>> +                runstate_set(RUN_STATE_POSTMIGRATE_RECOVERY);
>> +                fprintf(stderr, "1.1 : Error %s %d\n", strerror(-ret), 
>> -ret);
>> +                ret = qemu_migrate_postcopy_outgoing_recovery(s);
>> +                if(ret < 0) {
>> +                    break;
>> +                }
>> +
>> +            } else {
>> +                migrate_set_state(&s->state, current_active_state,
>> +                                             MIGRATION_STATUS_FAILED);
>> +                fprintf(stderr, "1.2 : Error %s %d\n", strerror(-ret), 
>> -ret);
>> +                trace_migration_thread_file_err();
>> +                break;
>> +            }
>>          }
>>          current_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
>>          if (current_time >= initial_time + BUFFER_DELAY) {
>> @@ -1797,6 +1815,19 @@ void migrate_fd_connect(MigrationState *s)
>>      s->migration_thread_running = true;
>>  }
>>
>> +int qemu_migrate_postcopy_outgoing_recovery(MigrationState* ms)
>> +{
>> +    migrate_set_state(&ms->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
>> +                                  MIGRATION_STATUS_POSTCOPY_RECOVERY);
>> +
>> +    /* Code for network recovery to be added here */
>> +    while(1) {
>> +        fprintf(stderr, "Not letting it fail\n");
>> +        sleep(2);
>> +    }
>> +
>> +}
>> +
>>  PostcopyState  postcopy_state_get(void)
>>  {
>>      return atomic_mb_read(&incoming_postcopy_state);
>> diff --git a/vl.c b/vl.c
>> index 5fd22cb..c237140 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -618,6 +618,10 @@ static const RunStateTransition 
>> runstate_transitions_def[] = {
>>      { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
>>      { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
>>      { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PRELAUNCH },
>> +    { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE_RECOVERY },
>> +
>> +    { RUN_STATE_POSTMIGRATE_RECOVERY, RUN_STATE_FINISH_MIGRATE },
>> +    { RUN_STATE_POSTMIGRATE_RECOVERY, RUN_STATE_SHUTDOWN },
>>
>>      { RUN_STATE_RESTORE_VM, RUN_STATE_RUNNING },
>>      { RUN_STATE_RESTORE_VM, RUN_STATE_PRELAUNCH },
>> --
>> 2.7.4
>
> Try to keep your patches together in a patch series; I find
>   git format-patch -n --cover-letter and then the range of patches to generate
> produces the nices result and then use git send-email.

Ok. I will use this from next time.

>
> Dave
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK



-- 

With regards,

Md Haris Iqbal,
Placement Coordinator, MTech IT
NITK Surathkal,
Contact: +91 8861996962