libxc/migration: Rationalise the 'checkpointed' field to 'stream_type'

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>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
index 1c358a05776b8b056e354f1afa84654454430c33..237603373c1b5ea50502d103bb77d679fa991e26 100644 (file)
--- a/tools/libxc/include/xenguest.h
+++ b/tools/libxc/include/xenguest.h
@@ -112,11 +112,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.
@@ -125,15 +126,15 @@ typedef enum {
  * @param io_fd the file descriptor to save a domain to
  * @param dom the id of the domain
  * @param flags XCFLAGS_xxx
- * @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
- * @param recv_fd Only used for XC_MIG_STREAM_COLO.  Contains backchannel from
+ * @param recv_fd Only used for XC_STREAM_COLO.  Contains backchannel from
  *        the destination side.
  * @return 0 on success, -1 on failure
  */
 int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom,
                    uint32_t flags, struct save_callbacks *callbacks,
-                   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 {
@@ -189,11 +190,11 @@ struct restore_callbacks {
  * @param console_evtchn the console event channel for this domain to use
  * @param console_mfn filled with the gfn of the console page
  * @param console_domid the backend domain for xenconsole
- * @param stream_type XC_MIG_STREAM_NONE if the far end of the stream is using
+ * @param stream_type XC_STREAM_PLAIN if the far end of the stream is using
  *        checkpointing
  * @param callbacks non-NULL to receive a callback to restore toolstack
  *        specific data
- * @param recv_df Only used for XC_MIG_STREAM_COLO.  Contains backchannel to
+ * @param send_back_fd Only used for XC_STREAM_COLO.  Contains backchannel to
  *        the source side.
  * @return 0 on success, -1 on failure
  */
@@ -201,7 +202,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
                       unsigned int store_evtchn, unsigned long *store_mfn,
                       uint32_t store_domid, unsigned int console_evtchn,
                       unsigned long *console_mfn, uint32_t console_domid,
-                      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 5a1d7e46f93890511661e78b49ce10e8519bee6a..6795c62ddc2803527bf0e74843cb86ef99231de0 100644 (file)
--- 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,
-                   xc_migration_stream_t stream_type, int recv_fd)
+                   xc_stream_type_t stream_type, int recv_fd)
 {
     errno = ENOSYS;
     return -1;
@@ -32,7 +32,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
                       unsigned int store_evtchn, unsigned long *store_mfn,
                       uint32_t store_domid, unsigned int console_evtchn,
                       unsigned long *console_mfn, uint32_t console_domid,
-                      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 9caad8bfcbc6ef5a53c8236727d4caaff5e3f4e5..b923ad5c1063d8a25d7e4d4727f35d0f26534f77 100644 (file)
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -203,6 +203,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. */
@@ -217,9 +220,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;
 
@@ -250,9 +250,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 19442c34536e27104d1e870daa085d6be277416a..d9bf6fb5a1b56c6bd613c5fa7d0b4d2b95d4fdcf 100644 (file)
--- a/tools/libxc/xc_sr_restore.c
+++ b/tools/libxc/xc_sr_restore.c
@@ -512,7 +512,7 @@ static int handle_checkpoint(struct xc_sr_context *ctx)
     int rc = 0, ret;
     unsigned i;
 
-    if ( !ctx->restore.checkpointed )
+    if ( ctx->stream_type == XC_STREAM_PLAIN )
     {
         ERROR("Found checkpoint in non-checkpointed stream");
         rc = -1;
@@ -554,7 +554,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 {                                                    \
@@ -673,7 +673,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)));
@@ -724,7 +724,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)));
     free(ctx->restore.buffered_records);
@@ -792,8 +792,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;
@@ -833,36 +832,42 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
                       unsigned int store_evtchn, unsigned long *store_mfn,
                       uint32_t store_domid, unsigned int console_evtchn,
                       unsigned long *console_gfn, uint32_t console_domid,
-                      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,
-        };
+    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.restore.console_evtchn = console_evtchn;
     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;
     }
 
     if ( xc_domain_getinfo(xch, dom, 1, &ctx.dominfo) != 1 )

diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index 6f61f85ee05003a4b69ade1797068092b6beadbc..0651fa92bc4a81faac019cb3f4e5bbb3b8bea90c 100644 (file)
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -660,7 +660,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 )
@@ -741,7 +741,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 )
@@ -870,7 +870,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);
@@ -890,7 +890,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
@@ -903,7 +903,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 )
@@ -917,14 +917,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 )
@@ -937,7 +937,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");
 
@@ -967,19 +967,18 @@ 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,
-                   xc_migration_stream_t stream_type, int recv_fd)
+                   xc_stream_type_t stream_type, int recv_fd)
 {
-    struct xc_sr_context ctx =
-        {
-            .xch = xch,
-            .fd = io_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 ( xc_domain_getinfo(xch, dom, 1, &ctx.dominfo) != 1 )
@@ -994,18 +993,24 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom,
         return -1;
     }
 
-    /* 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 checks for callbacks. */
-    if ( ctx.dominfo.hvm )
-        assert(callbacks->switch_qemu_logdirty);
-    if ( ctx.save.checkpointed )
-        assert(callbacks->checkpoint && callbacks->postcopy);
-    if ( ctx.save.checkpointed == 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:
+        if ( ctx.dominfo.hvm )
+            assert(callbacks->switch_qemu_logdirty);
+        break;
+
+    default:
+        assert(!"Bad stream_type");
+        break;
+    }
 
     DPRINTF("fd %d, dom %u, flags %u, hvm %d",
             io_fd, dom, flags, ctx.dominfo.hvm);

diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c
index 0f52930c45524c11f1cf8f7633eec304bae5eeb6..017c7cd98827391c9d7403f7936c39b0ac882c11 100644 (file)
--- a/tools/libxl/libxl_save_helper.c
+++ b/tools/libxl/libxl_save_helper.c
@@ -253,7 +253,7 @@ int main(int argc, char **argv)
         uint32_t dom =                      strtoul(NEXTARG,0,10);
         uint32_t flags =                    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_save(&helper_save_callbacks, cbflags);
@@ -275,7 +275,7 @@ int main(int argc, char **argv)
         unsigned console_evtchn =           strtoul(NEXTARG,0,10);
         domid_t console_domid =             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);