* [Qemu-devel] [Qemu-devel [RFC] [WIP] v1] Keeping the Source side alive incase of network failure (Migration recover from network failure)
@ 2016-05-31 19:11 Md Haris Iqbal
2016-06-01 16:15 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 3+ messages in thread
From: Md Haris Iqbal @ 2016-05-31 19:11 UTC (permalink / raw)
To: qemu-devel; +Cc: dgilbert, Md Haris Iqbal
---
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;
+
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);
+ if(ret != -EIO && s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE) {
+ /* Network Failure during postcopy */
+
+ current_active_state = MIGRATION_STATUS_POSTCOPY_RECOVERY;
+ 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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [Qemu-devel [RFC] [WIP] v1] Keeping the Source side alive incase of network failure (Migration recover from network failure)
2016-05-31 19:11 [Qemu-devel] [Qemu-devel [RFC] [WIP] v1] Keeping the Source side alive incase of network failure (Migration recover from network failure) Md Haris Iqbal
@ 2016-06-01 16:15 ` Dr. David Alan Gilbert
2016-06-02 8:57 ` haris iqbal
0 siblings, 1 reply; 3+ messages in thread
From: Dr. David Alan Gilbert @ 2016-06-01 16:15 UTC (permalink / raw)
To: Md Haris Iqbal; +Cc: qemu-devel
* Md Haris Iqbal (haris.phnx@gmail.com) wrote:
Remember to add a more detailed comment about what the patch is doing.
(And possibly split it up a bit more)
> ---
> 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.
> 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; 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.
> + 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')
> + 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.
Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [Qemu-devel [RFC] [WIP] v1] Keeping the Source side alive incase of network failure (Migration recover from network failure)
2016-06-01 16:15 ` Dr. David Alan Gilbert
@ 2016-06-02 8:57 ` haris iqbal
0 siblings, 0 replies; 3+ messages in thread
From: haris iqbal @ 2016-06-02 8:57 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: QEMU Developers
On Wed, Jun 1, 2016 at 9:45 PM, Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
> * Md Haris Iqbal (haris.phnx@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 / dgilbert@redhat.com / Manchester, UK
--
With regards,
Md Haris Iqbal,
Placement Coordinator, MTech IT
NITK Surathkal,
Contact: +91 8861996962
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-06-02 8:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-31 19:11 [Qemu-devel] [Qemu-devel [RFC] [WIP] v1] Keeping the Source side alive incase of network failure (Migration recover from network failure) Md Haris Iqbal
2016-06-01 16:15 ` Dr. David Alan Gilbert
2016-06-02 8:57 ` haris iqbal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).