[PATCH v6 00/16] Misc patches to aid migration v2 Remus support

[PATCH v6 00/16] 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-v6

v5->v6
Some minor changes, typos etc.

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
N = new
no mark = unchanged from last round


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

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

 docs/specs/libxc-migration-stream.pandoc |  29 +++++-
 tools/libxc/include/xenctrl.h            |   8 +-
 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               |  33 ++++--
 tools/libxc/xc_sr_restore.c              | 130 +++++++++++++++--------
 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, 303 insertions(+), 164 deletions(-)

-- 
1.9.1

[PATCH v6 01/16] 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>
Acked-by: Ian Campbell <Ian.Campbell@citrix.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 v6 02/16] 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 v6 03/16] 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>
Acked-by: Ian Campbell <Ian.Campbell@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 v6 04/16] 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 v6 05/16] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW

[Yang Hongyang]

There are cases where we only need to use the hypercall buffer data,
and do not use the xc_hypercall_buffer_t struct.
DECLARE_HYPERCALL_BUFFER_SHADOW defines a user pointer that can allow
us to access the hypercall buffer data but it also defines 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.

Example cases:
In send_all_pages(), we only need to use the hypercall 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 retrieve the dirty bitmap.
In send_some_pages(), we will also only need to use the dirty_bitmap.
the retrieve 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 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index a689caf..43fc31c 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -289,6 +289,7 @@ typedef struct xc_hypercall_buffer xc_hypercall_buffer_t;
  */
 #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 v6 06/16] 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 43fc31c..448063c 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -350,7 +350,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 v6 07/16] 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>
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>
Reviewed-by: Andrew Cooper <andrew.cooper3@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 v6 08/16] 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>
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>
Reviewed-by: Andrew Cooper <andrew.cooper3@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 v6 09/16] 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>
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>
Reviewed-by: Andrew Cooper <andrew.cooper3@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 v6 10/16] 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>
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>
Reviewed-by: Andrew Cooper <andrew.cooper3@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 v6 11/16] 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 v6 12/16] 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>
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>
Reviewed-by: Andrew Cooper <andrew.cooper3@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 v6 13/16] 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>
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>
Reviewed-by: Andrew Cooper <andrew.cooper3@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 v6 14/16] 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

[PATCH v6 15/16] libxc/restore: introduce setup() and cleanup() on restore

[Yang Hongyang]

introduce setup() and cleanup() which subsume the
ctx->restore.ops.{setup,cleanup}() calls and also
do memory alloc/free.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: 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 | 48 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
index 8022c3d..2345f66 100644
--- a/tools/libxc/xc_sr_restore.c
+++ b/tools/libxc/xc_sr_restore.c
@@ -510,6 +510,38 @@ static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec)
     return rc;
 }
 
+static int setup(struct xc_sr_context *ctx)
+{
+    xc_interface *xch = ctx->xch;
+    int rc;
+
+    rc = ctx->restore.ops.setup(ctx);
+    if ( rc )
+        goto err;
+
+    ctx->restore.max_populated_pfn = (32 * 1024 / 4) - 1;
+    ctx->restore.populated_pfns = bitmap_alloc(
+        ctx->restore.max_populated_pfn + 1);
+    if ( !ctx->restore.populated_pfns )
+    {
+        ERROR("Unable to allocate memory for populated_pfns bitmap");
+        rc = -1;
+        goto err;
+    }
+
+ err:
+    return rc;
+}
+
+static void cleanup(struct xc_sr_context *ctx)
+{
+    xc_interface *xch = ctx->xch;
+
+    free(ctx->restore.populated_pfns);
+    if ( ctx->restore.ops.cleanup(ctx) )
+        PERROR("Failed to clean up");
+}
+
 #ifdef XG_LIBXL_HVM_COMPAT
 extern int read_qemu(struct xc_sr_context *ctx);
 #endif
@@ -524,19 +556,10 @@ static int restore(struct xc_sr_context *ctx)
 
     IPRINTF("Restoring domain");
 
-    rc = ctx->restore.ops.setup(ctx);
+    rc = setup(ctx);
     if ( rc )
         goto err;
 
-    ctx->restore.max_populated_pfn = (32 * 1024 / 4) - 1;
-    ctx->restore.populated_pfns = bitmap_alloc(
-        ctx->restore.max_populated_pfn + 1);
-    if ( !ctx->restore.populated_pfns )
-    {
-        ERROR("Unable to allocate memory for populated_pfns bitmap");
-        goto err;
-    }
-
     do
     {
         rc = read_record(ctx, &rec);
@@ -571,10 +594,7 @@ static int restore(struct xc_sr_context *ctx)
     PERROR("Restore failed");
 
  done:
-    free(ctx->restore.populated_pfns);
-    rc = ctx->restore.ops.cleanup(ctx);
-    if ( rc )
-        PERROR("Failed to clean up");
+    cleanup(ctx);
 
     if ( saved_rc )
     {
-- 
1.9.1

[PATCH v6 16/16] libxc/restore: add checkpointed flag to the restore context

[Yang Hongyang]

add checkpointed flag to the restore context.

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>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/libxc/xc_sr_common.h  | 3 +++
 tools/libxc/xc_sr_restore.c | 1 +
 2 files changed, 4 insertions(+)

diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index 0ba9728..f8121e7 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 guest_type;
             uint32_t guest_page_size;
 
+            /* Plain VM, or checkpoints over time. */
+            bool checkpointed;
+
             /*
              * Xenstore and Console parameters.
              * INPUT:  evtchn & domid
diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
index 2345f66..9ab5760 100644
--- a/tools/libxc/xc_sr_restore.c
+++ b/tools/libxc/xc_sr_restore.c
@@ -624,6 +624,7 @@ int xc_domain_restore2(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 = checkpointed_stream;
     ctx.restore.callbacks = callbacks;
 
     IPRINTF("In experimental %s", __func__);
-- 
1.9.1

Re: [PATCH v6 11/16] libxc/save: rename send_some_pages to send_dirty_pages

[Andrew Cooper]

On 14/05/15 09:55, Yang Hongyang wrote:
> 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>

Reviewed-by: 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;
>  

Re: [PATCH v6 15/16] libxc/restore: introduce setup() and cleanup() on restore

[Andrew Cooper]

On 14/05/15 09:55, Yang Hongyang wrote:
> introduce setup() and cleanup() which subsume the
> ctx->restore.ops.{setup,cleanup}() calls and also
> do memory alloc/free.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> CC: 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>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
>  tools/libxc/xc_sr_restore.c | 48 ++++++++++++++++++++++++++++++++-------------
>  1 file changed, 34 insertions(+), 14 deletions(-)
>
> diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
> index 8022c3d..2345f66 100644
> --- a/tools/libxc/xc_sr_restore.c
> +++ b/tools/libxc/xc_sr_restore.c
> @@ -510,6 +510,38 @@ static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec)
>      return rc;
>  }
>  
> +static int setup(struct xc_sr_context *ctx)
> +{
> +    xc_interface *xch = ctx->xch;
> +    int rc;
> +
> +    rc = ctx->restore.ops.setup(ctx);
> +    if ( rc )
> +        goto err;
> +
> +    ctx->restore.max_populated_pfn = (32 * 1024 / 4) - 1;
> +    ctx->restore.populated_pfns = bitmap_alloc(
> +        ctx->restore.max_populated_pfn + 1);
> +    if ( !ctx->restore.populated_pfns )
> +    {
> +        ERROR("Unable to allocate memory for populated_pfns bitmap");
> +        rc = -1;
> +        goto err;
> +    }
> +
> + err:
> +    return rc;
> +}
> +
> +static void cleanup(struct xc_sr_context *ctx)
> +{
> +    xc_interface *xch = ctx->xch;
> +
> +    free(ctx->restore.populated_pfns);
> +    if ( ctx->restore.ops.cleanup(ctx) )
> +        PERROR("Failed to clean up");
> +}
> +
>  #ifdef XG_LIBXL_HVM_COMPAT
>  extern int read_qemu(struct xc_sr_context *ctx);
>  #endif
> @@ -524,19 +556,10 @@ static int restore(struct xc_sr_context *ctx)
>  
>      IPRINTF("Restoring domain");
>  
> -    rc = ctx->restore.ops.setup(ctx);
> +    rc = setup(ctx);
>      if ( rc )
>          goto err;
>  
> -    ctx->restore.max_populated_pfn = (32 * 1024 / 4) - 1;
> -    ctx->restore.populated_pfns = bitmap_alloc(
> -        ctx->restore.max_populated_pfn + 1);
> -    if ( !ctx->restore.populated_pfns )
> -    {
> -        ERROR("Unable to allocate memory for populated_pfns bitmap");
> -        goto err;
> -    }
> -
>      do
>      {
>          rc = read_record(ctx, &rec);
> @@ -571,10 +594,7 @@ static int restore(struct xc_sr_context *ctx)
>      PERROR("Restore failed");
>  
>   done:
> -    free(ctx->restore.populated_pfns);
> -    rc = ctx->restore.ops.cleanup(ctx);
> -    if ( rc )
> -        PERROR("Failed to clean up");
> +    cleanup(ctx);
>  
>      if ( saved_rc )
>      {

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

[Ian Campbell]

On Thu, 2015-05-14 at 16:55 +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>

You pointed me to the docs in v5, so my previous
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
stands.

(just replying here so I don't have to check two threads when I come to
reply).

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

[Ian Campbell]

On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

As I mentioned in v5 (after you sent v6) please propose a few words
about how this works with the v1 code and then:

        Acked-by: 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 v6 05/16] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW

[Ian Campbell]

On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote:
> There are cases where we only need to use the hypercall buffer data,
> and do not use the xc_hypercall_buffer_t struct.
> DECLARE_HYPERCALL_BUFFER_SHADOW defines a user pointer that can allow
> us to access the hypercall buffer data but it also defines 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.
> 
> Example cases:
> In send_all_pages(), we only need to use the hypercall 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 retrieve the dirty bitmap.
> In send_some_pages(), we will also only need to use the dirty_bitmap.
> the retrieve dirty bitmap hypercall are done by the caller.
> 
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>

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

Re: [PATCH v6 06/16] tools/libxc: add a check in xc_hypercall_buffer_free_pages macro

[Ian Campbell]

On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote:
> When we use a DECLARE_HYPERCALL_BUFFER_SHADOW define a user
                                                ^it defines

> pointer '_name' and a shadow xc_hypercall_buffer_t.
> then call xc_hypercall_buffer_free_pages(_xch, _name, _nr),
  ^When then calling

> the complier will report '_name' unused error, it's because
                                                 ^drop it's

> xc_hypercall_buffer_free_pages() is a MACRO and '_name'
                                                         ^is

> transparently converted to the hypercall buffer. it confused
                                                      confuses

> the caller because xc_hypercall_buffer_free_pages() do look
                                                      looks

> like a function and take '_name' as an arg.
                       takes

> 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>

I can fix those on commit if there is no other reason for a v7. 

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

Re: [PATCH v6 07/16] libxc/save: introduce setup() and cleanup() on save

[Ian Campbell]

On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote:
> introduce setup() and cleanup() which subsume the
> ctx->save.ops.{setup,cleanup}() calls.
> The SHADOW_OP_OFF hypercall also included in the cleanup().
                              ^is also

> 
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>

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

Re: [PATCH v6 08/16] libxc/save: rename to_send to dirty_bitmap

[Ian Campbell]

On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote:
> rename to_send to dirty_bitmap.
> 
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>

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

Re: [PATCH v6 09/16] libxc/save: adjust the memory allocation for migration

[Ian Campbell]

On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote:
> 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>

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

Re: [PATCH v6 10/16] libxc/save: remove bitmap param from send_some_pages

[Ian Campbell]

On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote:
> 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>

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

Re: [PATCH v6 11/16] libxc/save: rename send_some_pages to send_dirty_pages

[Ian Campbell]

On Thu, 2015-05-14 at 09:59 +0100, Andrew Cooper wrote:
> On 14/05/15 09:55, Yang Hongyang wrote:
> > 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>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH v6 12/16] libxc/save: reuse send_dirty_pages() in send_all_pages()

[Ian Campbell]

On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote:
> 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>

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

Re: [PATCH v6 13/16] libxc/restore: introduce process_record()

[Ian Campbell]

On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote:
> 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>

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

Re: [PATCH v6 14/16] libxc/restore: split read/handle qemu info

[Ian Campbell]

On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote:
> 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>

Andy seemed happy with this in v4, so:

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

Re: [PATCH v6 15/16] libxc/restore: introduce setup() and cleanup() on restore

[Ian Campbell]

On Thu, 2015-05-14 at 10:01 +0100, Andrew Cooper wrote:
> On 14/05/15 09:55, Yang Hongyang wrote:
> > introduce setup() and cleanup() which subsume the
> > ctx->restore.ops.{setup,cleanup}() calls and also
> > do memory alloc/free.
> >
> > Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> > CC: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH v6 16/16] libxc/restore: add checkpointed flag to the restore context

[Ian Campbell]

On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote:
> add checkpointed flag to the restore context.
> 
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>

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

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

[Ian Campbell]

On Thu, 2015-05-14 at 12:21 +0100, Ian Campbell wrote:
> On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote:
> > From: Andrew Cooper <andrew.cooper3@citrix.com>
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> As I mentioned in v5 (after you sent v6) please propose a few words
> about how this works with the v1 code and then:
> 
>         Acked-by: Ian Campbell <Ian.Campbell@citrix.com>

Actually since this is the only thing left for this series and timezones
suggest you won't be around until tomorrow I'm going to use:

        The old code checks the callbacks "postcopy & checkpoint",
        if the callbacks exists, it will call them. However this is
        unreliable, so add this flag to explicitly indicate a
        checkpointed stream in the new code. This is backward
        compatible with the legacy migration just don't know this flag
        and will ignore it.
        
(which is basically you explanation from v5 reworded slightly to fit a
commit log)

>         
> > 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;
> >      }
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v6 00/16] Misc patches to aid migration v2 Remus support

[Ian Campbell]

On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote:
> This is the combination of Andrew Cooper's misc patches and mine
> to aid migration v2 Remus support.

Applied, thanks. I made a few changes to some commit logs, I hope that's
ok.

Re: [PATCH v6 00/16] Misc patches to aid migration v2 Remus support

[Yang Hongyang]

On 05/14/2015 08:35 PM, Ian Campbell wrote:
> On Thu, 2015-05-14 at 16:55 +0800, Yang Hongyang wrote:
>> This is the combination of Andrew Cooper's misc patches and mine
>> to aid migration v2 Remus support.
>
> Applied, thanks. I made a few changes to some commit logs, I hope that's
> ok.
>

That's great! Thank you!

>
>
> .
>

-- 
Thanks,
Yang.