[PATCH v5 00/14] Misc patches to aid migration v2 Remus support

[PATCH v5 00/14] Misc patches to aid migration v2 Remus support

[Yang Hongyang]

This is the combination of Andrew Cooper's misc patches and mine
to aid migration v2 Remus support.

See individual patches for details.

Git tree available at:
    https://github.com/macrosheep/xen/tree/misc-remus-v5

v4->v5
Most of the changes are trival, like drop brackets in
DECLARE_HYPERCALL_BUFFER_SHADOW(), drop stray blank line etc.
Except the 6th patch which add a do{}while (0) in
xc_hypercall_buffer_free_pages macro.

Summary of changes:
M = modified
A = acked
no mark = unchanged from last round

Andrew Cooper (4):
   libxc/migration: Be rather stricter with illformed callers
   libxc/save: Adjust stream-position callbacks for checkpointed streams
M  libxc/migration: Specification update for CHECKPOINT records
   libxc/migration: Pass checkpoint information into the save algorithm.

Yang Hongyang (10):
   tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW
M  tools/libxc: add a check in xc_hypercall_buffer_free_pages macro
M  libxc/save: introduce setup() and cleanup() on save
   libxc/save: rename to_send to dirty_bitmap
M  libxc/save: adjust the memory allocation for migration
M  libxc/save: remove bitmap param from send_some_pages
   libxc/save: rename send_some_pages to send_dirty_pages
M  libxc/save: reuse send_dirty_pages() in send_all_pages()
   libxc/restore: introduce process_record()
   libxc/restore: split read/handle qemu info

 docs/specs/libxc-migration-stream.pandoc |  29 +++++-
 tools/libxc/include/xenctrl.h            |  11 +-
 tools/libxc/include/xenguest.h           |   1 +
 tools/libxc/xc_bitops.h                  |   5 +
 tools/libxc/xc_sr_common.c               |   1 +
 tools/libxc/xc_sr_common.h               |  30 ++++--
 tools/libxc/xc_sr_restore.c              |  89 ++++++++++------
 tools/libxc/xc_sr_restore_x86_hvm.c      |  28 ++++-
 tools/libxc/xc_sr_save.c                 | 171 ++++++++++++++++---------------
 tools/libxc/xc_sr_save_x86_hvm.c         |  30 +++---
 tools/libxc/xc_sr_save_x86_pv.c          |  29 ++++--
 tools/libxc/xc_sr_stream_format.h        |   1 +
 tools/libxl/libxl_dom.c                  |   1 +
 13 files changed, 272 insertions(+), 154 deletions(-)

-- 
1.9.1

[PATCH v5 01/14] libxc/migration: Be rather stricter with illformed callers

[Yang Hongyang]

From: Andrew Cooper <andrew.cooper3@citrix.com>

The migration code itself should be able to validly assume all mandatory
callbacks are set up.

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>
---
 tools/libxc/xc_sr_save.c         | 4 ++++
 tools/libxc/xc_sr_save_x86_hvm.c | 7 -------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index 5d9c267..83f0591 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -738,6 +738,10 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom,
     ctx.save.max_iterations = 5;
     ctx.save.dirty_threshold = 50;
 
+    /* Sanity checks for callbacks. */
+    if ( hvm )
+        assert(callbacks->switch_qemu_logdirty);
+
     IPRINTF("In experimental %s", __func__);
     DPRINTF("fd %d, dom %u, max_iters %u, max_factor %u, flags %u, hvm %d",
             io_fd, dom, max_iters, max_factor, flags, hvm);
diff --git a/tools/libxc/xc_sr_save_x86_hvm.c b/tools/libxc/xc_sr_save_x86_hvm.c
index 8baa104..58efdb9 100644
--- a/tools/libxc/xc_sr_save_x86_hvm.c
+++ b/tools/libxc/xc_sr_save_x86_hvm.c
@@ -166,13 +166,6 @@ static int x86_hvm_setup(struct xc_sr_context *ctx)
 {
     xc_interface *xch = ctx->xch;
 
-    if ( !ctx->save.callbacks->switch_qemu_logdirty )
-    {
-        ERROR("No switch_qemu_logdirty callback provided");
-        errno = EINVAL;
-        return -1;
-    }
-
     if ( ctx->save.callbacks->switch_qemu_logdirty(
              ctx->domid, 1, ctx->save.callbacks->data) )
     {
-- 
1.9.1

[PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams

[Yang Hongyang]

From: Andrew Cooper <andrew.cooper3@citrix.com>

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>
---
 tools/libxc/xc_sr_common.h       | 21 +++++++++++++--------
 tools/libxc/xc_sr_save.c         | 10 +++++++---
 tools/libxc/xc_sr_save_x86_hvm.c | 23 +++++++++++++++--------
 tools/libxc/xc_sr_save_x86_pv.c  | 29 +++++++++++++++++++----------
 4 files changed, 54 insertions(+), 29 deletions(-)

diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index ef42412..c4fe92c 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/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
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index 83f0591..66fcd3e 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/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;
diff --git a/tools/libxc/xc_sr_save_x86_hvm.c b/tools/libxc/xc_sr_save_x86_hvm.c
index 58efdb9..f4604db 100644
--- a/tools/libxc/xc_sr_save_x86_hvm.c
+++ b/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,
 };
 
 /*
diff --git a/tools/libxc/xc_sr_save_x86_pv.c b/tools/libxc/xc_sr_save_x86_pv.c
index a668221..f63f40b 100644
--- a/tools/libxc/xc_sr_save_x86_pv.c
+++ b/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,
 };
 
 /*
-- 
1.9.1

[PATCH v5 03/14] libxc/migration: Specification update for CHECKPOINT records

[Yang Hongyang]

From: Andrew Cooper <andrew.cooper3@citrix.com>

Checkpointed streams need to signal the end of a consistent view of VM state,
and the start of the libxl data.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 docs/specs/libxc-migration-stream.pandoc | 29 ++++++++++++++++++++++++++---
 tools/libxc/xc_sr_common.c               |  1 +
 tools/libxc/xc_sr_stream_format.h        |  1 +
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/docs/specs/libxc-migration-stream.pandoc b/docs/specs/libxc-migration-stream.pandoc
index 520240f..68fa513 100644
--- a/docs/specs/libxc-migration-stream.pandoc
+++ b/docs/specs/libxc-migration-stream.pandoc
@@ -37,8 +37,6 @@ Not Yet Included
 The following features are not yet fully specified and will be
 included in a future draft.
 
-* Remus
-
 * Page data compression.
 
 * ARM
@@ -227,7 +225,9 @@ type         0x00000000: END
 
              0x0000000D: VERIFY
 
-             0x0000000E - 0x7FFFFFFF: Reserved for future _mandatory_
+             0x0000000E: CHECKPOINT
+
+             0x0000000F - 0x7FFFFFFF: Reserved for future _mandatory_
              records.
 
              0x80000000 - 0xFFFFFFFF: Reserved for future _optional_
@@ -578,6 +578,29 @@ The verify record contains no fields; its body_length is 0.
 
 \clearpage
 
+CHECKPOINT
+----------
+
+A checkpoint record indicates that all the preceding records in the stream
+represent a consistent view of VM state.
+
+     0     1     2     3     4     5     6     7 octet
+    +-------------------------------------------------+
+
+The checkpoint record contains no fields; its body_length is 0
+
+If the stream is embedded in a higher level toolstack stream, the
+CHECKPOINT record marks the end of the libxc portion of the stream
+and the stream is handed back to the higher level for further
+processing.
+
+The higher level stream may then hand the stream back to libxc to
+process another set of records for the next consistent VM state
+snapshot.  This next set of records may be terminated by another
+CHECKPOINT record or an END record.
+
+\clearpage
+
 Layout
 ======
 
diff --git a/tools/libxc/xc_sr_common.c b/tools/libxc/xc_sr_common.c
index 59e0c5d..945cfa6 100644
--- a/tools/libxc/xc_sr_common.c
+++ b/tools/libxc/xc_sr_common.c
@@ -34,6 +34,7 @@ static const char *mandatory_rec_types[] =
     [REC_TYPE_TOOLSTACK]            = "Toolstack",
     [REC_TYPE_X86_PV_VCPU_MSRS]     = "x86 PV vcpu msrs",
     [REC_TYPE_VERIFY]               = "Verify",
+    [REC_TYPE_CHECKPOINT]           = "Checkpoint",
 };
 
 const char *rec_type_to_str(uint32_t type)
diff --git a/tools/libxc/xc_sr_stream_format.h b/tools/libxc/xc_sr_stream_format.h
index d116ca6..6d0f8fd 100644
--- a/tools/libxc/xc_sr_stream_format.h
+++ b/tools/libxc/xc_sr_stream_format.h
@@ -74,6 +74,7 @@ struct xc_sr_rhdr
 #define REC_TYPE_TOOLSTACK            0x0000000bU
 #define REC_TYPE_X86_PV_VCPU_MSRS     0x0000000cU
 #define REC_TYPE_VERIFY               0x0000000dU
+#define REC_TYPE_CHECKPOINT           0x0000000eU
 
 #define REC_TYPE_OPTIONAL             0x80000000U
 
-- 
1.9.1

[PATCH v5 04/14] libxc/migration: Pass checkpoint information into the save algorithm.

[Yang Hongyang]

From: Andrew Cooper <andrew.cooper3@citrix.com>

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>
---
 tools/libxc/include/xenguest.h | 1 +
 tools/libxc/xc_sr_common.h     | 3 +++
 tools/libxc/xc_sr_save.c       | 3 +++
 tools/libxl/libxl_dom.c        | 1 +
 4 files changed, 8 insertions(+)

diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
index 8e39075..7581263 100644
--- a/tools/libxc/include/xenguest.h
+++ b/tools/libxc/include/xenguest.h
@@ -30,6 +30,7 @@
 #define XCFLAGS_HVM       (1 << 2)
 #define XCFLAGS_STDVGA    (1 << 3)
 #define XCFLAGS_CHECKPOINT_COMPRESS    (1 << 4)
+#define XCFLAGS_CHECKPOINTED    (1 << 5)
 
 #define X86_64_B_SIZE   64 
 #define X86_32_B_SIZE   32
diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index c4fe92c..c0f90d4 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -174,6 +174,9 @@ struct xc_sr_context
             /* Live migrate vs non live suspend. */
             bool live;
 
+            /* Plain VM, or checkpoints over time. */
+            bool checkpointed;
+
             /* Further debugging information in the stream. */
             bool debug;
 
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index 66fcd3e..caa727d 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -732,6 +732,7 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom,
     ctx.save.callbacks = callbacks;
     ctx.save.live  = !!(flags & XCFLAGS_LIVE);
     ctx.save.debug = !!(flags & XCFLAGS_DEBUG);
+    ctx.save.checkpointed = !!(flags & XCFLAGS_CHECKPOINTED);
 
     /*
      * TODO: Find some time to better tweak the live migration algorithm.
@@ -745,6 +746,8 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom,
     /* Sanity checks for callbacks. */
     if ( hvm )
         assert(callbacks->switch_qemu_logdirty);
+    if ( ctx.save.checkpointed )
+        assert(callbacks->checkpoint && callbacks->postcopy);
 
     IPRINTF("In experimental %s", __func__);
     DPRINTF("fd %d, dom %u, max_iters %u, max_factor %u, flags %u, hvm %d",
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index f408646..a0c9850 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -2003,6 +2003,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
 
     if (r_info != NULL) {
         dss->interval = r_info->interval;
+        dss->xcflags |= XCFLAGS_CHECKPOINTED;
         if (libxl_defbool_val(r_info->compression))
             dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;
     }
-- 
1.9.1

[PATCH v5 05/14] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW

[Yang Hongyang]

There are cases that we only need to use the hypercall buffer data,
and do not use the xc_hypercall_buffer_t struct.
DECLARE_HYPERCALL_BUFFER_SHADOW define a user pointer that can allow
us to access the hypercall buffer data but it also define a
xc_hypercall_buffer_t that we don't use, the compiler will report arg
unused error.
add __attribute__((unused)) before xc_hypercall_buffer_t to avoid
the compiler error.

The cases for example:
In send_all_pages(), we only need to use the haypercall buffer data
which is a dirty bitmap, we set the dirty bitmap to all dirty and call
send_dirty_pages, we will not use the xc_hypercall_buffer_t and hypercall
to retrive the dirty bitmap.
In send_some_pages(), we will also only need to use the dirty_bitmap.
the retrive dirty bitmap hypercall are done by the caller.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/libxc/include/xenctrl.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index a689caf..0804257 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -284,11 +284,15 @@ typedef struct xc_hypercall_buffer xc_hypercall_buffer_t;
  * Useful when a hypercall buffer is passed to a function and access
  * via the user pointer is required.
  *
+ * The shadow xc_hypercall_buffer_t may be unused, add
+ * __attribute__((unused)) to avoid compiler error.
+ *
  * See DECLARE_HYPERCALL_BUFFER_ARGUMENT() if the user pointer is not
  * required.
  */
 #define DECLARE_HYPERCALL_BUFFER_SHADOW(_type, _name, _hbuf)   \
     _type *(_name) = (_hbuf)->hbuf;                            \
+    __attribute__((unused))                                    \
     xc_hypercall_buffer_t XC__HYPERCALL_BUFFER_NAME(_name) = { \
         .hbuf = (void *)-1,                                    \
         .param_shadow = (_hbuf),                               \
-- 
1.9.1

[PATCH v5 06/14] tools/libxc: add a check in xc_hypercall_buffer_free_pages macro

[Yang Hongyang]

When we use a DECLARE_HYPERCALL_BUFFER_SHADOW define a user
pointer '_name' and a shadow xc_hypercall_buffer_t.
then call xc_hypercall_buffer_free_pages(_xch, _name, _nr),
the complier will report '_name' unused error, it's because
xc_hypercall_buffer_free_pages() is a MACRO and '_name'
transparently converted to the hypercall buffer. it confused
the caller because xc_hypercall_buffer_free_pages() do look
like a function and take '_name' as an arg.
Add an if check to let the compiler think we are actually
using the argument '_name'.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/libxc/include/xenctrl.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 0804257..202f612 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -353,7 +353,12 @@ void xc__hypercall_buffer_free(xc_interface *xch, xc_hypercall_buffer_t *b);
 void *xc__hypercall_buffer_alloc_pages(xc_interface *xch, xc_hypercall_buffer_t *b, int nr_pages);
 #define xc_hypercall_buffer_alloc_pages(_xch, _name, _nr) xc__hypercall_buffer_alloc_pages(_xch, HYPERCALL_BUFFER(_name), _nr)
 void xc__hypercall_buffer_free_pages(xc_interface *xch, xc_hypercall_buffer_t *b, int nr_pages);
-#define xc_hypercall_buffer_free_pages(_xch, _name, _nr) xc__hypercall_buffer_free_pages(_xch, HYPERCALL_BUFFER(_name), _nr)
+#define xc_hypercall_buffer_free_pages(_xch, _name, _nr)                    \
+    do {                                                                    \
+        if ( _name )                                                        \
+            xc__hypercall_buffer_free_pages(_xch, HYPERCALL_BUFFER(_name),  \
+                                            _nr);                           \
+    } while (0)
 
 /*
  * Array of hypercall buffers.
-- 
1.9.1

[PATCH v5 07/14] libxc/save: introduce setup() and cleanup() on save

[Yang Hongyang]

introduce setup() and cleanup() which subsume the
ctx->save.ops.{setup,cleanup}() calls.
The SHADOW_OP_OFF hypercall also included in the cleanup().

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-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>
---
 tools/libxc/xc_sr_save.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index caa727d..f4ab5c5 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -637,6 +637,31 @@ static int send_domain_memory_nonlive(struct xc_sr_context *ctx)
     return rc;
 }
 
+static int setup(struct xc_sr_context *ctx)
+{
+    int rc;
+
+    rc = ctx->save.ops.setup(ctx);
+    if ( rc )
+        goto err;
+
+    rc = 0;
+
+ err:
+    return rc;
+}
+
+static void cleanup(struct xc_sr_context *ctx)
+{
+    xc_interface *xch = ctx->xch;
+
+    xc_shadow_control(xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_OFF,
+                      NULL, 0, NULL, 0, NULL);
+
+    if ( ctx->save.ops.cleanup(ctx) )
+        PERROR("Failed to clean up");
+}
+
 /*
  * Save a domain.
  */
@@ -648,7 +673,7 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type)
     IPRINTF("Saving domain %d, type %s",
             ctx->domid, dhdr_type_to_str(guest_type));
 
-    rc = ctx->save.ops.setup(ctx);
+    rc = setup(ctx);
     if ( rc )
         goto err;
 
@@ -701,12 +726,7 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type)
     PERROR("Save failed");
 
  done:
-    xc_shadow_control(xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_OFF,
-                      NULL, 0, NULL, 0, NULL);
-
-    rc = ctx->save.ops.cleanup(ctx);
-    if ( rc )
-        PERROR("Failed to clean up");
+    cleanup(ctx);
 
     if ( saved_rc )
     {
-- 
1.9.1

[PATCH v5 08/14] libxc/save: rename to_send to dirty_bitmap

[Yang Hongyang]

rename to_send to dirty_bitmap.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-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>
---
 tools/libxc/xc_sr_save.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index f4ab5c5..5d08620 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -475,20 +475,20 @@ static int update_progress_string(struct xc_sr_context *ctx,
 static int send_domain_memory_live(struct xc_sr_context *ctx)
 {
     xc_interface *xch = ctx->xch;
-    DECLARE_HYPERCALL_BUFFER(unsigned long, to_send);
+    DECLARE_HYPERCALL_BUFFER(unsigned long, dirty_bitmap);
     xc_shadow_op_stats_t stats = { 0, ctx->save.p2m_size };
     char *progress_str = NULL;
     unsigned x;
     int rc = -1;
 
-    to_send = xc_hypercall_buffer_alloc_pages(
-        xch, to_send, NRPAGES(bitmap_size(ctx->save.p2m_size)));
+    dirty_bitmap = xc_hypercall_buffer_alloc_pages(
+        xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->save.p2m_size)));
 
     ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE *
                                   sizeof(*ctx->save.batch_pfns));
     ctx->save.deferred_pages = calloc(1, bitmap_size(ctx->save.p2m_size));
 
-    if ( !ctx->save.batch_pfns || !to_send || !ctx->save.deferred_pages )
+    if ( !ctx->save.batch_pfns || !dirty_bitmap || !ctx->save.deferred_pages )
     {
         ERROR("Unable to allocate memory for to_{send,fix}/batch bitmaps");
         goto out;
@@ -512,7 +512,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
     {
         if ( xc_shadow_control(
                  xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_CLEAN,
-                 HYPERCALL_BUFFER(to_send), ctx->save.p2m_size,
+                 HYPERCALL_BUFFER(dirty_bitmap), ctx->save.p2m_size,
                  NULL, 0, &stats) != ctx->save.p2m_size )
         {
             PERROR("Failed to retrieve logdirty bitmap");
@@ -527,7 +527,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
         if ( rc )
             goto out;
 
-        rc = send_some_pages(ctx, to_send, stats.dirty_count);
+        rc = send_some_pages(ctx, dirty_bitmap, stats.dirty_count);
         if ( rc )
             goto out;
     }
@@ -538,7 +538,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
 
     if ( xc_shadow_control(
              xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_CLEAN,
-             HYPERCALL_BUFFER(to_send), ctx->save.p2m_size,
+             HYPERCALL_BUFFER(dirty_bitmap), ctx->save.p2m_size,
              NULL, 0, &stats) != ctx->save.p2m_size )
     {
         PERROR("Failed to retrieve logdirty bitmap");
@@ -550,9 +550,9 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
     if ( rc )
         goto out;
 
-    bitmap_or(to_send, ctx->save.deferred_pages, ctx->save.p2m_size);
+    bitmap_or(dirty_bitmap, ctx->save.deferred_pages, ctx->save.p2m_size);
 
-    rc = send_some_pages(ctx, to_send,
+    rc = send_some_pages(ctx, dirty_bitmap,
                          stats.dirty_count + ctx->save.nr_deferred_pages);
     if ( rc )
         goto out;
@@ -578,7 +578,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
 
         if ( xc_shadow_control(
                  xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_PEEK,
-                 HYPERCALL_BUFFER(to_send), ctx->save.p2m_size,
+                 HYPERCALL_BUFFER(dirty_bitmap), ctx->save.p2m_size,
                  NULL, 0, &stats) != ctx->save.p2m_size )
         {
             PERROR("Failed to retrieve logdirty bitmap");
@@ -593,7 +593,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
   out:
     xc_set_progress_prefix(xch, NULL);
     free(progress_str);
-    xc_hypercall_buffer_free_pages(xch, to_send,
+    xc_hypercall_buffer_free_pages(xch, dirty_bitmap,
                                    NRPAGES(bitmap_size(ctx->save.p2m_size)));
     free(ctx->save.deferred_pages);
     free(ctx->save.batch_pfns);
-- 
1.9.1

[PATCH v5 09/14] libxc/save: adjust the memory allocation for migration

[Yang Hongyang]

Move the memory allocation before the concrete live/nolive save
in order to avoid the free/alloc memory loop when using Remus.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-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>
---
 tools/libxc/xc_sr_common.h |  1 +
 tools/libxc/xc_sr_save.c   | 60 ++++++++++++++++++++++------------------------
 2 files changed, 29 insertions(+), 32 deletions(-)

diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index c0f90d4..276d00a 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -190,6 +190,7 @@ struct xc_sr_context
             unsigned nr_batch_pfns;
             unsigned long *deferred_pages;
             unsigned long nr_deferred_pages;
+            xc_hypercall_buffer_t dirty_bitmap_hbuf;
         } save;
 
         struct /* Restore data. */
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index 5d08620..beb54c4 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -475,24 +475,12 @@ static int update_progress_string(struct xc_sr_context *ctx,
 static int send_domain_memory_live(struct xc_sr_context *ctx)
 {
     xc_interface *xch = ctx->xch;
-    DECLARE_HYPERCALL_BUFFER(unsigned long, dirty_bitmap);
     xc_shadow_op_stats_t stats = { 0, ctx->save.p2m_size };
     char *progress_str = NULL;
     unsigned x;
     int rc = -1;
-
-    dirty_bitmap = xc_hypercall_buffer_alloc_pages(
-        xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->save.p2m_size)));
-
-    ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE *
-                                  sizeof(*ctx->save.batch_pfns));
-    ctx->save.deferred_pages = calloc(1, bitmap_size(ctx->save.p2m_size));
-
-    if ( !ctx->save.batch_pfns || !dirty_bitmap || !ctx->save.deferred_pages )
-    {
-        ERROR("Unable to allocate memory for to_{send,fix}/batch bitmaps");
-        goto out;
-    }
+    DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
+                                    &ctx->save.dirty_bitmap_hbuf);
 
     rc = enable_logdirty(ctx);
     if ( rc )
@@ -593,10 +581,6 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
   out:
     xc_set_progress_prefix(xch, NULL);
     free(progress_str);
-    xc_hypercall_buffer_free_pages(xch, dirty_bitmap,
-                                   NRPAGES(bitmap_size(ctx->save.p2m_size)));
-    free(ctx->save.deferred_pages);
-    free(ctx->save.batch_pfns);
     return rc;
 }
 
@@ -609,17 +593,6 @@ static int send_domain_memory_nonlive(struct xc_sr_context *ctx)
     xc_interface *xch = ctx->xch;
     int rc = -1;
 
-    ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE *
-                                  sizeof(*ctx->save.batch_pfns));
-    ctx->save.deferred_pages = calloc(1, bitmap_size(ctx->save.p2m_size));
-
-    if ( !ctx->save.batch_pfns || !ctx->save.deferred_pages )
-    {
-        PERROR("Failed to allocate memory for nonlive tracking structures");
-        errno = ENOMEM;
-        goto err;
-    }
-
     rc = suspend_domain(ctx);
     if ( rc )
         goto err;
@@ -631,15 +604,30 @@ static int send_domain_memory_nonlive(struct xc_sr_context *ctx)
         goto err;
 
  err:
-    free(ctx->save.deferred_pages);
-    free(ctx->save.batch_pfns);
-
     return rc;
 }
 
 static int setup(struct xc_sr_context *ctx)
 {
+    xc_interface *xch = ctx->xch;
     int rc;
+    DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
+                                    &ctx->save.dirty_bitmap_hbuf);
+
+    dirty_bitmap = xc_hypercall_buffer_alloc_pages(
+                   xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->save.p2m_size)));
+    ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE *
+                                  sizeof(*ctx->save.batch_pfns));
+    ctx->save.deferred_pages = calloc(1, bitmap_size(ctx->save.p2m_size));
+
+    if ( !ctx->save.batch_pfns || !dirty_bitmap || !ctx->save.deferred_pages )
+    {
+        ERROR("Unable to allocate memory for dirty bitmaps, batch pfns and"
+              " deferred pages");
+        rc = -1;
+        errno = ENOMEM;
+        goto err;
+    }
 
     rc = ctx->save.ops.setup(ctx);
     if ( rc )
@@ -654,12 +642,20 @@ static int setup(struct xc_sr_context *ctx)
 static void cleanup(struct xc_sr_context *ctx)
 {
     xc_interface *xch = ctx->xch;
+    DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
+                                    &ctx->save.dirty_bitmap_hbuf);
+
 
     xc_shadow_control(xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_OFF,
                       NULL, 0, NULL, 0, NULL);
 
     if ( ctx->save.ops.cleanup(ctx) )
         PERROR("Failed to clean up");
+
+    xc_hypercall_buffer_free_pages(xch, dirty_bitmap,
+                                   NRPAGES(bitmap_size(ctx->save.p2m_size)));
+    free(ctx->save.deferred_pages);
+    free(ctx->save.batch_pfns);
 }
 
 /*
-- 
1.9.1

[PATCH v5 10/14] libxc/save: remove bitmap param from send_some_pages

[Yang Hongyang]

In last patch we added dirty bitmap to the save context,
we no longer need to pass this param to send_some_pages.
We can get dirty bitmap from the save context.
'entries' should stay as it is a useful sanity check.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-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>
---
 tools/libxc/xc_sr_save.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index beb54c4..adb5cce 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -374,23 +374,24 @@ static int send_all_pages(struct xc_sr_context *ctx)
 }
 
 /*
- * Send a subset of pages in the guests p2m, according to the provided bitmap.
+ * Send a subset of pages in the guests p2m, according to the dirty bitmap.
  * Used for each subsequent iteration of the live migration loop.
  *
  * Bitmap is bounded by p2m_size.
  */
 static int send_some_pages(struct xc_sr_context *ctx,
-                           unsigned long *bitmap,
                            unsigned long entries)
 {
     xc_interface *xch = ctx->xch;
     xen_pfn_t p;
     unsigned long written;
     int rc;
+    DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
+                                    &ctx->save.dirty_bitmap_hbuf);
 
     for ( p = 0, written = 0; p < ctx->save.p2m_size; ++p )
     {
-        if ( !test_bit(p, bitmap) )
+        if ( !test_bit(p, dirty_bitmap) )
             continue;
 
         rc = add_to_batch(ctx, p);
@@ -515,7 +516,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
         if ( rc )
             goto out;
 
-        rc = send_some_pages(ctx, dirty_bitmap, stats.dirty_count);
+        rc = send_some_pages(ctx, stats.dirty_count);
         if ( rc )
             goto out;
     }
@@ -540,8 +541,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
 
     bitmap_or(dirty_bitmap, ctx->save.deferred_pages, ctx->save.p2m_size);
 
-    rc = send_some_pages(ctx, dirty_bitmap,
-                         stats.dirty_count + ctx->save.nr_deferred_pages);
+    rc = send_some_pages(ctx, stats.dirty_count + ctx->save.nr_deferred_pages);
     if ( rc )
         goto out;
 
-- 
1.9.1

[PATCH v5 11/14] libxc/save: rename send_some_pages to send_dirty_pages

[Yang Hongyang]

rename send_some_pages to send_dirty_pages, no functional change.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/libxc/xc_sr_save.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index adb5cce..3768bda 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -379,8 +379,8 @@ static int send_all_pages(struct xc_sr_context *ctx)
  *
  * Bitmap is bounded by p2m_size.
  */
-static int send_some_pages(struct xc_sr_context *ctx,
-                           unsigned long entries)
+static int send_dirty_pages(struct xc_sr_context *ctx,
+                            unsigned long entries)
 {
     xc_interface *xch = ctx->xch;
     xen_pfn_t p;
@@ -516,7 +516,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
         if ( rc )
             goto out;
 
-        rc = send_some_pages(ctx, stats.dirty_count);
+        rc = send_dirty_pages(ctx, stats.dirty_count);
         if ( rc )
             goto out;
     }
@@ -541,7 +541,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
 
     bitmap_or(dirty_bitmap, ctx->save.deferred_pages, ctx->save.p2m_size);
 
-    rc = send_some_pages(ctx, stats.dirty_count + ctx->save.nr_deferred_pages);
+    rc = send_dirty_pages(ctx, stats.dirty_count + ctx->save.nr_deferred_pages);
     if ( rc )
         goto out;
 
-- 
1.9.1

[PATCH v5 12/14] libxc/save: reuse send_dirty_pages() in send_all_pages()

[Yang Hongyang]

introduce bitmap_set() to set the entire bitmap.
in send_all_pages(), set the entire bitmap and call send_dirty_pages().

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-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>
---
 tools/libxc/xc_bitops.h  |  5 +++++
 tools/libxc/xc_sr_save.c | 44 ++++++++++++++------------------------------
 2 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/tools/libxc/xc_bitops.h b/tools/libxc/xc_bitops.h
index dfce3b8..cd749f4 100644
--- a/tools/libxc/xc_bitops.h
+++ b/tools/libxc/xc_bitops.h
@@ -26,6 +26,11 @@ static inline unsigned long *bitmap_alloc(int nr_bits)
     return calloc(1, bitmap_size(nr_bits));
 }
 
+static inline void bitmap_set(unsigned long *addr, int nr_bits)
+{
+    memset(addr, 0xff, bitmap_size(nr_bits));
+}
+
 static inline void bitmap_clear(unsigned long *addr, int nr_bits)
 {
     memset(addr, 0, bitmap_size(nr_bits));
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index 3768bda..1d0a46d 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -344,36 +344,6 @@ static int suspend_domain(struct xc_sr_context *ctx)
 }
 
 /*
- * Send all pages in the guests p2m.  Used as the first iteration of the live
- * migration loop, and for a non-live save.
- */
-static int send_all_pages(struct xc_sr_context *ctx)
-{
-    xc_interface *xch = ctx->xch;
-    xen_pfn_t p;
-    int rc;
-
-    for ( p = 0; p < ctx->save.p2m_size; ++p )
-    {
-        rc = add_to_batch(ctx, p);
-        if ( rc )
-            return rc;
-
-        /* Update progress every 4MB worth of memory sent. */
-        if ( (p & ((1U << (22 - 12)) - 1)) == 0 )
-            xc_report_progress_step(xch, p, ctx->save.p2m_size);
-    }
-
-    rc = flush_batch(ctx);
-    if ( rc )
-        return rc;
-
-    xc_report_progress_step(xch, ctx->save.p2m_size,
-                            ctx->save.p2m_size);
-    return 0;
-}
-
-/*
  * Send a subset of pages in the guests p2m, according to the dirty bitmap.
  * Used for each subsequent iteration of the live migration loop.
  *
@@ -416,6 +386,20 @@ static int send_dirty_pages(struct xc_sr_context *ctx,
     return 0;
 }
 
+/*
+ * Send all pages in the guests p2m.  Used as the first iteration of the live
+ * migration loop, and for a non-live save.
+ */
+static int send_all_pages(struct xc_sr_context *ctx)
+{
+    DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
+                                    &ctx->save.dirty_bitmap_hbuf);
+
+    bitmap_set(dirty_bitmap, ctx->save.p2m_size);
+
+    return send_dirty_pages(ctx, ctx->save.p2m_size);
+}
+
 static int enable_logdirty(struct xc_sr_context *ctx)
 {
     xc_interface *xch = ctx->xch;
-- 
1.9.1

[PATCH v5 13/14] libxc/restore: introduce process_record()

[Yang Hongyang]

Move record handle codes into a function process_record().
It will be used multiple times by Remus.
No functional change.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-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>
---
 tools/libxc/xc_sr_restore.c | 77 +++++++++++++++++++++++++--------------------
 1 file changed, 43 insertions(+), 34 deletions(-)

diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
index 0bf4bae..53bd674 100644
--- a/tools/libxc/xc_sr_restore.c
+++ b/tools/libxc/xc_sr_restore.c
@@ -468,6 +468,48 @@ static int handle_page_data(struct xc_sr_context *ctx, struct xc_sr_record *rec)
     return rc;
 }
 
+static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec)
+{
+    xc_interface *xch = ctx->xch;
+    int rc = 0;
+
+    switch ( rec->type )
+    {
+    case REC_TYPE_END:
+        break;
+
+    case REC_TYPE_PAGE_DATA:
+        rc = handle_page_data(ctx, rec);
+        break;
+
+    case REC_TYPE_VERIFY:
+        DPRINTF("Verify mode enabled");
+        ctx->restore.verify = true;
+        break;
+
+    default:
+        rc = ctx->restore.ops.process_record(ctx, rec);
+        break;
+    }
+
+    free(rec->data);
+
+    if ( rc == RECORD_NOT_PROCESSED )
+    {
+        if ( rec->type & REC_TYPE_OPTIONAL )
+            DPRINTF("Ignoring optional record %#x (%s)",
+                    rec->type, rec_type_to_str(rec->type));
+        else
+        {
+            ERROR("Mandatory record %#x (%s) not handled",
+                  rec->type, rec_type_to_str(rec->type));
+            rc = -1;
+        }
+    }
+
+    return rc;
+}
+
 /*
  * Restore a domain.
  */
@@ -498,40 +540,7 @@ static int restore(struct xc_sr_context *ctx)
         if ( rc )
             goto err;
 
-        switch ( rec.type )
-        {
-        case REC_TYPE_END:
-            break;
-
-        case REC_TYPE_PAGE_DATA:
-            rc = handle_page_data(ctx, &rec);
-            break;
-
-        case REC_TYPE_VERIFY:
-            DPRINTF("Verify mode enabled");
-            ctx->restore.verify = true;
-            break;
-
-        default:
-            rc = ctx->restore.ops.process_record(ctx, &rec);
-            break;
-        }
-
-        free(rec.data);
-
-        if ( rc == RECORD_NOT_PROCESSED )
-        {
-            if ( rec.type & REC_TYPE_OPTIONAL )
-                DPRINTF("Ignoring optional record %#x (%s)",
-                        rec.type, rec_type_to_str(rec.type));
-            else
-            {
-                ERROR("Mandatory record %#x (%s) not handled",
-                      rec.type, rec_type_to_str(rec.type));
-                rc = -1;
-            }
-        }
-
+        rc = process_record(ctx, &rec);
         if ( rc )
             goto err;
 
-- 
1.9.1

[PATCH v5 14/14] libxc/restore: split read/handle qemu info

[Yang Hongyang]

Split read/handle qemu info. The receiving of qemu info
should be done while we receive the migration stream,
handle_qemu will be called when the stream complete.
Otherwise, it will break Remus because read_record()
won't read qemu info and stream_complete will be called
at failover.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/libxc/xc_sr_common.h          |  5 +++++
 tools/libxc/xc_sr_restore.c         | 12 ++++++++++++
 tools/libxc/xc_sr_restore_x86_hvm.c | 28 +++++++++++++++++++++++++---
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index 276d00a..0ba9728 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -288,6 +288,11 @@ struct xc_sr_context
                     /* HVM context blob. */
                     void *context;
                     size_t contextsz;
+
+#ifdef XG_LIBXL_HVM_COMPAT
+                    uint32_t qlen;
+                    void *qbuf;
+#endif
                 } restore;
             };
         } x86_hvm;
diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
index 53bd674..8022c3d 100644
--- a/tools/libxc/xc_sr_restore.c
+++ b/tools/libxc/xc_sr_restore.c
@@ -510,6 +510,9 @@ static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec)
     return rc;
 }
 
+#ifdef XG_LIBXL_HVM_COMPAT
+extern int read_qemu(struct xc_sr_context *ctx);
+#endif
 /*
  * Restore a domain.
  */
@@ -546,6 +549,15 @@ static int restore(struct xc_sr_context *ctx)
 
     } while ( rec.type != REC_TYPE_END );
 
+#ifdef XG_LIBXL_HVM_COMPAT
+    if ( ctx->dominfo.hvm )
+    {
+        rc = read_qemu(ctx);
+        if ( rc )
+            goto err;
+    }
+#endif
+
     rc = ctx->restore.ops.stream_complete(ctx);
     if ( rc )
         goto err;
diff --git a/tools/libxc/xc_sr_restore_x86_hvm.c b/tools/libxc/xc_sr_restore_x86_hvm.c
index 6e9b318..6f5af0e 100644
--- a/tools/libxc/xc_sr_restore_x86_hvm.c
+++ b/tools/libxc/xc_sr_restore_x86_hvm.c
@@ -94,14 +94,14 @@ static int handle_hvm_params(struct xc_sr_context *ctx,
 }
 
 #ifdef XG_LIBXL_HVM_COMPAT
-static int handle_qemu(struct xc_sr_context *ctx)
+int read_qemu(struct xc_sr_context *ctx);
+int read_qemu(struct xc_sr_context *ctx)
 {
     xc_interface *xch = ctx->xch;
-    char qemusig[21], path[256];
+    char qemusig[21];
     uint32_t qlen;
     void *qbuf = NULL;
     int rc = -1;
-    FILE *fp = NULL;
 
     if ( read_exact(ctx->fd, qemusig, sizeof(qemusig)) )
     {
@@ -137,6 +137,28 @@ static int handle_qemu(struct xc_sr_context *ctx)
         goto out;
     }
 
+    /* With Remus, this could be read many times */
+    if ( ctx->x86_hvm.restore.qbuf )
+        free(ctx->x86_hvm.restore.qbuf);
+    ctx->x86_hvm.restore.qbuf = qbuf;
+    ctx->x86_hvm.restore.qlen = qlen;
+    rc = 0;
+
+out:
+    if (rc)
+        free(qbuf);
+    return rc;
+}
+
+static int handle_qemu(struct xc_sr_context *ctx)
+{
+    xc_interface *xch = ctx->xch;
+    char path[256];
+    uint32_t qlen = ctx->x86_hvm.restore.qlen;
+    void *qbuf = ctx->x86_hvm.restore.qbuf;
+    int rc = -1;
+    FILE *fp = NULL;
+
     sprintf(path, XC_DEVICE_MODEL_RESTORE_FILE".%u", ctx->domid);
     fp = fopen(path, "wb");
     if ( !fp )
-- 
1.9.1

Re: [PATCH v5 00/14] Misc patches to aid migration v2 Remus support

[Andrew Cooper]

On 13/05/2015 02:53, Yang Hongyang wrote:
> This is the combination of Andrew Cooper's misc patches and mine
> to aid migration v2 Remus support.

Everything looks in order now from my  point of view!

~Andrew

>
> See individual patches for details.
>
> Git tree available at:
>     https://github.com/macrosheep/xen/tree/misc-remus-v5
>
> v4->v5
> Most of the changes are trival, like drop brackets in
> DECLARE_HYPERCALL_BUFFER_SHADOW(), drop stray blank line etc.
> Except the 6th patch which add a do{}while (0) in
> xc_hypercall_buffer_free_pages macro.
>
> Summary of changes:
> M = modified
> A = acked
> no mark = unchanged from last round
>
> Andrew Cooper (4):
>    libxc/migration: Be rather stricter with illformed callers
>    libxc/save: Adjust stream-position callbacks for checkpointed streams
> M  libxc/migration: Specification update for CHECKPOINT records
>    libxc/migration: Pass checkpoint information into the save algorithm.
>
> Yang Hongyang (10):
>    tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW
> M  tools/libxc: add a check in xc_hypercall_buffer_free_pages macro
> M  libxc/save: introduce setup() and cleanup() on save
>    libxc/save: rename to_send to dirty_bitmap
> M  libxc/save: adjust the memory allocation for migration
> M  libxc/save: remove bitmap param from send_some_pages
>    libxc/save: rename send_some_pages to send_dirty_pages
> M  libxc/save: reuse send_dirty_pages() in send_all_pages()
>    libxc/restore: introduce process_record()
>    libxc/restore: split read/handle qemu info
>
>  docs/specs/libxc-migration-stream.pandoc |  29 +++++-
>  tools/libxc/include/xenctrl.h            |  11 +-
>  tools/libxc/include/xenguest.h           |   1 +
>  tools/libxc/xc_bitops.h                  |   5 +
>  tools/libxc/xc_sr_common.c               |   1 +
>  tools/libxc/xc_sr_common.h               |  30 ++++--
>  tools/libxc/xc_sr_restore.c              |  89 ++++++++++------
>  tools/libxc/xc_sr_restore_x86_hvm.c      |  28 ++++-
>  tools/libxc/xc_sr_save.c                 | 171 ++++++++++++++++---------------
>  tools/libxc/xc_sr_save_x86_hvm.c         |  30 +++---
>  tools/libxc/xc_sr_save_x86_pv.c          |  29 ++++--
>  tools/libxc/xc_sr_stream_format.h        |   1 +
>  tools/libxl/libxl_dom.c                  |   1 +
>  13 files changed, 272 insertions(+), 154 deletions(-)
>

Re: [PATCH v5 01/14] libxc/migration: Be rather stricter with illformed callers

[Ian Campbell]

On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> The migration code itself should be able to validly assume all mandatory
> callbacks are set up.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Ian Campbell <Ian.Campbell@citrix.com>

Re: [PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams

[Ian Campbell]

On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> 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>

Subject to my finding a suitable documentation update further down the
series:

Acked-by: Ian Campbell <Ian.Campbell@citrix.com>

Re: [PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams

[Andrew Cooper]

On 13/05/15 16:46, Ian Campbell wrote:
> On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
>> From: Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> 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>
> Subject to my finding a suitable documentation update further down the
> series:
>
> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>

What update are you expecting?  X86_PV_INFO is already strictly defined
as "sent once" in the spec.

~Andrew

Re: [PATCH v5 03/14] libxc/migration: Specification update for CHECKPOINT records

[Ian Campbell]

On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Checkpointed streams need to signal the end of a consistent view of VM state,
> and the start of the libxl data.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>

Acked-by: Ian Campbell <Ian.Campbell@citrix.com>

Re: [PATCH v5 04/14] libxc/migration: Pass checkpoint information into the save algorithm.

[Ian Campbell]

On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>

OOI how was this signalled to the old code?

> 
> 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>
> ---
>  tools/libxc/include/xenguest.h | 1 +
>  tools/libxc/xc_sr_common.h     | 3 +++
>  tools/libxc/xc_sr_save.c       | 3 +++
>  tools/libxl/libxl_dom.c        | 1 +
>  4 files changed, 8 insertions(+)
> 
> diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
> index 8e39075..7581263 100644
> --- a/tools/libxc/include/xenguest.h
> +++ b/tools/libxc/include/xenguest.h
> @@ -30,6 +30,7 @@
>  #define XCFLAGS_HVM       (1 << 2)
>  #define XCFLAGS_STDVGA    (1 << 3)
>  #define XCFLAGS_CHECKPOINT_COMPRESS    (1 << 4)
> +#define XCFLAGS_CHECKPOINTED    (1 << 5)
>  
>  #define X86_64_B_SIZE   64 
>  #define X86_32_B_SIZE   32
> diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
> index c4fe92c..c0f90d4 100644
> --- a/tools/libxc/xc_sr_common.h
> +++ b/tools/libxc/xc_sr_common.h
> @@ -174,6 +174,9 @@ struct xc_sr_context
>              /* Live migrate vs non live suspend. */
>              bool live;
>  
> +            /* Plain VM, or checkpoints over time. */
> +            bool checkpointed;
> +
>              /* Further debugging information in the stream. */
>              bool debug;
>  
> diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
> index 66fcd3e..caa727d 100644
> --- a/tools/libxc/xc_sr_save.c
> +++ b/tools/libxc/xc_sr_save.c
> @@ -732,6 +732,7 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom,
>      ctx.save.callbacks = callbacks;
>      ctx.save.live  = !!(flags & XCFLAGS_LIVE);
>      ctx.save.debug = !!(flags & XCFLAGS_DEBUG);
> +    ctx.save.checkpointed = !!(flags & XCFLAGS_CHECKPOINTED);
>  
>      /*
>       * TODO: Find some time to better tweak the live migration algorithm.
> @@ -745,6 +746,8 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom,
>      /* Sanity checks for callbacks. */
>      if ( hvm )
>          assert(callbacks->switch_qemu_logdirty);
> +    if ( ctx.save.checkpointed )
> +        assert(callbacks->checkpoint && callbacks->postcopy);
>  
>      IPRINTF("In experimental %s", __func__);
>      DPRINTF("fd %d, dom %u, max_iters %u, max_factor %u, flags %u, hvm %d",
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index f408646..a0c9850 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -2003,6 +2003,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
>  
>      if (r_info != NULL) {
>          dss->interval = r_info->interval;
> +        dss->xcflags |= XCFLAGS_CHECKPOINTED;
>          if (libxl_defbool_val(r_info->compression))
>              dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;
>      }

Re: [PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams

[Ian Campbell]

On Wed, 2015-05-13 at 16:47 +0100, Andrew Cooper wrote:
> On 13/05/15 16:46, Ian Campbell wrote:
> > On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> >> From: Andrew Cooper <andrew.cooper3@citrix.com>
> >>
> >> 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>
> > Subject to my finding a suitable documentation update further down the
> > series:
> >
> > Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
> 
> What update are you expecting?  X86_PV_INFO is already strictly defined
> as "sent once" in the spec.

Something defining a checkpointed stread.

Re: [PATCH v5 05/14] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW

[Ian Campbell]

On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> There are cases that we only need to use the hypercall buffer data,

"cases where"
> and do not use the xc_hypercall_buffer_t struct.
> DECLARE_HYPERCALL_BUFFER_SHADOW define a user pointer that can allow

"defines a user pointer"

> us to access the hypercall buffer data but it also define a

"defines"

> xc_hypercall_buffer_t that we don't use, the compiler will report arg
> unused error.
> add __attribute__((unused)) before xc_hypercall_buffer_t to avoid
> the compiler error.
> 
> The cases for example:

"Example cases:"

> In send_all_pages(), we only need to use the haypercall buffer data

"hypercall"

> which is a dirty bitmap, we set the dirty bitmap to all dirty and call
> send_dirty_pages, we will not use the xc_hypercall_buffer_t and hypercall
> to retrive the dirty bitmap.

"retrieve"

> In send_some_pages(), we will also only need to use the dirty_bitmap.
> the retrive dirty bitmap hypercall are done by the caller.

and again.

> 
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  tools/libxc/include/xenctrl.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index a689caf..0804257 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -284,11 +284,15 @@ typedef struct xc_hypercall_buffer xc_hypercall_buffer_t;
>   * Useful when a hypercall buffer is passed to a function and access
>   * via the user pointer is required.
>   *
> + * The shadow xc_hypercall_buffer_t may be unused, add
> + * __attribute__((unused)) to avoid compiler error.

No need for this comment IMHO.

>   * See DECLARE_HYPERCALL_BUFFER_ARGUMENT() if the user pointer is not
>   * required.
>   */
>  #define DECLARE_HYPERCALL_BUFFER_SHADOW(_type, _name, _hbuf)   \
>      _type *(_name) = (_hbuf)->hbuf;                            \
> +    __attribute__((unused))                                    \

This is a somewhat unconventional location for such a tagm but putting
it elsewhere would involve faff-some rewrapping.

So apart from the typos looks good.


>      xc_hypercall_buffer_t XC__HYPERCALL_BUFFER_NAME(_name) = { \
>          .hbuf = (void *)-1,                                    \
>          .param_shadow = (_hbuf),                               \

Re: [PATCH v5 04/14] libxc/migration: Pass checkpoint information into the save algorithm.

[Yang Hongyang]

On 05/13/2015 11:49 PM, Ian Campbell wrote:
> On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
>> From: Andrew Cooper <andrew.cooper3@citrix.com>
>
> OOI how was this signalled to the old code?

The old code check the callbacks "postcopy & checkpoint",
if the callbacks exists, it will call them which I think is
unreliable, so I add this flag to explicitly indicate a
checkpointed stream in the new code. However, it is backward
compatible, the legacy migration just don't know this flag
and will ignore it.

>
>>
>> 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>
>> ---
>>   tools/libxc/include/xenguest.h | 1 +
>>   tools/libxc/xc_sr_common.h     | 3 +++
>>   tools/libxc/xc_sr_save.c       | 3 +++
>>   tools/libxl/libxl_dom.c        | 1 +
>>   4 files changed, 8 insertions(+)
>>
>> diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
>> index 8e39075..7581263 100644
>> --- a/tools/libxc/include/xenguest.h
>> +++ b/tools/libxc/include/xenguest.h
>> @@ -30,6 +30,7 @@
>>   #define XCFLAGS_HVM       (1 << 2)
>>   #define XCFLAGS_STDVGA    (1 << 3)
>>   #define XCFLAGS_CHECKPOINT_COMPRESS    (1 << 4)
>> +#define XCFLAGS_CHECKPOINTED    (1 << 5)
>>
>>   #define X86_64_B_SIZE   64
>>   #define X86_32_B_SIZE   32
>> diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
>> index c4fe92c..c0f90d4 100644
>> --- a/tools/libxc/xc_sr_common.h
>> +++ b/tools/libxc/xc_sr_common.h
>> @@ -174,6 +174,9 @@ struct xc_sr_context
>>               /* Live migrate vs non live suspend. */
>>               bool live;
>>
>> +            /* Plain VM, or checkpoints over time. */
>> +            bool checkpointed;
>> +
>>               /* Further debugging information in the stream. */
>>               bool debug;
>>
>> diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
>> index 66fcd3e..caa727d 100644
>> --- a/tools/libxc/xc_sr_save.c
>> +++ b/tools/libxc/xc_sr_save.c
>> @@ -732,6 +732,7 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom,
>>       ctx.save.callbacks = callbacks;
>>       ctx.save.live  = !!(flags & XCFLAGS_LIVE);
>>       ctx.save.debug = !!(flags & XCFLAGS_DEBUG);
>> +    ctx.save.checkpointed = !!(flags & XCFLAGS_CHECKPOINTED);
>>
>>       /*
>>        * TODO: Find some time to better tweak the live migration algorithm.
>> @@ -745,6 +746,8 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom,
>>       /* Sanity checks for callbacks. */
>>       if ( hvm )
>>           assert(callbacks->switch_qemu_logdirty);
>> +    if ( ctx.save.checkpointed )
>> +        assert(callbacks->checkpoint && callbacks->postcopy);
>>
>>       IPRINTF("In experimental %s", __func__);
>>       DPRINTF("fd %d, dom %u, max_iters %u, max_factor %u, flags %u, hvm %d",
>> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
>> index f408646..a0c9850 100644
>> --- a/tools/libxl/libxl_dom.c
>> +++ b/tools/libxl/libxl_dom.c
>> @@ -2003,6 +2003,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
>>
>>       if (r_info != NULL) {
>>           dss->interval = r_info->interval;
>> +        dss->xcflags |= XCFLAGS_CHECKPOINTED;
>>           if (libxl_defbool_val(r_info->compression))
>>               dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;
>>       }
>
>
> .
>

-- 
Thanks,
Yang.

Re: [PATCH v5 05/14] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW

[Yang Hongyang]

On 05/13/2015 11:57 PM, Ian Campbell wrote:
> On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
>> There are cases that we only need to use the hypercall buffer data,
>
> "cases where"
>> and do not use the xc_hypercall_buffer_t struct.
>> DECLARE_HYPERCALL_BUFFER_SHADOW define a user pointer that can allow
>
> "defines a user pointer"
>
>> us to access the hypercall buffer data but it also define a
>
> "defines"
>
>> xc_hypercall_buffer_t that we don't use, the compiler will report arg
>> unused error.
>> add __attribute__((unused)) before xc_hypercall_buffer_t to avoid
>> the compiler error.
>>
>> The cases for example:
>
> "Example cases:"
>
>> In send_all_pages(), we only need to use the haypercall buffer data
>
> "hypercall"
>
>> which is a dirty bitmap, we set the dirty bitmap to all dirty and call
>> send_dirty_pages, we will not use the xc_hypercall_buffer_t and hypercall
>> to retrive the dirty bitmap.
>
> "retrieve"
>
>> In send_some_pages(), we will also only need to use the dirty_bitmap.
>> the retrive dirty bitmap hypercall are done by the caller.
>
> and again.
>
>>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>>   tools/libxc/include/xenctrl.h | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
>> index a689caf..0804257 100644
>> --- a/tools/libxc/include/xenctrl.h
>> +++ b/tools/libxc/include/xenctrl.h
>> @@ -284,11 +284,15 @@ typedef struct xc_hypercall_buffer xc_hypercall_buffer_t;
>>    * Useful when a hypercall buffer is passed to a function and access
>>    * via the user pointer is required.
>>    *
>> + * The shadow xc_hypercall_buffer_t may be unused, add
>> + * __attribute__((unused)) to avoid compiler error.
>
> No need for this comment IMHO.
>
>>    * See DECLARE_HYPERCALL_BUFFER_ARGUMENT() if the user pointer is not
>>    * required.
>>    */
>>   #define DECLARE_HYPERCALL_BUFFER_SHADOW(_type, _name, _hbuf)   \
>>       _type *(_name) = (_hbuf)->hbuf;                            \
>> +    __attribute__((unused))                                    \
>
> This is a somewhat unconventional location for such a tagm but putting
> it elsewhere would involve faff-some rewrapping.
>
> So apart from the typos looks good.

Will fix those typos, thank you!

>
>
>>       xc_hypercall_buffer_t XC__HYPERCALL_BUFFER_NAME(_name) = { \
>>           .hbuf = (void *)-1,                                    \
>>           .param_shadow = (_hbuf),                               \
>
>
> .
>

-- 
Thanks,
Yang.

Re: [PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams

[Yang Hongyang]

On 05/13/2015 11:56 PM, Ian Campbell wrote:
> On Wed, 2015-05-13 at 16:47 +0100, Andrew Cooper wrote:
>> On 13/05/15 16:46, Ian Campbell wrote:
>>> On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
>>>> From: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>
>>>> 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>
>>> Subject to my finding a suitable documentation update further down the
>>> series:
>>>
>>> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
>>
>> What update are you expecting?  X86_PV_INFO is already strictly defined
>> as "sent once" in the spec.
>
> Something defining a checkpointed stread.

I think this is done in the 3rd patch? Can I take this ack?

>
> .
>

-- 
Thanks,
Yang.

Re: [PATCH v5 04/14] libxc/migration: Pass checkpoint information into the save algorithm.

[Ian Campbell]

On Thu, 2015-05-14 at 09:03 +0800, Yang Hongyang wrote:
> 
> On 05/13/2015 11:49 PM, Ian Campbell wrote:
> > On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> >> From: Andrew Cooper <andrew.cooper3@citrix.com>
> >
> > OOI how was this signalled to the old code?
> 
> The old code check the callbacks "postcopy & checkpoint",
> if the callbacks exists, it will call them which I think is
> unreliable, so I add this flag to explicitly indicate a
> checkpointed stream in the new code. However, it is backward
> compatible, the legacy migration just don't know this flag
> and will ignore it.

That makes sense, thanks.

I think it would be good to quickly mention this in the commit log. If
there is no other reason to make a v7 then you can just reply to v6 with
some text and I'll merge it in on commit.

Re: [PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams

[Ian Campbell]

On Thu, 2015-05-14 at 09:52 +0800, Yang Hongyang wrote:
> 
> On 05/13/2015 11:56 PM, Ian Campbell wrote:
> > On Wed, 2015-05-13 at 16:47 +0100, Andrew Cooper wrote:
> >> On 13/05/15 16:46, Ian Campbell wrote:
> >>> On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> >>>> From: Andrew Cooper <andrew.cooper3@citrix.com>
> >>>>
> >>>> 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>
> >>> Subject to my finding a suitable documentation update further down the
> >>> series:
> >>>
> >>> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
> >>
> >> What update are you expecting?  X86_PV_INFO is already strictly defined
> >> as "sent once" in the spec.
> >
> > Something defining a checkpointed stread.
> 
> I think this is done in the 3rd patch? Can I take this ack?

Yes, thanks.