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

[PATCH v4 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-v4

Andrew Cooper (4):
  libxc/migration: Be rather stricter with illformed callers
  libxc/save: Adjust stream-position callbacks for checkpointed streams
  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
  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

 docs/specs/libxc-migration-stream.pandoc |  33 +++++-
 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               |  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, 274 insertions(+), 153 deletions(-)

-- 
1.9.1

[PATCH v4 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 v4 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 v4 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 | 33 +++++++++++++++++++++++++++++---
 tools/libxc/xc_sr_common.c               |  1 +
 tools/libxc/xc_sr_stream_format.h        |  1 +
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/docs/specs/libxc-migration-stream.pandoc b/docs/specs/libxc-migration-stream.pandoc
index 520240f..842938c 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,33 @@ 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
+
+A stream containing checkpoint records must have indicated itself as a
+checkpointed stream in the Image Header. Conversely, a stream not
+identified as checkpointed must not contain checkpoint records.
+
+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

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

[Andrew Cooper]

On 12/05/15 12:25, 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>
> 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 | 33 +++++++++++++++++++++++++++++---
>  tools/libxc/xc_sr_common.c               |  1 +
>  tools/libxc/xc_sr_stream_format.h        |  1 +
>  3 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/docs/specs/libxc-migration-stream.pandoc b/docs/specs/libxc-migration-stream.pandoc
> index 520240f..842938c 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,33 @@ 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
> +
> +A stream containing checkpoint records must have indicated itself as a
> +checkpointed stream in the Image Header. Conversely, a stream not
> +identified as checkpointed must not contain checkpoint records.

The above paragraph needs deleting.  It was my mistake for not deleting
it in my series.

~Andrew

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

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

[Yang Hongyang]

On 05/12/2015 08:05 PM, Andrew Cooper wrote:
> On 12/05/15 12:25, 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>
>> 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 | 33 +++++++++++++++++++++++++++++---
>>   tools/libxc/xc_sr_common.c               |  1 +
>>   tools/libxc/xc_sr_stream_format.h        |  1 +
>>   3 files changed, 32 insertions(+), 3 deletions(-)
>>
>> diff --git a/docs/specs/libxc-migration-stream.pandoc b/docs/specs/libxc-migration-stream.pandoc
>> index 520240f..842938c 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,33 @@ 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
>> +
>> +A stream containing checkpoint records must have indicated itself as a
>> +checkpointed stream in the Image Header. Conversely, a stream not
>> +identified as checkpointed must not contain checkpoint records.
>
> The above paragraph needs deleting.  It was my mistake for not deleting
> it in my series.

Ok, will delete it.

>
> ~Andrew
>
>> +
>> +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
>>
>
> .
>

-- 
Thanks,
Yang.

[PATCH v4 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 v4 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 v4 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 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 0804257..35fb3ac 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -353,7 +353,9 @@ 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)        \
+    if ( _name )                                                \
+        xc__hypercall_buffer_free_pages(_xch, HYPERCALL_BUFFER(_name), _nr)
 
 /*
  * Array of hypercall buffers.
-- 
1.9.1

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

[Andrew Cooper]

On 12/05/15 12:25, Yang Hongyang wrote:
> 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 | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 0804257..35fb3ac 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -353,7 +353,9 @@ 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)        \
> +    if ( _name )                                                \
> +        xc__hypercall_buffer_free_pages(_xch, HYPERCALL_BUFFER(_name), _nr)

This needs to be wrapped in a standard "do { ... } while (0)" for macros

~Andrew

>  
>  /*
>   * Array of hypercall buffers.

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

[Yang Hongyang]

On 05/12/2015 08:10 PM, Andrew Cooper wrote:
> On 12/05/15 12:25, Yang Hongyang wrote:
>> 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 | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
>> index 0804257..35fb3ac 100644
>> --- a/tools/libxc/include/xenctrl.h
>> +++ b/tools/libxc/include/xenctrl.h
>> @@ -353,7 +353,9 @@ 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)        \
>> +    if ( _name )                                                \
>> +        xc__hypercall_buffer_free_pages(_xch, HYPERCALL_BUFFER(_name), _nr)
>
> This needs to be wrapped in a standard "do { ... } while (0)" for macros

Ok, thank you!

>
> ~Andrew
>
>>
>>   /*
>>    * Array of hypercall buffers.
>
> .
>

-- 
Thanks,
Yang.

[PATCH v4 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>
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 | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index caa727d..8a847fc 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -637,6 +637,32 @@ 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 +674,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 +727,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

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

[Andrew Cooper]

On 12/05/15 12:25, 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().
>
> 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 | 35 ++++++++++++++++++++++++++++-------
>  1 file changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
> index caa727d..8a847fc 100644
> --- a/tools/libxc/xc_sr_save.c
> +++ b/tools/libxc/xc_sr_save.c
> @@ -637,6 +637,32 @@ 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;
> +

Stray blank line which can be dropped.

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

> +}
> +
> +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 +674,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 +727,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 )
>      {

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

[Yang Hongyang]

On 05/12/2015 08:11 PM, Andrew Cooper wrote:
> On 12/05/15 12:25, 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().
>>
>> 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 | 35 ++++++++++++++++++++++++++++-------
>>   1 file changed, 28 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
>> index caa727d..8a847fc 100644
>> --- a/tools/libxc/xc_sr_save.c
>> +++ b/tools/libxc/xc_sr_save.c
>> @@ -637,6 +637,32 @@ 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;
>> +
>
> Stray blank line which can be dropped.

OK
>
> Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
>> +}
>> +
>> +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 +674,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 +727,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 )
>>       {
>
> .
>

-- 
Thanks,
Yang.

[PATCH v4 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>
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 | 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 8a847fc..368aacb 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

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

[Andrew Cooper]

On 12/05/15 12:25, Yang Hongyang wrote:
> 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 8a847fc..368aacb 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);

[PATCH v4 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>
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 |  1 +
 tools/libxc/xc_sr_save.c   | 61 ++++++++++++++++++++++------------------------
 2 files changed, 30 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 368aacb..0953c35 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 )
@@ -655,12 +643,21 @@ 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

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

[Andrew Cooper]

On 12/05/15 12:25, 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>
> 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 |  1 +
>  tools/libxc/xc_sr_save.c   | 61 ++++++++++++++++++++++------------------------
>  2 files changed, 30 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 368aacb..0953c35 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));

You can drop this extra bracketing now that
DECLARE_HYPERCALL_BUFFER_SHADOW() has been fixed.

>  
>      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 )
> @@ -655,12 +643,21 @@ 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));

And also drop this bracketing.

> +
>  
>      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);
> +

Stray blank line.

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

>  }
>  
>  /*

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

[Yang Hongyang]

On 05/12/2015 08:14 PM, Andrew Cooper wrote:
> On 12/05/15 12:25, 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>
>> 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 |  1 +
>>   tools/libxc/xc_sr_save.c   | 61 ++++++++++++++++++++++------------------------
>>   2 files changed, 30 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 368aacb..0953c35 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));
>
> You can drop this extra bracketing now that
> DECLARE_HYPERCALL_BUFFER_SHADOW() has been fixed.

Ok, will drop the bracketing in all DECLARE_HYPERCALL_BUFFER_SHADOW() calls

>
>>
>>       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 )
>> @@ -655,12 +643,21 @@ 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));
>
> And also drop this bracketing.
>
>> +
>>
>>       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);
>> +
>
> Stray blank line.
>
> Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
>>   }
>>
>>   /*
>
> .
>

-- 
Thanks,
Yang.

[PATCH v4 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>
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 | 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 0953c35..af2c859 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

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

[Andrew Cooper]

On 12/05/15 12:25, 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>
> 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 0953c35..af2c859 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;
>  

[PATCH v4 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 af2c859..8273a9b 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

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

[Andrew Cooper]

On 12/05/15 12:25, 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 af2c859..8273a9b 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;
>  

[PATCH v4 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 8273a9b..0007085 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

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

[Andrew Cooper]

On 12/05/15 12:25, 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>
> 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 8273a9b..0007085 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));

Can drop this bracketing as well.

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

[PATCH v4 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 v4 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 v4 14/14] libxc/restore: split read/handle qemu info

[Andrew Cooper]

On 12/05/15 12:25, 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>

Tools Maintainers: I have not reviewed this in detail but I am happy
with it.  It is all just changes to the compat code which will disappear
anyway when I get full libxl/migrationv2 working.

~Andrew

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