On Wed, Jun 1, 2016 at 9:45 PM, Dr. David Alan Gilbert
<dgilb...@redhat.com> wrote:
> * Md Haris Iqbal (haris.p...@gmail.com) 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 / dgilb...@redhat.com / Manchester, UK
--
With regards,
Md Haris Iqbal,
Placement Coordinator, MTech IT
NITK Surathkal,
Contact: +91 8861996962
