diff mbox

[RFC,18/29] migration: new state "postcopy-recover"

Message ID 1501229198-30588-19-git-send-email-peterx@redhat.com
State New
Headers show

Commit Message

Peter Xu July 28, 2017, 8:06 a.m. UTC
Introducing new migration state "postcopy-recover". If a migration
procedure is paused and the connection is rebuilt afterward
successfully, we'll switch the source VM state from "postcopy-paused" to
the new state "postcopy-recover", then we'll do the resume logic in the
migration thread (along with the return path thread).

This patch only do the state switch on source side. Another following up
patch will handle the state switching on destination side using the same
status bit.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/migration.c | 45 +++++++++++++++++++++++++++++++++++++++++----
 qapi-schema.json      |  4 +++-
 2 files changed, 44 insertions(+), 5 deletions(-)

Comments

Dr. David Alan Gilbert Aug. 1, 2017, 11:36 a.m. UTC | #1
* Peter Xu (peterx@redhat.com) wrote:
> Introducing new migration state "postcopy-recover". If a migration
> procedure is paused and the connection is rebuilt afterward
> successfully, we'll switch the source VM state from "postcopy-paused" to
> the new state "postcopy-recover", then we'll do the resume logic in the
> migration thread (along with the return path thread).
> 
> This patch only do the state switch on source side. Another following up
> patch will handle the state switching on destination side using the same
> status bit.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/migration.c | 45 +++++++++++++++++++++++++++++++++++++++++----
>  qapi-schema.json      |  4 +++-
>  2 files changed, 44 insertions(+), 5 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 64de0ee..3aabe11 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -495,6 +495,7 @@ static bool migration_is_setup_or_active(int state)
>      case MIGRATION_STATUS_ACTIVE:
>      case MIGRATION_STATUS_POSTCOPY_ACTIVE:
>      case MIGRATION_STATUS_POSTCOPY_PAUSED:
> +    case MIGRATION_STATUS_POSTCOPY_RECOVER:
>      case MIGRATION_STATUS_SETUP:
>          return true;
>  
> @@ -571,6 +572,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>      case MIGRATION_STATUS_CANCELLING:
>      case MIGRATION_STATUS_POSTCOPY_ACTIVE:
>      case MIGRATION_STATUS_POSTCOPY_PAUSED:
> +    case MIGRATION_STATUS_POSTCOPY_RECOVER:
>           /* TODO add some postcopy stats */
>          info->has_status = true;
>          info->has_total_time = true;
> @@ -2018,6 +2020,13 @@ static bool postcopy_should_start(MigrationState *s)
>      return atomic_read(&s->start_postcopy) || s->start_postcopy_fast;
>  }
>  
> +/* Return zero if success, or <0 for error */
> +static int postcopy_do_resume(MigrationState *s)
> +{
> +    /* TODO: do the resume logic */
> +    return 0;
> +}
> +
>  /*
>   * We don't return until we are in a safe state to continue current
>   * postcopy migration.  Returns true to continue the migration, or
> @@ -2026,7 +2035,9 @@ static bool postcopy_should_start(MigrationState *s)
>  static bool postcopy_pause(MigrationState *s)
>  {
>      assert(s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE);
> -    migrate_set_state(&s->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
> +
> +do_pause:
> +    migrate_set_state(&s->state, s->state,
>                        MIGRATION_STATUS_POSTCOPY_PAUSED);
>  
>      /* Current channel is possibly broken. Release it. */
> @@ -2043,9 +2054,32 @@ static bool postcopy_pause(MigrationState *s)
>          qemu_sem_wait(&s->postcopy_pause_sem);
>      }
>  
> -    trace_postcopy_pause_continued();
> +    if (s->state == MIGRATION_STATUS_POSTCOPY_RECOVER) {
> +        /* We were waken up by a recover procedure. Give it a shot */
>  
> -    return true;
> +        /*
> +         * Firstly, let's wake up the return path now, with a new
> +         * return path channel.
> +         */
> +        qemu_sem_post(&s->postcopy_pause_rp_sem);
> +
> +        /* Do the resume logic */
> +        if (postcopy_do_resume(s) == 0) {
> +            /* Let's continue! */
> +            trace_postcopy_pause_continued();
> +            return true;
> +        } else {
> +            /*
> +             * Something wrong happened during the recovery, let's
> +             * pause again. Pause is always better than throwing data
> +             * away.
> +             */
> +            goto do_pause;

You should be able to turn this around into a do {} while or similar
rather than goto.

Dave

> +        }
> +    } else {
> +        /* This is not right... Time to quit. */
> +        return false;
> +    }
>  }
>  
>  /* Return true if we want to stop the migration, otherwise false. */
> @@ -2300,7 +2334,10 @@ void migrate_fd_connect(MigrationState *s)
>      }
>  
>      if (resume) {
> -        /* TODO: do the resume logic */
> +        /* Wakeup the main migration thread to do the recovery */
> +        migrate_set_state(&s->state, MIGRATION_STATUS_POSTCOPY_PAUSED,
> +                          MIGRATION_STATUS_POSTCOPY_RECOVER);
> +        qemu_sem_post(&s->postcopy_pause_sem);
>          return;
>      }
>  
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 27b7c4c..10f1f60 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -669,6 +669,8 @@
>  #
>  # @postcopy-paused: during postcopy but paused. (since 2.10)
>  #
> +# @postcopy-recover: trying to recover from a paused postcopy. (since 2.11)
> +#
>  # @completed: migration is finished.
>  #
>  # @failed: some error occurred during migration process.
> @@ -682,7 +684,7 @@
>  { 'enum': 'MigrationStatus',
>    'data': [ 'none', 'setup', 'cancelling', 'cancelled',
>              'active', 'postcopy-active', 'postcopy-paused',
> -            'completed', 'failed', 'colo' ] }
> +            'postcopy-recover', 'completed', 'failed', 'colo' ] }
>  
>  ##
>  # @MigrationInfo:
> -- 
> 2.7.4
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Peter Xu Aug. 2, 2017, 6:42 a.m. UTC | #2
On Tue, Aug 01, 2017 at 12:36:22PM +0100, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:

