libxc/save: Adjust stream-position callbacks for checkpointed streams

There are some records which should only be sent once in the stream, and not
repeated for each checkpoint.  {start,end}_of_stream() become per-checkpoint,
and a new start_of_stream() is introduced.

There is no resulting change record order, but the X86_PV_INFO record is
identified as once per stream.  Currently the X86_PV_P2M_FRAMES record is as
well, but this is because of an implementation bug and can move back to being
on an as-needed basis when fixed.

In addition, a few minor adjustments of comments and layout.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Yang Hongyang <yanghy@cn.fujitsu.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>

tools/libxc/xc_sr_common.h

@@ -61,19 +61,24 @@ struct xc_sr_save_ops
     int (*setup)(struct xc_sr_context *ctx);
 
     /**
-     * Write records which need to be at the start of the stream.  This is
-     * called after the Image and Domain headers are written.  (Any records
-     * which need to be ahead of the memory.)
+     * Send records which need to be at the start of the stream.  This is
+     * called once, after the Image and Domain headers are written.
      */
     int (*start_of_stream)(struct xc_sr_context *ctx);
 
     /**
-     * Write records which need to be at the end of the stream, following the
-     * complete memory contents.  The caller shall handle writing the END
-     * record into the stream.  (Any records which need to be after the memory
-     * is complete.)
+     * Send records which need to be at the start of a checkpoint.  This is
+     * called once, or once per checkpoint in a checkpointed stream, and is
+     * ahead of memory data.
      */
-    int (*end_of_stream)(struct xc_sr_context *ctx);
+    int (*start_of_checkpoint)(struct xc_sr_context *ctx);
+
+    /**
+     * Send records which need to be at the end of the checkpoint.  This is
+     * called once, or once per checkpoint in a checkpointed stream, and is
+     * after the memory data.
+     */
+    int (*end_of_checkpoint)(struct xc_sr_context *ctx);
 
     /**
      * Clean up the local environment.  Will be called exactly once, either

tools/libxc/xc_sr_save.c

@@ -662,6 +662,10 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type)
     if ( rc )
         goto err;
 
+    rc = ctx->save.ops.start_of_checkpoint(ctx);
+    if ( rc )
+        goto err;
+
     if ( ctx->save.live )
         rc = send_domain_memory_live(ctx);
     else
@@ -678,12 +682,12 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type)
         goto err;
     }
 
-    xc_report_progress_single(xch, "End of stream");
-
-    rc = ctx->save.ops.end_of_stream(ctx);
+    rc = ctx->save.ops.end_of_checkpoint(ctx);
     if ( rc )
         goto err;
 
+    xc_report_progress_single(xch, "End of stream");
+
     rc = write_end_record(ctx);
     if ( rc )
         goto err;

tools/libxc/xc_sr_save_x86_hvm.c

@@ -184,7 +184,13 @@ static int x86_hvm_start_of_stream(struct xc_sr_context *ctx)
     return 0;
 }
 
-static int x86_hvm_end_of_stream(struct xc_sr_context *ctx)
+static int x86_hvm_start_of_checkpoint(struct xc_sr_context *ctx)
+{
+    /* no-op */
+    return 0;
+}
+
+static int x86_hvm_end_of_checkpoint(struct xc_sr_context *ctx)
 {
     int rc;
 
@@ -209,7 +215,7 @@ static int x86_hvm_end_of_stream(struct xc_sr_context *ctx)
     if ( rc )
         return rc;
 
-    return rc;
+    return 0;
 }
 
 static int x86_hvm_cleanup(struct xc_sr_context *ctx)
@@ -230,12 +236,13 @@ static int x86_hvm_cleanup(struct xc_sr_context *ctx)
 
 struct xc_sr_save_ops save_ops_x86_hvm =
 {
-    .pfn_to_gfn      = x86_hvm_pfn_to_gfn,
-    .normalise_page  = x86_hvm_normalise_page,
-    .setup           = x86_hvm_setup,
-    .start_of_stream = x86_hvm_start_of_stream,
-    .end_of_stream   = x86_hvm_end_of_stream,
-    .cleanup         = x86_hvm_cleanup,
+    .pfn_to_gfn          = x86_hvm_pfn_to_gfn,
+    .normalise_page      = x86_hvm_normalise_page,
+    .setup               = x86_hvm_setup,
+    .start_of_stream     = x86_hvm_start_of_stream,
+    .start_of_checkpoint = x86_hvm_start_of_checkpoint,
+    .end_of_checkpoint   = x86_hvm_end_of_checkpoint,
+    .cleanup             = x86_hvm_cleanup,
 };
 
 /*

tools/libxc/xc_sr_save_x86_pv.c

@@ -816,6 +816,12 @@ static int x86_pv_start_of_stream(struct xc_sr_context *ctx)
     if ( rc )
         return rc;
 
+    /*
+     * Ideally should be able to change during migration.  Currently
+     * corruption will occur if the contents or location of the P2M changes
+     * during the live migration loop.  If one is very lucky, the breakage
+     * will not be subtle.
+     */
     rc = write_x86_pv_p2m_frames(ctx);
     if ( rc )
         return rc;
@@ -823,10 +829,12 @@ static int x86_pv_start_of_stream(struct xc_sr_context *ctx)
     return 0;
 }
 
-/*
- * save_ops function.  Writes tail records information into the stream.
- */
-static int x86_pv_end_of_stream(struct xc_sr_context *ctx)
+static int x86_pv_start_of_checkpoint(struct xc_sr_context *ctx)
+{
+    return 0;
+}
+
+static int x86_pv_end_of_checkpoint(struct xc_sr_context *ctx)
 {
     int rc;
 
@@ -866,12 +874,13 @@ static int x86_pv_cleanup(struct xc_sr_context *ctx)
 
 struct xc_sr_save_ops save_ops_x86_pv =
 {
-    .pfn_to_gfn      = x86_pv_pfn_to_gfn,
-    .normalise_page  = x86_pv_normalise_page,
-    .setup           = x86_pv_setup,
-    .start_of_stream = x86_pv_start_of_stream,
-    .end_of_stream   = x86_pv_end_of_stream,
-    .cleanup         = x86_pv_cleanup,
+    .pfn_to_gfn          = x86_pv_pfn_to_gfn,
+    .normalise_page      = x86_pv_normalise_page,
+    .setup               = x86_pv_setup,
+    .start_of_stream     = x86_pv_start_of_stream,
+    .start_of_checkpoint = x86_pv_start_of_checkpoint,
+    .end_of_checkpoint   = x86_pv_end_of_checkpoint,
+    .cleanup             = x86_pv_cleanup,
 };
 
 /*