[Xen-devel] [PATCH 03/12] libxc/migration: Rationalise the 'checkpointed' field to 'stream_type'

[Andrew Cooper <andrew.cooper3@citrix.com>]

Originally, 'checkpointed' was a boolean signalling the difference between a
plain and a Remus stream.  COLO was added later, but several bits of code
retained boolean-style logic.  While correct, it is confusing to follow.

Additionally, XC_MIG_STREAM_NONE means "no checkpoints" but reads as "no
stream".

Consolidate all the logic on the term 'stream_type', and rename STREAM_NONE
to STREAM_PLAIN.  Re-position the stream_type variable so it isn't
duplicated in both the save and restore unions.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 tools/libxc/include/xenguest.h  | 15 +++++++-------
 tools/libxc/xc_nomigrate.c      |  4 ++--
 tools/libxc/xc_sr_common.h      |  9 +++------
 tools/libxc/xc_sr_restore.c     | 33 +++++++++++++++++++------------
 tools/libxc/xc_sr_save.c        | 44 ++++++++++++++++++++++++-----------------
 tools/libxl/libxl_save_helper.c |  4 ++--
 6 files changed, 61 insertions(+), 48 deletions(-)

diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
index b4b2e19619..9ba09af743 100644
--- a/tools/libxc/include/xenguest.h
+++ b/tools/libxc/include/xenguest.h
@@ -115,11 +115,12 @@ struct save_callbacks {
     void* data;
 };
 
+/* Type of stream.  Plain, or using a continuous replication protocol? */
 typedef enum {
-    XC_MIG_STREAM_NONE, /* plain stream */
-    XC_MIG_STREAM_REMUS,
-    XC_MIG_STREAM_COLO,
-} xc_migration_stream_t;
+    XC_STREAM_PLAIN,
+    XC_STREAM_REMUS,
+    XC_STREAM_COLO,
+} xc_stream_type_t;
 
 /**
  * This function will save a running domain.
@@ -127,14 +128,14 @@ typedef enum {
  * @parm xch a handle to an open hypervisor interface
  * @parm fd the file descriptor to save a domain to
  * @parm dom the id of the domain
- * @param stream_type XC_MIG_STREAM_NONE if the far end of the stream
+ * @param stream_type XC_STREAM_PLAIN if the far end of the stream
  *        doesn't use checkpointing
  * @return 0 on success, -1 on failure
  */
 int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom,
                    uint32_t flags /* XCFLAGS_xxx */,
                    struct save_callbacks* callbacks, int hvm,
-                   xc_migration_stream_t stream_type, int recv_fd);
+                   xc_stream_type_t stream_type, int recv_fd);
 
 /* callbacks provided by xc_domain_restore */
 struct restore_callbacks {
@@ -198,7 +199,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
                       uint32_t store_domid, unsigned int console_evtchn,
                       unsigned long *console_mfn, uint32_t console_domid,
                       unsigned int hvm, unsigned int pae,
-                      xc_migration_stream_t stream_type,
+                      xc_stream_type_t stream_type,
                       struct restore_callbacks *callbacks, int send_back_fd);
 
 /**
diff --git a/tools/libxc/xc_nomigrate.c b/tools/libxc/xc_nomigrate.c
index 6d6169d5ad..3099e3278c 100644
--- a/tools/libxc/xc_nomigrate.c
+++ b/tools/libxc/xc_nomigrate.c
@@ -22,7 +22,7 @@
 
 int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t flags,
                    struct save_callbacks* callbacks, int hvm,
-                   xc_migration_stream_t stream_type, int recv_fd)
+                   xc_stream_type_t stream_type, int recv_fd)
 {
     errno = ENOSYS;
     return -1;
@@ -33,7 +33,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
                       uint32_t store_domid, unsigned int console_evtchn,
                       unsigned long *console_mfn, uint32_t console_domid,
                       unsigned int hvm, unsigned int pae,
-                      xc_migration_stream_t stream_type,
+                      xc_stream_type_t stream_type,
                       struct restore_callbacks *callbacks, int send_back_fd)
 {
     errno = ENOSYS;
diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index 19b053911f..4db63a63b2 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -205,6 +205,9 @@ struct xc_sr_context
     uint32_t domid;
     int fd;
 
+    /* Plain VM, or checkpoints over time. */
+    xc_stream_type_t stream_type;
+
     xc_dominfo_t dominfo;
 
     union /* Common save or restore data. */