[...]
> > @@ -2043,9 +2054,32 @@ static bool postcopy_pause(MigrationState *s)
> >          qemu_sem_wait(&s->postcopy_pause_sem);
> >      }
> >  
> > -    trace_postcopy_pause_continued();
> > +    if (s->state == MIGRATION_STATUS_POSTCOPY_RECOVER) {
> > +        /* We were waken up by a recover procedure. Give it a shot */
> >  
> > -    return true;
> > +        /*
> > +         * Firstly, let's wake up the return path now, with a new
> > +         * return path channel.
> > +         */
> > +        qemu_sem_post(&s->postcopy_pause_rp_sem);
> > +
> > +        /* Do the resume logic */
> > +        if (postcopy_do_resume(s) == 0) {
> > +            /* Let's continue! */
> > +            trace_postcopy_pause_continued();
> > +            return true;
> > +        } else {
> > +            /*
> > +             * Something wrong happened during the recovery, let's
> > +             * pause again. Pause is always better than throwing data
> > +             * away.
> > +             */
> > +            goto do_pause;
> 
> You should be able to turn this around into a do {} while or similar
> rather than goto.

Indeed. Fixing up.  Thanks,
diff mbox

Patch

diff --git a/migration/migration.c b/migration/migration.c
index 64de0ee..3aabe11 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -495,6 +495,7 @@  static bool migration_is_setup_or_active(int state)
     case MIGRATION_STATUS_ACTIVE:
     case MIGRATION_STATUS_POSTCOPY_ACTIVE:
     case MIGRATION_STATUS_POSTCOPY_PAUSED:
+    case MIGRATION_STATUS_POSTCOPY_RECOVER:
     case MIGRATION_STATUS_SETUP:
         return true;
 
@@ -571,6 +572,7 @@  MigrationInfo *qmp_query_migrate(Error **errp)
     case MIGRATION_STATUS_CANCELLING:
     case MIGRATION_STATUS_POSTCOPY_ACTIVE:
     case MIGRATION_STATUS_POSTCOPY_PAUSED:
+    case MIGRATION_STATUS_POSTCOPY_RECOVER:
          /* TODO add some postcopy stats */
         info->has_status = true;
         info->has_total_time = true;
@@ -2018,6 +2020,13 @@  static bool postcopy_should_start(MigrationState *s)
     return atomic_read(&s->start_postcopy) || s->start_postcopy_fast;
 }
 
+/* Return zero if success, or <0 for error */
+static int postcopy_do_resume(MigrationState *s)
+{
+    /* TODO: do the resume logic */
+    return 0;
+}
+
 /*
  * We don't return until we are in a safe state to continue current
  * postcopy migration.  Returns true to continue the migration, or
@@ -2026,7 +2035,9 @@  static bool postcopy_should_start(MigrationState *s)
 static bool postcopy_pause(MigrationState *s)
 {
     assert(s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE);
-    migrate_set_state(&s->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
+
+do_pause:
+    migrate_set_state(&s->state, s->state,
                       MIGRATION_STATUS_POSTCOPY_PAUSED);
 
     /* Current channel is possibly broken. Release it. */
@@ -2043,9 +2054,32 @@  static bool postcopy_pause(MigrationState *s)
         qemu_sem_wait(&s->postcopy_pause_sem);
     }
 
-    trace_postcopy_pause_continued();
+    if (s->state == MIGRATION_STATUS_POSTCOPY_RECOVER) {
+        /* We were waken up by a recover procedure. Give it a shot */
 
-    return true;
+        /*
+         * Firstly, let's wake up the return path now, with a new
+         * return path channel.
+         */
+        qemu_sem_post(&s->postcopy_pause_rp_sem);
+
+        /* Do the resume logic */
+        if (postcopy_do_resume(s) == 0) {
+            /* Let's continue! */
+            trace_postcopy_pause_continued();
+            return true;
+        } else {
+            /*
+             * Something wrong happened during the recovery, let's
+             * pause again. Pause is always better than throwing data
+             * away.
+             */
+            goto do_pause;
+        }
+    } else {
+        /* This is not right... Time to quit. */
+        return false;
+    }
 }
 
 /* Return true if we want to stop the migration, otherwise false. */
@@ -2300,7 +2334,10 @@  void migrate_fd_connect(MigrationState *s)
     }
 
     if (resume) {
-        /* TODO: do the resume logic */
+        /* Wakeup the main migration thread to do the recovery */
+        migrate_set_state(&s->state, MIGRATION_STATUS_POSTCOPY_PAUSED,
+                          MIGRATION_STATUS_POSTCOPY_RECOVER);
+        qemu_sem_post(&s->postcopy_pause_sem);
         return;
     }
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 27b7c4c..10f1f60 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -669,6 +669,8 @@ 
 #
 # @postcopy-paused: during postcopy but paused. (since 2.10)
 #
+# @postcopy-recover: trying to recover from a paused postcopy. (since 2.11)
+#
 # @completed: migration is finished.
 #
 # @failed: some error occurred during migration process.
@@ -682,7 +684,7 @@ 
 { 'enum': 'MigrationStatus',
   'data': [ 'none', 'setup', 'cancelling', 'cancelled',
             'active', 'postcopy-active', 'postcopy-paused',
-            'completed', 'failed', 'colo' ] }
+            'postcopy-recover', 'completed', 'failed', 'colo' ] }
 
 ##
 # @MigrationInfo: