* [PATCH v4 00/14] Misc patches to aid migration v2 Remus support
@ 2015-05-12 11:25 Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 01/14] libxc/migration: Be rather stricter with illformed callers Yang Hongyang
` (13 more replies)
0 siblings, 14 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-12 11:25 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, wency, andrew.cooper3, yunhong.jiang,
eddie.dong, rshriram, ian.jackson
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v4 01/14] libxc/migration: Be rather stricter with illformed callers
2015-05-12 11:25 [PATCH v4 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
@ 2015-05-12 11:25 ` Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams Yang Hongyang
` (12 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-12 11:25 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, wency, andrew.cooper3, yunhong.jiang,
eddie.dong, rshriram, ian.jackson
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams
2015-05-12 11:25 [PATCH v4 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 01/14] libxc/migration: Be rather stricter with illformed callers Yang Hongyang
@ 2015-05-12 11:25 ` Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 03/14] libxc/migration: Specification update for CHECKPOINT records Yang Hongyang
` (11 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-12 11:25 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, wency, andrew.cooper3, yunhong.jiang,
eddie.dong, rshriram, ian.jackson
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 03/14] libxc/migration: Specification update for CHECKPOINT records
2015-05-12 11:25 [PATCH v4 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 01/14] libxc/migration: Be rather stricter with illformed callers Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams Yang Hongyang
@ 2015-05-12 11:25 ` Yang Hongyang
2015-05-12 12:05 ` Andrew Cooper
2015-05-12 11:25 ` [PATCH v4 04/14] libxc/migration: Pass checkpoint information into the save algorithm Yang Hongyang
` (10 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-12 11:25 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, wency, andrew.cooper3, yunhong.jiang,
eddie.dong, rshriram, ian.jackson
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 04/14] libxc/migration: Pass checkpoint information into the save algorithm.
2015-05-12 11:25 [PATCH v4 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (2 preceding siblings ...)
2015-05-12 11:25 ` [PATCH v4 03/14] libxc/migration: Specification update for CHECKPOINT records Yang Hongyang
@ 2015-05-12 11:25 ` Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 05/14] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW Yang Hongyang
` (9 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-12 11:25 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, wency, andrew.cooper3, yunhong.jiang,
eddie.dong, rshriram, ian.jackson
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 05/14] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW
2015-05-12 11:25 [PATCH v4 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (3 preceding siblings ...)
2015-05-12 11:25 ` [PATCH v4 04/14] libxc/migration: Pass checkpoint information into the save algorithm Yang Hongyang
@ 2015-05-12 11:25 ` Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 06/14] tools/libxc: add a check in xc_hypercall_buffer_free_pages macro Yang Hongyang
` (8 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-12 11:25 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, wency, andrew.cooper3, yunhong.jiang,
eddie.dong, rshriram, ian.jackson
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 06/14] tools/libxc: add a check in xc_hypercall_buffer_free_pages macro
2015-05-12 11:25 [PATCH v4 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (4 preceding siblings ...)
2015-05-12 11:25 ` [PATCH v4 05/14] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW Yang Hongyang
@ 2015-05-12 11:25 ` Yang Hongyang
2015-05-12 12:10 ` Andrew Cooper
2015-05-12 11:25 ` [PATCH v4 07/14] libxc/save: introduce setup() and cleanup() on save Yang Hongyang
` (7 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-12 11:25 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, wency, andrew.cooper3, yunhong.jiang,
eddie.dong, rshriram, ian.jackson
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 07/14] libxc/save: introduce setup() and cleanup() on save
2015-05-12 11:25 [PATCH v4 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (5 preceding siblings ...)
2015-05-12 11:25 ` [PATCH v4 06/14] tools/libxc: add a check in xc_hypercall_buffer_free_pages macro Yang Hongyang
@ 2015-05-12 11:25 ` Yang Hongyang
2015-05-12 12:11 ` Andrew Cooper
2015-05-12 11:25 ` [PATCH v4 08/14] libxc/save: rename to_send to dirty_bitmap Yang Hongyang
` (6 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-12 11:25 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, wency, andrew.cooper3, yunhong.jiang,
eddie.dong, rshriram, ian.jackson
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 08/14] libxc/save: rename to_send to dirty_bitmap
2015-05-12 11:25 [PATCH v4 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (6 preceding siblings ...)
2015-05-12 11:25 ` [PATCH v4 07/14] libxc/save: introduce setup() and cleanup() on save Yang Hongyang
@ 2015-05-12 11:25 ` Yang Hongyang
2015-05-12 12:11 ` Andrew Cooper
2015-05-12 11:25 ` [PATCH v4 09/14] libxc/save: adjust the memory allocation for migration Yang Hongyang
` (5 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-12 11:25 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, wency, andrew.cooper3, yunhong.jiang,
eddie.dong, rshriram, ian.jackson
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 09/14] libxc/save: adjust the memory allocation for migration
2015-05-12 11:25 [PATCH v4 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (7 preceding siblings ...)
2015-05-12 11:25 ` [PATCH v4 08/14] libxc/save: rename to_send to dirty_bitmap Yang Hongyang
@ 2015-05-12 11:25 ` Yang Hongyang
2015-05-12 12:14 ` Andrew Cooper
2015-05-12 11:25 ` [PATCH v4 10/14] libxc/save: remove bitmap param from send_some_pages Yang Hongyang
` (4 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-12 11:25 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, wency, andrew.cooper3, yunhong.jiang,
eddie.dong, rshriram, ian.jackson
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 10/14] libxc/save: remove bitmap param from send_some_pages
2015-05-12 11:25 [PATCH v4 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (8 preceding siblings ...)
2015-05-12 11:25 ` [PATCH v4 09/14] libxc/save: adjust the memory allocation for migration Yang Hongyang
@ 2015-05-12 11:25 ` Yang Hongyang
2015-05-12 12:15 ` Andrew Cooper
2015-05-12 11:25 ` [PATCH v4 11/14] libxc/save: rename send_some_pages to send_dirty_pages Yang Hongyang
` (3 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-12 11:25 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, wency, andrew.cooper3, yunhong.jiang,
eddie.dong, rshriram, ian.jackson
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 11/14] libxc/save: rename send_some_pages to send_dirty_pages
2015-05-12 11:25 [PATCH v4 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (9 preceding siblings ...)
2015-05-12 11:25 ` [PATCH v4 10/14] libxc/save: remove bitmap param from send_some_pages Yang Hongyang
@ 2015-05-12 11:25 ` Yang Hongyang
2015-05-12 12:15 ` Andrew Cooper
2015-05-12 11:25 ` [PATCH v4 12/14] libxc/save: reuse send_dirty_pages() in send_all_pages() Yang Hongyang
` (2 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-12 11:25 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, wency, andrew.cooper3, yunhong.jiang,
eddie.dong, rshriram, ian.jackson
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 12/14] libxc/save: reuse send_dirty_pages() in send_all_pages()
2015-05-12 11:25 [PATCH v4 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (10 preceding siblings ...)
2015-05-12 11:25 ` [PATCH v4 11/14] libxc/save: rename send_some_pages to send_dirty_pages Yang Hongyang
@ 2015-05-12 11:25 ` Yang Hongyang
2015-05-12 12:17 ` Andrew Cooper
2015-05-12 11:25 ` [PATCH v4 13/14] libxc/restore: introduce process_record() Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 14/14] libxc/restore: split read/handle qemu info Yang Hongyang
13 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-12 11:25 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, wency, andrew.cooper3, yunhong.jiang,
eddie.dong, rshriram, ian.jackson
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 13/14] libxc/restore: introduce process_record()
2015-05-12 11:25 [PATCH v4 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (11 preceding siblings ...)
2015-05-12 11:25 ` [PATCH v4 12/14] libxc/save: reuse send_dirty_pages() in send_all_pages() Yang Hongyang
@ 2015-05-12 11:25 ` Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 14/14] libxc/restore: split read/handle qemu info Yang Hongyang
13 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-12 11:25 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, wency, andrew.cooper3, yunhong.jiang,
eddie.dong, rshriram, ian.jackson
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 14/14] libxc/restore: split read/handle qemu info
2015-05-12 11:25 [PATCH v4 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (12 preceding siblings ...)
2015-05-12 11:25 ` [PATCH v4 13/14] libxc/restore: introduce process_record() Yang Hongyang
@ 2015-05-12 11:25 ` Yang Hongyang
2015-05-12 12:20 ` Andrew Cooper
13 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-12 11:25 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, wency, andrew.cooper3, yunhong.jiang,
eddie.dong, rshriram, ian.jackson
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v4 03/14] libxc/migration: Specification update for CHECKPOINT records
2015-05-12 11:25 ` [PATCH v4 03/14] libxc/migration: Specification update for CHECKPOINT records Yang Hongyang
@ 2015-05-12 12:05 ` Andrew Cooper
2015-05-13 0:47 ` Yang Hongyang
0 siblings, 1 reply; 28+ messages in thread
From: Andrew Cooper @ 2015-05-12 12:05 UTC (permalink / raw)
To: Yang Hongyang, xen-devel
Cc: wei.liu2, ian.campbell, wency, ian.jackson, yunhong.jiang,
eddie.dong, rshriram
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
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 06/14] tools/libxc: add a check in xc_hypercall_buffer_free_pages macro
2015-05-12 11:25 ` [PATCH v4 06/14] tools/libxc: add a check in xc_hypercall_buffer_free_pages macro Yang Hongyang
@ 2015-05-12 12:10 ` Andrew Cooper
2015-05-13 0:48 ` Yang Hongyang
0 siblings, 1 reply; 28+ messages in thread
From: Andrew Cooper @ 2015-05-12 12:10 UTC (permalink / raw)
To: Yang Hongyang, xen-devel
Cc: wei.liu2, ian.campbell, wency, ian.jackson, yunhong.jiang,
eddie.dong, rshriram
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 07/14] libxc/save: introduce setup() and cleanup() on save
2015-05-12 11:25 ` [PATCH v4 07/14] libxc/save: introduce setup() and cleanup() on save Yang Hongyang
@ 2015-05-12 12:11 ` Andrew Cooper
2015-05-13 0:48 ` Yang Hongyang
0 siblings, 1 reply; 28+ messages in thread
From: Andrew Cooper @ 2015-05-12 12:11 UTC (permalink / raw)
To: Yang Hongyang, xen-devel
Cc: wei.liu2, ian.campbell, wency, ian.jackson, yunhong.jiang,
eddie.dong, rshriram
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 )
> {
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 08/14] libxc/save: rename to_send to dirty_bitmap
2015-05-12 11:25 ` [PATCH v4 08/14] libxc/save: rename to_send to dirty_bitmap Yang Hongyang
@ 2015-05-12 12:11 ` Andrew Cooper
0 siblings, 0 replies; 28+ messages in thread
From: Andrew Cooper @ 2015-05-12 12:11 UTC (permalink / raw)
To: Yang Hongyang, xen-devel
Cc: wei.liu2, ian.campbell, wency, ian.jackson, yunhong.jiang,
eddie.dong, rshriram
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);
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 09/14] libxc/save: adjust the memory allocation for migration
2015-05-12 11:25 ` [PATCH v4 09/14] libxc/save: adjust the memory allocation for migration Yang Hongyang
@ 2015-05-12 12:14 ` Andrew Cooper
2015-05-13 0:49 ` Yang Hongyang
0 siblings, 1 reply; 28+ messages in thread
From: Andrew Cooper @ 2015-05-12 12:14 UTC (permalink / raw)
To: Yang Hongyang, xen-devel
Cc: wei.liu2, ian.campbell, wency, ian.jackson, yunhong.jiang,
eddie.dong, rshriram
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>
> }
>
> /*
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 10/14] libxc/save: remove bitmap param from send_some_pages
2015-05-12 11:25 ` [PATCH v4 10/14] libxc/save: remove bitmap param from send_some_pages Yang Hongyang
@ 2015-05-12 12:15 ` Andrew Cooper
0 siblings, 0 replies; 28+ messages in thread
From: Andrew Cooper @ 2015-05-12 12:15 UTC (permalink / raw)
To: Yang Hongyang, xen-devel
Cc: wei.liu2, ian.campbell, wency, ian.jackson, yunhong.jiang,
eddie.dong, rshriram
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;
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 11/14] libxc/save: rename send_some_pages to send_dirty_pages
2015-05-12 11:25 ` [PATCH v4 11/14] libxc/save: rename send_some_pages to send_dirty_pages Yang Hongyang
@ 2015-05-12 12:15 ` Andrew Cooper
0 siblings, 0 replies; 28+ messages in thread
From: Andrew Cooper @ 2015-05-12 12:15 UTC (permalink / raw)
To: xen-devel
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;
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 12/14] libxc/save: reuse send_dirty_pages() in send_all_pages()
2015-05-12 11:25 ` [PATCH v4 12/14] libxc/save: reuse send_dirty_pages() in send_all_pages() Yang Hongyang
@ 2015-05-12 12:17 ` Andrew Cooper
0 siblings, 0 replies; 28+ messages in thread
From: Andrew Cooper @ 2015-05-12 12:17 UTC (permalink / raw)
To: Yang Hongyang, xen-devel
Cc: wei.liu2, ian.campbell, wency, ian.jackson, yunhong.jiang,
eddie.dong, rshriram
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;
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 14/14] libxc/restore: split read/handle qemu info
2015-05-12 11:25 ` [PATCH v4 14/14] libxc/restore: split read/handle qemu info Yang Hongyang
@ 2015-05-12 12:20 ` Andrew Cooper
0 siblings, 0 replies; 28+ messages in thread
From: Andrew Cooper @ 2015-05-12 12:20 UTC (permalink / raw)
To: Yang Hongyang, xen-devel
Cc: wei.liu2, ian.campbell, wency, ian.jackson, yunhong.jiang,
eddie.dong, rshriram
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 )
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 03/14] libxc/migration: Specification update for CHECKPOINT records
2015-05-12 12:05 ` Andrew Cooper
@ 2015-05-13 0:47 ` Yang Hongyang
0 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 0:47 UTC (permalink / raw)
To: Andrew Cooper, xen-devel
Cc: wei.liu2, ian.campbell, wency, ian.jackson, yunhong.jiang,
eddie.dong, rshriram
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 06/14] tools/libxc: add a check in xc_hypercall_buffer_free_pages macro
2015-05-12 12:10 ` Andrew Cooper
@ 2015-05-13 0:48 ` Yang Hongyang
0 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 0:48 UTC (permalink / raw)
To: Andrew Cooper, xen-devel
Cc: wei.liu2, ian.campbell, wency, ian.jackson, yunhong.jiang,
eddie.dong, rshriram
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 07/14] libxc/save: introduce setup() and cleanup() on save
2015-05-12 12:11 ` Andrew Cooper
@ 2015-05-13 0:48 ` Yang Hongyang
0 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 0:48 UTC (permalink / raw)
To: Andrew Cooper, xen-devel
Cc: wei.liu2, ian.campbell, wency, ian.jackson, yunhong.jiang,
eddie.dong, rshriram
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 09/14] libxc/save: adjust the memory allocation for migration
2015-05-12 12:14 ` Andrew Cooper
@ 2015-05-13 0:49 ` Yang Hongyang
0 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 0:49 UTC (permalink / raw)
To: Andrew Cooper, xen-devel
Cc: wei.liu2, ian.campbell, wency, ian.jackson, yunhong.jiang,
eddie.dong, rshriram
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2015-05-13 0:49 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-12 11:25 [PATCH v4 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 01/14] libxc/migration: Be rather stricter with illformed callers Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 03/14] libxc/migration: Specification update for CHECKPOINT records Yang Hongyang
2015-05-12 12:05 ` Andrew Cooper
2015-05-13 0:47 ` Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 04/14] libxc/migration: Pass checkpoint information into the save algorithm Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 05/14] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 06/14] tools/libxc: add a check in xc_hypercall_buffer_free_pages macro Yang Hongyang
2015-05-12 12:10 ` Andrew Cooper
2015-05-13 0:48 ` Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 07/14] libxc/save: introduce setup() and cleanup() on save Yang Hongyang
2015-05-12 12:11 ` Andrew Cooper
2015-05-13 0:48 ` Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 08/14] libxc/save: rename to_send to dirty_bitmap Yang Hongyang
2015-05-12 12:11 ` Andrew Cooper
2015-05-12 11:25 ` [PATCH v4 09/14] libxc/save: adjust the memory allocation for migration Yang Hongyang
2015-05-12 12:14 ` Andrew Cooper
2015-05-13 0:49 ` Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 10/14] libxc/save: remove bitmap param from send_some_pages Yang Hongyang
2015-05-12 12:15 ` Andrew Cooper
2015-05-12 11:25 ` [PATCH v4 11/14] libxc/save: rename send_some_pages to send_dirty_pages Yang Hongyang
2015-05-12 12:15 ` Andrew Cooper
2015-05-12 11:25 ` [PATCH v4 12/14] libxc/save: reuse send_dirty_pages() in send_all_pages() Yang Hongyang
2015-05-12 12:17 ` Andrew Cooper
2015-05-12 11:25 ` [PATCH v4 13/14] libxc/restore: introduce process_record() Yang Hongyang
2015-05-12 11:25 ` [PATCH v4 14/14] libxc/restore: split read/handle qemu info Yang Hongyang
2015-05-12 12:20 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.