@@ -219,9 +222,6 @@ struct xc_sr_context
             /* Live migrate vs non live suspend. */
             bool live;
 
-            /* Plain VM, or checkpoints over time. */
-            int checkpointed;
-
             /* Further debugging information in the stream. */
             bool debug;
 
@@ -252,9 +252,6 @@ struct xc_sr_context
             uint32_t guest_type;
             uint32_t guest_page_size;
 
-            /* Plain VM, or checkpoints over time. */
-            int checkpointed;
-
             /* Currently buffering records between a checkpoint */
             bool buffer_all_records;
 
diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
index 98f3fe4098..7872b71ab5 100644
--- a/tools/libxc/xc_sr_restore.c
+++ b/tools/libxc/xc_sr_restore.c
@@ -511,7 +511,7 @@ static int handle_checkpoint(struct xc_sr_context *ctx)
     int rc = 0, ret;
     unsigned int i;
 
-    if ( !ctx->restore.checkpointed )
+    if ( ctx->stream_type == XC_STREAM_PLAIN )
     {
         ERROR("Found checkpoint in non-checkpointed stream");
         rc = -1;
@@ -553,7 +553,7 @@ static int handle_checkpoint(struct xc_sr_context *ctx)
     else
         ctx->restore.buffer_all_records = true;
 
-    if ( ctx->restore.checkpointed == XC_MIG_STREAM_COLO )
+    if ( ctx->stream_type == XC_STREAM_COLO )
     {
 #define HANDLE_CALLBACK_RETURN_VALUE(ret)                   \
     do {                                                    \
@@ -672,7 +672,7 @@ static int setup(struct xc_sr_context *ctx)
     DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
                                     &ctx->restore.dirty_bitmap_hbuf);
 
-    if ( ctx->restore.checkpointed == XC_MIG_STREAM_COLO )
+    if ( ctx->stream_type == XC_STREAM_COLO )
     {
         dirty_bitmap = xc_hypercall_buffer_alloc_pages(
             xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->restore.p2m_size)));
@@ -723,7 +723,7 @@ static void cleanup(struct xc_sr_context *ctx)
     for ( i = 0; i < ctx->restore.buffered_rec_num; i++ )
         free(ctx->restore.buffered_records[i].data);
 
-    if ( ctx->restore.checkpointed == XC_MIG_STREAM_COLO )
+    if ( ctx->stream_type == XC_STREAM_COLO )
         xc_hypercall_buffer_free_pages(
             xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->restore.p2m_size)));
 
@@ -793,7 +793,7 @@ static int restore(struct xc_sr_context *ctx)
     } while ( rec.type != REC_TYPE_END );
 
  remus_failover:
-    if ( ctx->restore.checkpointed == XC_MIG_STREAM_COLO )
+    if ( ctx->stream_type == XC_STREAM_COLO )
     {
         /* With COLO, we have already called stream_complete */
         rc = 0;
@@ -834,13 +834,14 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
                       uint32_t store_domid, unsigned int console_evtchn,
                       unsigned long *console_gfn, uint32_t console_domid,
                       unsigned int hvm, unsigned int pae,
-                      xc_migration_stream_t stream_type,
+                      xc_stream_type_t stream_type,
                       struct restore_callbacks *callbacks, int send_back_fd)
 {
     xen_pfn_t nr_pfns;
     struct xc_sr_context ctx = {
         .xch = xch,
         .fd = io_fd,
+        .stream_type = stream_type,
     };
 
     /* GCC 4.4 (of CentOS 6.x vintage) can' t initialise anonymous unions. */
@@ -848,21 +849,27 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
     ctx.restore.console_domid = console_domid;
     ctx.restore.xenstore_evtchn = store_evtchn;
     ctx.restore.xenstore_domid = store_domid;
-    ctx.restore.checkpointed = stream_type;
     ctx.restore.callbacks = callbacks;
     ctx.restore.send_back_fd = send_back_fd;
 
-    /* Sanity checks for callbacks. */
-    if ( stream_type )
-        assert(callbacks->checkpoint);
-
-    if ( ctx.restore.checkpointed == XC_MIG_STREAM_COLO )
+    /* Sanity check stream_type-related parameters */
+    switch ( stream_type )
     {
-        /* this is COLO restore */
+    case XC_STREAM_COLO:
         assert(callbacks->suspend &&
                callbacks->postcopy &&
                callbacks->wait_checkpoint &&
                callbacks->restore_results);
+        /* Fallthrough */
+    case XC_STREAM_REMUS:
+        assert(callbacks->checkpoint);
+        /* Fallthrough */
+    case XC_STREAM_PLAIN:
+        break;
+
+    default:
+        assert(!"Bad stream_type");
+        break;
     }
 
     DPRINTF("fd %d, dom %u, hvm %u, pae %u, stream_type %d",
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index 9764aa743f..5467965b08 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -658,7 +658,7 @@ static int suspend_and_send_dirty(struct xc_sr_context *ctx)
 
     bitmap_or(dirty_bitmap, ctx->save.deferred_pages, ctx->save.p2m_size);
 
-    if ( !ctx->save.live && ctx->save.checkpointed == XC_MIG_STREAM_COLO )
+    if ( !ctx->save.live && ctx->stream_type == XC_STREAM_COLO )
     {
         rc = colo_merge_secondary_dirty_bitmap(ctx);
         if ( rc )
@@ -735,7 +735,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
     if ( rc )
         goto out;
 
-    if ( ctx->save.debug && ctx->save.checkpointed != XC_MIG_STREAM_NONE )
+    if ( ctx->save.debug && ctx->stream_type != XC_STREAM_PLAIN )
     {
         rc = verify_frames(ctx);
         if ( rc )
@@ -861,7 +861,7 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type)
 
         if ( ctx->save.live )
             rc = send_domain_memory_live(ctx);
-        else if ( ctx->save.checkpointed != XC_MIG_STREAM_NONE )
+        else if ( ctx->stream_type != XC_STREAM_PLAIN )
             rc = send_domain_memory_checkpointed(ctx);
         else
             rc = send_domain_memory_nonlive(ctx);
@@ -881,7 +881,7 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type)
         if ( rc )
             goto err;
 
-        if ( ctx->save.checkpointed != XC_MIG_STREAM_NONE )
+        if ( ctx->stream_type != XC_STREAM_PLAIN )
         {
             /*
              * We have now completed the initial live portion of the checkpoint
@@ -894,7 +894,7 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type)
             if ( rc )
                 goto err;
 
-            if ( ctx->save.checkpointed == XC_MIG_STREAM_COLO )
+            if ( ctx->stream_type == XC_STREAM_COLO )
             {
                 rc = ctx->save.callbacks->checkpoint(ctx->save.callbacks->data);
                 if ( !rc )
@@ -908,14 +908,14 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type)
             if ( rc <= 0 )
                 goto err;
 
-            if ( ctx->save.checkpointed == XC_MIG_STREAM_COLO )
+            if ( ctx->stream_type == XC_STREAM_COLO )
             {
                 rc = ctx->save.callbacks->wait_checkpoint(
                     ctx->save.callbacks->data);
                 if ( rc <= 0 )
                     goto err;
             }
-            else if ( ctx->save.checkpointed == XC_MIG_STREAM_REMUS )
+            else if ( ctx->stream_type == XC_STREAM_REMUS )
             {
                 rc = ctx->save.callbacks->checkpoint(ctx->save.callbacks->data);
                 if ( rc <= 0 )
@@ -928,7 +928,7 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type)
                 goto err;
             }
         }
-    } while ( ctx->save.checkpointed != XC_MIG_STREAM_NONE );
+    } while ( ctx->stream_type != XC_STREAM_PLAIN );
 
     xc_report_progress_single(xch, "End of stream");
 
@@ -958,32 +958,40 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type)
 
 int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom,
                    uint32_t flags, struct save_callbacks *callbacks,
-                   int hvm, xc_migration_stream_t stream_type, int recv_fd)
+                   int hvm, xc_stream_type_t stream_type, int recv_fd)
 {
     struct xc_sr_context ctx = {
         .xch = xch,
         .fd = io_fd,
+        .stream_type = stream_type,
     };
 
     /* GCC 4.4 (of CentOS 6.x vintage) can' t initialise anonymous unions. */
     ctx.save.callbacks = callbacks;
     ctx.save.live  = !!(flags & XCFLAGS_LIVE);
     ctx.save.debug = !!(flags & XCFLAGS_DEBUG);
-    ctx.save.checkpointed = stream_type;
     ctx.save.recv_fd = recv_fd;
 
-    /* If altering migration_stream update this assert too. */
-    assert(stream_type == XC_MIG_STREAM_NONE ||
-           stream_type == XC_MIG_STREAM_REMUS ||
-           stream_type == XC_MIG_STREAM_COLO);
+    /* Sanity check stream_type-related parameters */
+    switch ( stream_type )
+    {
+    case XC_STREAM_COLO:
+        assert(callbacks->wait_checkpoint);
+        /* Fallthrough */
+    case XC_STREAM_REMUS:
+        assert(callbacks->checkpoint && callbacks->postcopy);
+        /* Fallthrough */
+    case XC_STREAM_PLAIN:
+        break;
+
+    default:
+        assert(!"Bad stream_type");
+        break;
+    }
 
     /* Sanity checks for callbacks. */
     if ( hvm )
         assert(callbacks->switch_qemu_logdirty);
-    if ( ctx.save.checkpointed )
-        assert(callbacks->checkpoint && callbacks->postcopy);
-    if ( ctx.save.checkpointed == XC_MIG_STREAM_COLO )
-        assert(callbacks->wait_checkpoint);
 
     DPRINTF("fd %d, dom %u, flags %u, hvm %d", io_fd, dom, flags, hvm);
 
diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c
index 38089a002d..398df00dd6 100644
--- a/tools/libxl/libxl_save_helper.c
+++ b/tools/libxl/libxl_save_helper.c
@@ -254,7 +254,7 @@ int main(int argc, char **argv)
         uint32_t flags =                    strtoul(NEXTARG,0,10);
         int hvm =                           atoi(NEXTARG);
         unsigned cbflags =                  strtoul(NEXTARG,0,10);
-        xc_migration_stream_t stream_type = strtoul(NEXTARG,0,10);
+        xc_stream_type_t stream_type =      strtoul(NEXTARG,0,10);
         assert(!*++argv);
 
         helper_setcallbacks_save(&helper_save_callbacks, cbflags);
@@ -278,7 +278,7 @@ int main(int argc, char **argv)
         unsigned int hvm =                  strtoul(NEXTARG,0,10);
         unsigned int pae =                  strtoul(NEXTARG,0,10);
         unsigned cbflags =                  strtoul(NEXTARG,0,10);
-        xc_migration_stream_t stream_type = strtoul(NEXTARG,0,10);
+        xc_stream_type_t stream_type =      strtoul(NEXTARG,0,10);
         assert(!*++argv);
 
         helper_setcallbacks_restore(&helper_restore_callbacks, cbflags);
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel