* [PATCH v5 01/14] libxc/migration: Be rather stricter with illformed callers
2015-05-13 1:53 [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
@ 2015-05-13 1:53 ` Yang Hongyang
2015-05-13 15:45 ` Ian Campbell
2015-05-13 1:53 ` [PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams Yang Hongyang
` (13 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 1:53 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
* Re: [PATCH v5 01/14] libxc/migration: Be rather stricter with illformed callers
2015-05-13 1:53 ` [PATCH v5 01/14] libxc/migration: Be rather stricter with illformed callers Yang Hongyang
@ 2015-05-13 15:45 ` Ian Campbell
0 siblings, 0 replies; 28+ messages in thread
From: Ian Campbell @ 2015-05-13 15:45 UTC (permalink / raw)
To: Yang Hongyang
Cc: wei.liu2, eddie.dong, wency, andrew.cooper3, yunhong.jiang,
ian.jackson, xen-devel, rshriram
On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>
>
> The migration code itself should be able to validly assume all mandatory
> callbacks are set up.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams
2015-05-13 1:53 [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
2015-05-13 1:53 ` [PATCH v5 01/14] libxc/migration: Be rather stricter with illformed callers Yang Hongyang
@ 2015-05-13 1:53 ` Yang Hongyang
2015-05-13 15:46 ` Ian Campbell
2015-05-13 1:53 ` [PATCH v5 03/14] libxc/migration: Specification update for CHECKPOINT records Yang Hongyang
` (12 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 1:53 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
* Re: [PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams
2015-05-13 1:53 ` [PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams Yang Hongyang
@ 2015-05-13 15:46 ` Ian Campbell
2015-05-13 15:47 ` Andrew Cooper
0 siblings, 1 reply; 28+ messages in thread
From: Ian Campbell @ 2015-05-13 15:46 UTC (permalink / raw)
To: Yang Hongyang
Cc: wei.liu2, eddie.dong, wency, andrew.cooper3, yunhong.jiang,
ian.jackson, xen-devel, rshriram
On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>
>
> There are some records which should only be sent once in the stream, and not
> repeated for each checkpoint. {start,end}_of_stream() become per-checkpoint,
> and a new start_of_stream() is introduced.
>
> There is no resulting change record order, but the X86_PV_INFO record is
> identified as once per stream. Currently the X86_PV_P2M_FRAMES record is as
> well, but this is because of an implementation bug and can move back to being
> on an as-needed basis when fixed.
>
> In addition, a few minor adjustments of comments and layout.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Subject to my finding a suitable documentation update further down the
series:
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams
2015-05-13 15:46 ` Ian Campbell
@ 2015-05-13 15:47 ` Andrew Cooper
2015-05-13 15:56 ` Ian Campbell
0 siblings, 1 reply; 28+ messages in thread
From: Andrew Cooper @ 2015-05-13 15:47 UTC (permalink / raw)
To: Ian Campbell, Yang Hongyang
Cc: wei.liu2, wency, ian.jackson, yunhong.jiang, eddie.dong,
xen-devel, rshriram
On 13/05/15 16:46, Ian Campbell wrote:
> On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
>> From: Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> There are some records which should only be sent once in the stream, and not
>> repeated for each checkpoint. {start,end}_of_stream() become per-checkpoint,
>> and a new start_of_stream() is introduced.
>>
>> There is no resulting change record order, but the X86_PV_INFO record is
>> identified as once per stream. Currently the X86_PV_P2M_FRAMES record is as
>> well, but this is because of an implementation bug and can move back to being
>> on an as-needed basis when fixed.
>>
>> In addition, a few minor adjustments of comments and layout.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Subject to my finding a suitable documentation update further down the
> series:
>
> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
What update are you expecting? X86_PV_INFO is already strictly defined
as "sent once" in the spec.
~Andrew
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams
2015-05-13 15:47 ` Andrew Cooper
@ 2015-05-13 15:56 ` Ian Campbell
2015-05-14 1:52 ` Yang Hongyang
0 siblings, 1 reply; 28+ messages in thread
From: Ian Campbell @ 2015-05-13 15:56 UTC (permalink / raw)
To: Andrew Cooper
Cc: wei.liu2, wency, ian.jackson, yunhong.jiang, eddie.dong,
xen-devel, rshriram, Yang Hongyang
On Wed, 2015-05-13 at 16:47 +0100, Andrew Cooper wrote:
> On 13/05/15 16:46, Ian Campbell wrote:
> > On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> >> From: Andrew Cooper <andrew.cooper3@citrix.com>
> >>
> >> There are some records which should only be sent once in the stream, and not
> >> repeated for each checkpoint. {start,end}_of_stream() become per-checkpoint,
> >> and a new start_of_stream() is introduced.
> >>
> >> There is no resulting change record order, but the X86_PV_INFO record is
> >> identified as once per stream. Currently the X86_PV_P2M_FRAMES record is as
> >> well, but this is because of an implementation bug and can move back to being
> >> on an as-needed basis when fixed.
> >>
> >> In addition, a few minor adjustments of comments and layout.
> >>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Subject to my finding a suitable documentation update further down the
> > series:
> >
> > Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
>
> What update are you expecting? X86_PV_INFO is already strictly defined
> as "sent once" in the spec.
Something defining a checkpointed stread.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams
2015-05-13 15:56 ` Ian Campbell
@ 2015-05-14 1:52 ` Yang Hongyang
2015-05-14 11:18 ` Ian Campbell
0 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-14 1:52 UTC (permalink / raw)
To: Ian Campbell, Andrew Cooper
Cc: wei.liu2, wency, ian.jackson, yunhong.jiang, eddie.dong,
xen-devel, rshriram
On 05/13/2015 11:56 PM, Ian Campbell wrote:
> On Wed, 2015-05-13 at 16:47 +0100, Andrew Cooper wrote:
>> On 13/05/15 16:46, Ian Campbell wrote:
>>> On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
>>>> From: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>
>>>> There are some records which should only be sent once in the stream, and not
>>>> repeated for each checkpoint. {start,end}_of_stream() become per-checkpoint,
>>>> and a new start_of_stream() is introduced.
>>>>
>>>> There is no resulting change record order, but the X86_PV_INFO record is
>>>> identified as once per stream. Currently the X86_PV_P2M_FRAMES record is as
>>>> well, but this is because of an implementation bug and can move back to being
>>>> on an as-needed basis when fixed.
>>>>
>>>> In addition, a few minor adjustments of comments and layout.
>>>>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Subject to my finding a suitable documentation update further down the
>>> series:
>>>
>>> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
>>
>> What update are you expecting? X86_PV_INFO is already strictly defined
>> as "sent once" in the spec.
>
> Something defining a checkpointed stread.
I think this is done in the 3rd patch? Can I take this ack?
>
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams
2015-05-14 1:52 ` Yang Hongyang
@ 2015-05-14 11:18 ` Ian Campbell
0 siblings, 0 replies; 28+ messages in thread
From: Ian Campbell @ 2015-05-14 11:18 UTC (permalink / raw)
To: Yang Hongyang
Cc: wei.liu2, wency, Andrew Cooper, yunhong.jiang, eddie.dong,
xen-devel, rshriram, ian.jackson
On Thu, 2015-05-14 at 09:52 +0800, Yang Hongyang wrote:
>
> On 05/13/2015 11:56 PM, Ian Campbell wrote:
> > On Wed, 2015-05-13 at 16:47 +0100, Andrew Cooper wrote:
> >> On 13/05/15 16:46, Ian Campbell wrote:
> >>> On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> >>>> From: Andrew Cooper <andrew.cooper3@citrix.com>
> >>>>
> >>>> There are some records which should only be sent once in the stream, and not
> >>>> repeated for each checkpoint. {start,end}_of_stream() become per-checkpoint,
> >>>> and a new start_of_stream() is introduced.
> >>>>
> >>>> There is no resulting change record order, but the X86_PV_INFO record is
> >>>> identified as once per stream. Currently the X86_PV_P2M_FRAMES record is as
> >>>> well, but this is because of an implementation bug and can move back to being
> >>>> on an as-needed basis when fixed.
> >>>>
> >>>> In addition, a few minor adjustments of comments and layout.
> >>>>
> >>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >>> Subject to my finding a suitable documentation update further down the
> >>> series:
> >>>
> >>> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
> >>
> >> What update are you expecting? X86_PV_INFO is already strictly defined
> >> as "sent once" in the spec.
> >
> > Something defining a checkpointed stread.
>
> I think this is done in the 3rd patch? Can I take this ack?
Yes, thanks.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 03/14] libxc/migration: Specification update for CHECKPOINT records
2015-05-13 1:53 [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
2015-05-13 1:53 ` [PATCH v5 01/14] libxc/migration: Be rather stricter with illformed callers Yang Hongyang
2015-05-13 1:53 ` [PATCH v5 02/14] libxc/save: Adjust stream-position callbacks for checkpointed streams Yang Hongyang
@ 2015-05-13 1:53 ` Yang Hongyang
2015-05-13 15:48 ` Ian Campbell
2015-05-13 1:53 ` [PATCH v5 04/14] libxc/migration: Pass checkpoint information into the save algorithm Yang Hongyang
` (11 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 1:53 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 | 29 ++++++++++++++++++++++++++---
tools/libxc/xc_sr_common.c | 1 +
tools/libxc/xc_sr_stream_format.h | 1 +
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/docs/specs/libxc-migration-stream.pandoc b/docs/specs/libxc-migration-stream.pandoc
index 520240f..68fa513 100644
--- a/docs/specs/libxc-migration-stream.pandoc
+++ b/docs/specs/libxc-migration-stream.pandoc
@@ -37,8 +37,6 @@ Not Yet Included
The following features are not yet fully specified and will be
included in a future draft.
-* Remus
-
* Page data compression.
* ARM
@@ -227,7 +225,9 @@ type 0x00000000: END
0x0000000D: VERIFY
- 0x0000000E - 0x7FFFFFFF: Reserved for future _mandatory_
+ 0x0000000E: CHECKPOINT
+
+ 0x0000000F - 0x7FFFFFFF: Reserved for future _mandatory_
records.
0x80000000 - 0xFFFFFFFF: Reserved for future _optional_
@@ -578,6 +578,29 @@ The verify record contains no fields; its body_length is 0.
\clearpage
+CHECKPOINT
+----------
+
+A checkpoint record indicates that all the preceding records in the stream
+represent a consistent view of VM state.
+
+ 0 1 2 3 4 5 6 7 octet
+ +-------------------------------------------------+
+
+The checkpoint record contains no fields; its body_length is 0
+
+If the stream is embedded in a higher level toolstack stream, the
+CHECKPOINT record marks the end of the libxc portion of the stream
+and the stream is handed back to the higher level for further
+processing.
+
+The higher level stream may then hand the stream back to libxc to
+process another set of records for the next consistent VM state
+snapshot. This next set of records may be terminated by another
+CHECKPOINT record or an END record.
+
+\clearpage
+
Layout
======
diff --git a/tools/libxc/xc_sr_common.c b/tools/libxc/xc_sr_common.c
index 59e0c5d..945cfa6 100644
--- a/tools/libxc/xc_sr_common.c
+++ b/tools/libxc/xc_sr_common.c
@@ -34,6 +34,7 @@ static const char *mandatory_rec_types[] =
[REC_TYPE_TOOLSTACK] = "Toolstack",
[REC_TYPE_X86_PV_VCPU_MSRS] = "x86 PV vcpu msrs",
[REC_TYPE_VERIFY] = "Verify",
+ [REC_TYPE_CHECKPOINT] = "Checkpoint",
};
const char *rec_type_to_str(uint32_t type)
diff --git a/tools/libxc/xc_sr_stream_format.h b/tools/libxc/xc_sr_stream_format.h
index d116ca6..6d0f8fd 100644
--- a/tools/libxc/xc_sr_stream_format.h
+++ b/tools/libxc/xc_sr_stream_format.h
@@ -74,6 +74,7 @@ struct xc_sr_rhdr
#define REC_TYPE_TOOLSTACK 0x0000000bU
#define REC_TYPE_X86_PV_VCPU_MSRS 0x0000000cU
#define REC_TYPE_VERIFY 0x0000000dU
+#define REC_TYPE_CHECKPOINT 0x0000000eU
#define REC_TYPE_OPTIONAL 0x80000000U
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v5 03/14] libxc/migration: Specification update for CHECKPOINT records
2015-05-13 1:53 ` [PATCH v5 03/14] libxc/migration: Specification update for CHECKPOINT records Yang Hongyang
@ 2015-05-13 15:48 ` Ian Campbell
0 siblings, 0 replies; 28+ messages in thread
From: Ian Campbell @ 2015-05-13 15:48 UTC (permalink / raw)
To: Yang Hongyang
Cc: wei.liu2, eddie.dong, wency, andrew.cooper3, yunhong.jiang,
ian.jackson, xen-devel, rshriram
On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Checkpointed streams need to signal the end of a consistent view of VM state,
> and the start of the libxl data.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 04/14] libxc/migration: Pass checkpoint information into the save algorithm.
2015-05-13 1:53 [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (2 preceding siblings ...)
2015-05-13 1:53 ` [PATCH v5 03/14] libxc/migration: Specification update for CHECKPOINT records Yang Hongyang
@ 2015-05-13 1:53 ` Yang Hongyang
2015-05-13 15:49 ` Ian Campbell
2015-05-13 1:53 ` [PATCH v5 05/14] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW Yang Hongyang
` (10 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 1:53 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
* Re: [PATCH v5 04/14] libxc/migration: Pass checkpoint information into the save algorithm.
2015-05-13 1:53 ` [PATCH v5 04/14] libxc/migration: Pass checkpoint information into the save algorithm Yang Hongyang
@ 2015-05-13 15:49 ` Ian Campbell
2015-05-14 1:03 ` Yang Hongyang
0 siblings, 1 reply; 28+ messages in thread
From: Ian Campbell @ 2015-05-13 15:49 UTC (permalink / raw)
To: Yang Hongyang
Cc: wei.liu2, eddie.dong, wency, andrew.cooper3, yunhong.jiang,
ian.jackson, xen-devel, rshriram
On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>
OOI how was this signalled to the old code?
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
> tools/libxc/include/xenguest.h | 1 +
> tools/libxc/xc_sr_common.h | 3 +++
> tools/libxc/xc_sr_save.c | 3 +++
> tools/libxl/libxl_dom.c | 1 +
> 4 files changed, 8 insertions(+)
>
> diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
> index 8e39075..7581263 100644
> --- a/tools/libxc/include/xenguest.h
> +++ b/tools/libxc/include/xenguest.h
> @@ -30,6 +30,7 @@
> #define XCFLAGS_HVM (1 << 2)
> #define XCFLAGS_STDVGA (1 << 3)
> #define XCFLAGS_CHECKPOINT_COMPRESS (1 << 4)
> +#define XCFLAGS_CHECKPOINTED (1 << 5)
>
> #define X86_64_B_SIZE 64
> #define X86_32_B_SIZE 32
> diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
> index c4fe92c..c0f90d4 100644
> --- a/tools/libxc/xc_sr_common.h
> +++ b/tools/libxc/xc_sr_common.h
> @@ -174,6 +174,9 @@ struct xc_sr_context
> /* Live migrate vs non live suspend. */
> bool live;
>
> + /* Plain VM, or checkpoints over time. */
> + bool checkpointed;
> +
> /* Further debugging information in the stream. */
> bool debug;
>
> diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
> index 66fcd3e..caa727d 100644
> --- a/tools/libxc/xc_sr_save.c
> +++ b/tools/libxc/xc_sr_save.c
> @@ -732,6 +732,7 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom,
> ctx.save.callbacks = callbacks;
> ctx.save.live = !!(flags & XCFLAGS_LIVE);
> ctx.save.debug = !!(flags & XCFLAGS_DEBUG);
> + ctx.save.checkpointed = !!(flags & XCFLAGS_CHECKPOINTED);
>
> /*
> * TODO: Find some time to better tweak the live migration algorithm.
> @@ -745,6 +746,8 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom,
> /* Sanity checks for callbacks. */
> if ( hvm )
> assert(callbacks->switch_qemu_logdirty);
> + if ( ctx.save.checkpointed )
> + assert(callbacks->checkpoint && callbacks->postcopy);
>
> IPRINTF("In experimental %s", __func__);
> DPRINTF("fd %d, dom %u, max_iters %u, max_factor %u, flags %u, hvm %d",
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index f408646..a0c9850 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -2003,6 +2003,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
>
> if (r_info != NULL) {
> dss->interval = r_info->interval;
> + dss->xcflags |= XCFLAGS_CHECKPOINTED;
> if (libxl_defbool_val(r_info->compression))
> dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;
> }
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 04/14] libxc/migration: Pass checkpoint information into the save algorithm.
2015-05-13 15:49 ` Ian Campbell
@ 2015-05-14 1:03 ` Yang Hongyang
2015-05-14 11:17 ` Ian Campbell
0 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-14 1:03 UTC (permalink / raw)
To: Ian Campbell
Cc: wei.liu2, eddie.dong, wency, andrew.cooper3, yunhong.jiang,
ian.jackson, xen-devel, rshriram
On 05/13/2015 11:49 PM, Ian Campbell wrote:
> On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
>> From: Andrew Cooper <andrew.cooper3@citrix.com>
>
> OOI how was this signalled to the old code?
The old code check the callbacks "postcopy & checkpoint",
if the callbacks exists, it will call them which I think is
unreliable, so I add this flag to explicitly indicate a
checkpointed stream in the new code. However, it is backward
compatible, the legacy migration just don't know this flag
and will ignore it.
>
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Yang Hongyang <yanghy@cn.fujitsu.com>
>> ---
>> tools/libxc/include/xenguest.h | 1 +
>> tools/libxc/xc_sr_common.h | 3 +++
>> tools/libxc/xc_sr_save.c | 3 +++
>> tools/libxl/libxl_dom.c | 1 +
>> 4 files changed, 8 insertions(+)
>>
>> diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
>> index 8e39075..7581263 100644
>> --- a/tools/libxc/include/xenguest.h
>> +++ b/tools/libxc/include/xenguest.h
>> @@ -30,6 +30,7 @@
>> #define XCFLAGS_HVM (1 << 2)
>> #define XCFLAGS_STDVGA (1 << 3)
>> #define XCFLAGS_CHECKPOINT_COMPRESS (1 << 4)
>> +#define XCFLAGS_CHECKPOINTED (1 << 5)
>>
>> #define X86_64_B_SIZE 64
>> #define X86_32_B_SIZE 32
>> diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
>> index c4fe92c..c0f90d4 100644
>> --- a/tools/libxc/xc_sr_common.h
>> +++ b/tools/libxc/xc_sr_common.h
>> @@ -174,6 +174,9 @@ struct xc_sr_context
>> /* Live migrate vs non live suspend. */
>> bool live;
>>
>> + /* Plain VM, or checkpoints over time. */
>> + bool checkpointed;
>> +
>> /* Further debugging information in the stream. */
>> bool debug;
>>
>> diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
>> index 66fcd3e..caa727d 100644
>> --- a/tools/libxc/xc_sr_save.c
>> +++ b/tools/libxc/xc_sr_save.c
>> @@ -732,6 +732,7 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom,
>> ctx.save.callbacks = callbacks;
>> ctx.save.live = !!(flags & XCFLAGS_LIVE);
>> ctx.save.debug = !!(flags & XCFLAGS_DEBUG);
>> + ctx.save.checkpointed = !!(flags & XCFLAGS_CHECKPOINTED);
>>
>> /*
>> * TODO: Find some time to better tweak the live migration algorithm.
>> @@ -745,6 +746,8 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom,
>> /* Sanity checks for callbacks. */
>> if ( hvm )
>> assert(callbacks->switch_qemu_logdirty);
>> + if ( ctx.save.checkpointed )
>> + assert(callbacks->checkpoint && callbacks->postcopy);
>>
>> IPRINTF("In experimental %s", __func__);
>> DPRINTF("fd %d, dom %u, max_iters %u, max_factor %u, flags %u, hvm %d",
>> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
>> index f408646..a0c9850 100644
>> --- a/tools/libxl/libxl_dom.c
>> +++ b/tools/libxl/libxl_dom.c
>> @@ -2003,6 +2003,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
>>
>> if (r_info != NULL) {
>> dss->interval = r_info->interval;
>> + dss->xcflags |= XCFLAGS_CHECKPOINTED;
>> if (libxl_defbool_val(r_info->compression))
>> dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;
>> }
>
>
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 04/14] libxc/migration: Pass checkpoint information into the save algorithm.
2015-05-14 1:03 ` Yang Hongyang
@ 2015-05-14 11:17 ` Ian Campbell
0 siblings, 0 replies; 28+ messages in thread
From: Ian Campbell @ 2015-05-14 11:17 UTC (permalink / raw)
To: Yang Hongyang
Cc: wei.liu2, eddie.dong, wency, andrew.cooper3, yunhong.jiang,
ian.jackson, xen-devel, rshriram
On Thu, 2015-05-14 at 09:03 +0800, Yang Hongyang wrote:
>
> On 05/13/2015 11:49 PM, Ian Campbell wrote:
> > On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> >> From: Andrew Cooper <andrew.cooper3@citrix.com>
> >
> > OOI how was this signalled to the old code?
>
> The old code check the callbacks "postcopy & checkpoint",
> if the callbacks exists, it will call them which I think is
> unreliable, so I add this flag to explicitly indicate a
> checkpointed stream in the new code. However, it is backward
> compatible, the legacy migration just don't know this flag
> and will ignore it.
That makes sense, thanks.
I think it would be good to quickly mention this in the commit log. If
there is no other reason to make a v7 then you can just reply to v6 with
some text and I'll merge it in on commit.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 05/14] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW
2015-05-13 1:53 [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (3 preceding siblings ...)
2015-05-13 1:53 ` [PATCH v5 04/14] libxc/migration: Pass checkpoint information into the save algorithm Yang Hongyang
@ 2015-05-13 1:53 ` Yang Hongyang
2015-05-13 15:57 ` Ian Campbell
2015-05-13 1:53 ` [PATCH v5 06/14] tools/libxc: add a check in xc_hypercall_buffer_free_pages macro Yang Hongyang
` (9 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 1:53 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
* Re: [PATCH v5 05/14] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW
2015-05-13 1:53 ` [PATCH v5 05/14] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW Yang Hongyang
@ 2015-05-13 15:57 ` Ian Campbell
2015-05-14 1:06 ` Yang Hongyang
0 siblings, 1 reply; 28+ messages in thread
From: Ian Campbell @ 2015-05-13 15:57 UTC (permalink / raw)
To: Yang Hongyang
Cc: wei.liu2, wency, andrew.cooper3, yunhong.jiang, eddie.dong,
xen-devel, rshriram, ian.jackson
On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
> There are cases that we only need to use the hypercall buffer data,
"cases where"
> and do not use the xc_hypercall_buffer_t struct.
> DECLARE_HYPERCALL_BUFFER_SHADOW define a user pointer that can allow
"defines a user pointer"
> us to access the hypercall buffer data but it also define a
"defines"
> xc_hypercall_buffer_t that we don't use, the compiler will report arg
> unused error.
> add __attribute__((unused)) before xc_hypercall_buffer_t to avoid
> the compiler error.
>
> The cases for example:
"Example cases:"
> In send_all_pages(), we only need to use the haypercall buffer data
"hypercall"
> which is a dirty bitmap, we set the dirty bitmap to all dirty and call
> send_dirty_pages, we will not use the xc_hypercall_buffer_t and hypercall
> to retrive the dirty bitmap.
"retrieve"
> In send_some_pages(), we will also only need to use the dirty_bitmap.
> the retrive dirty bitmap hypercall are done by the caller.
and again.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> tools/libxc/include/xenctrl.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index a689caf..0804257 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -284,11 +284,15 @@ typedef struct xc_hypercall_buffer xc_hypercall_buffer_t;
> * Useful when a hypercall buffer is passed to a function and access
> * via the user pointer is required.
> *
> + * The shadow xc_hypercall_buffer_t may be unused, add
> + * __attribute__((unused)) to avoid compiler error.
No need for this comment IMHO.
> * See DECLARE_HYPERCALL_BUFFER_ARGUMENT() if the user pointer is not
> * required.
> */
> #define DECLARE_HYPERCALL_BUFFER_SHADOW(_type, _name, _hbuf) \
> _type *(_name) = (_hbuf)->hbuf; \
> + __attribute__((unused)) \
This is a somewhat unconventional location for such a tagm but putting
it elsewhere would involve faff-some rewrapping.
So apart from the typos looks good.
> xc_hypercall_buffer_t XC__HYPERCALL_BUFFER_NAME(_name) = { \
> .hbuf = (void *)-1, \
> .param_shadow = (_hbuf), \
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 05/14] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW
2015-05-13 15:57 ` Ian Campbell
@ 2015-05-14 1:06 ` Yang Hongyang
0 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-14 1:06 UTC (permalink / raw)
To: Ian Campbell
Cc: wei.liu2, wency, andrew.cooper3, yunhong.jiang, eddie.dong,
xen-devel, rshriram, ian.jackson
On 05/13/2015 11:57 PM, Ian Campbell wrote:
> On Wed, 2015-05-13 at 09:53 +0800, Yang Hongyang wrote:
>> There are cases that we only need to use the hypercall buffer data,
>
> "cases where"
>> and do not use the xc_hypercall_buffer_t struct.
>> DECLARE_HYPERCALL_BUFFER_SHADOW define a user pointer that can allow
>
> "defines a user pointer"
>
>> us to access the hypercall buffer data but it also define a
>
> "defines"
>
>> xc_hypercall_buffer_t that we don't use, the compiler will report arg
>> unused error.
>> add __attribute__((unused)) before xc_hypercall_buffer_t to avoid
>> the compiler error.
>>
>> The cases for example:
>
> "Example cases:"
>
>> In send_all_pages(), we only need to use the haypercall buffer data
>
> "hypercall"
>
>> which is a dirty bitmap, we set the dirty bitmap to all dirty and call
>> send_dirty_pages, we will not use the xc_hypercall_buffer_t and hypercall
>> to retrive the dirty bitmap.
>
> "retrieve"
>
>> In send_some_pages(), we will also only need to use the dirty_bitmap.
>> the retrive dirty bitmap hypercall are done by the caller.
>
> and again.
>
>>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> tools/libxc/include/xenctrl.h | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
>> index a689caf..0804257 100644
>> --- a/tools/libxc/include/xenctrl.h
>> +++ b/tools/libxc/include/xenctrl.h
>> @@ -284,11 +284,15 @@ typedef struct xc_hypercall_buffer xc_hypercall_buffer_t;
>> * Useful when a hypercall buffer is passed to a function and access
>> * via the user pointer is required.
>> *
>> + * The shadow xc_hypercall_buffer_t may be unused, add
>> + * __attribute__((unused)) to avoid compiler error.
>
> No need for this comment IMHO.
>
>> * See DECLARE_HYPERCALL_BUFFER_ARGUMENT() if the user pointer is not
>> * required.
>> */
>> #define DECLARE_HYPERCALL_BUFFER_SHADOW(_type, _name, _hbuf) \
>> _type *(_name) = (_hbuf)->hbuf; \
>> + __attribute__((unused)) \
>
> This is a somewhat unconventional location for such a tagm but putting
> it elsewhere would involve faff-some rewrapping.
>
> So apart from the typos looks good.
Will fix those typos, thank you!
>
>
>> xc_hypercall_buffer_t XC__HYPERCALL_BUFFER_NAME(_name) = { \
>> .hbuf = (void *)-1, \
>> .param_shadow = (_hbuf), \
>
>
> .
>
--
Thanks,
Yang.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 06/14] tools/libxc: add a check in xc_hypercall_buffer_free_pages macro
2015-05-13 1:53 [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (4 preceding siblings ...)
2015-05-13 1:53 ` [PATCH v5 05/14] tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW Yang Hongyang
@ 2015-05-13 1:53 ` Yang Hongyang
2015-05-13 1:53 ` [PATCH v5 07/14] libxc/save: introduce setup() and cleanup() on save Yang Hongyang
` (8 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 1:53 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 | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 0804257..202f612 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -353,7 +353,12 @@ void xc__hypercall_buffer_free(xc_interface *xch, xc_hypercall_buffer_t *b);
void *xc__hypercall_buffer_alloc_pages(xc_interface *xch, xc_hypercall_buffer_t *b, int nr_pages);
#define xc_hypercall_buffer_alloc_pages(_xch, _name, _nr) xc__hypercall_buffer_alloc_pages(_xch, HYPERCALL_BUFFER(_name), _nr)
void xc__hypercall_buffer_free_pages(xc_interface *xch, xc_hypercall_buffer_t *b, int nr_pages);
-#define xc_hypercall_buffer_free_pages(_xch, _name, _nr) xc__hypercall_buffer_free_pages(_xch, HYPERCALL_BUFFER(_name), _nr)
+#define xc_hypercall_buffer_free_pages(_xch, _name, _nr) \
+ do { \
+ if ( _name ) \
+ xc__hypercall_buffer_free_pages(_xch, HYPERCALL_BUFFER(_name), \
+ _nr); \
+ } while (0)
/*
* Array of hypercall buffers.
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 07/14] libxc/save: introduce setup() and cleanup() on save
2015-05-13 1:53 [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (5 preceding siblings ...)
2015-05-13 1:53 ` [PATCH v5 06/14] tools/libxc: add a check in xc_hypercall_buffer_free_pages macro Yang Hongyang
@ 2015-05-13 1:53 ` Yang Hongyang
2015-05-13 1:53 ` [PATCH v5 08/14] libxc/save: rename to_send to dirty_bitmap Yang Hongyang
` (7 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 1:53 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>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
tools/libxc/xc_sr_save.c | 34 +++++++++++++++++++++++++++-------
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index caa727d..f4ab5c5 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -637,6 +637,31 @@ static int send_domain_memory_nonlive(struct xc_sr_context *ctx)
return rc;
}
+static int setup(struct xc_sr_context *ctx)
+{
+ int rc;
+
+ rc = ctx->save.ops.setup(ctx);
+ if ( rc )
+ goto err;
+
+ rc = 0;
+
+ err:
+ return rc;
+}
+
+static void cleanup(struct xc_sr_context *ctx)
+{
+ xc_interface *xch = ctx->xch;
+
+ xc_shadow_control(xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_OFF,
+ NULL, 0, NULL, 0, NULL);
+
+ if ( ctx->save.ops.cleanup(ctx) )
+ PERROR("Failed to clean up");
+}
+
/*
* Save a domain.
*/
@@ -648,7 +673,7 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type)
IPRINTF("Saving domain %d, type %s",
ctx->domid, dhdr_type_to_str(guest_type));
- rc = ctx->save.ops.setup(ctx);
+ rc = setup(ctx);
if ( rc )
goto err;
@@ -701,12 +726,7 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type)
PERROR("Save failed");
done:
- xc_shadow_control(xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_OFF,
- NULL, 0, NULL, 0, NULL);
-
- rc = ctx->save.ops.cleanup(ctx);
- if ( rc )
- PERROR("Failed to clean up");
+ cleanup(ctx);
if ( saved_rc )
{
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 08/14] libxc/save: rename to_send to dirty_bitmap
2015-05-13 1:53 [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (6 preceding siblings ...)
2015-05-13 1:53 ` [PATCH v5 07/14] libxc/save: introduce setup() and cleanup() on save Yang Hongyang
@ 2015-05-13 1:53 ` Yang Hongyang
2015-05-13 1:53 ` [PATCH v5 09/14] libxc/save: adjust the memory allocation for migration Yang Hongyang
` (6 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 1:53 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>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
tools/libxc/xc_sr_save.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index f4ab5c5..5d08620 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -475,20 +475,20 @@ static int update_progress_string(struct xc_sr_context *ctx,
static int send_domain_memory_live(struct xc_sr_context *ctx)
{
xc_interface *xch = ctx->xch;
- DECLARE_HYPERCALL_BUFFER(unsigned long, to_send);
+ DECLARE_HYPERCALL_BUFFER(unsigned long, dirty_bitmap);
xc_shadow_op_stats_t stats = { 0, ctx->save.p2m_size };
char *progress_str = NULL;
unsigned x;
int rc = -1;
- to_send = xc_hypercall_buffer_alloc_pages(
- xch, to_send, NRPAGES(bitmap_size(ctx->save.p2m_size)));
+ dirty_bitmap = xc_hypercall_buffer_alloc_pages(
+ xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->save.p2m_size)));
ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE *
sizeof(*ctx->save.batch_pfns));
ctx->save.deferred_pages = calloc(1, bitmap_size(ctx->save.p2m_size));
- if ( !ctx->save.batch_pfns || !to_send || !ctx->save.deferred_pages )
+ if ( !ctx->save.batch_pfns || !dirty_bitmap || !ctx->save.deferred_pages )
{
ERROR("Unable to allocate memory for to_{send,fix}/batch bitmaps");
goto out;
@@ -512,7 +512,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
{
if ( xc_shadow_control(
xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_CLEAN,
- HYPERCALL_BUFFER(to_send), ctx->save.p2m_size,
+ HYPERCALL_BUFFER(dirty_bitmap), ctx->save.p2m_size,
NULL, 0, &stats) != ctx->save.p2m_size )
{
PERROR("Failed to retrieve logdirty bitmap");
@@ -527,7 +527,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
if ( rc )
goto out;
- rc = send_some_pages(ctx, to_send, stats.dirty_count);
+ rc = send_some_pages(ctx, dirty_bitmap, stats.dirty_count);
if ( rc )
goto out;
}
@@ -538,7 +538,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
if ( xc_shadow_control(
xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_CLEAN,
- HYPERCALL_BUFFER(to_send), ctx->save.p2m_size,
+ HYPERCALL_BUFFER(dirty_bitmap), ctx->save.p2m_size,
NULL, 0, &stats) != ctx->save.p2m_size )
{
PERROR("Failed to retrieve logdirty bitmap");
@@ -550,9 +550,9 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
if ( rc )
goto out;
- bitmap_or(to_send, ctx->save.deferred_pages, ctx->save.p2m_size);
+ bitmap_or(dirty_bitmap, ctx->save.deferred_pages, ctx->save.p2m_size);
- rc = send_some_pages(ctx, to_send,
+ rc = send_some_pages(ctx, dirty_bitmap,
stats.dirty_count + ctx->save.nr_deferred_pages);
if ( rc )
goto out;
@@ -578,7 +578,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
if ( xc_shadow_control(
xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_PEEK,
- HYPERCALL_BUFFER(to_send), ctx->save.p2m_size,
+ HYPERCALL_BUFFER(dirty_bitmap), ctx->save.p2m_size,
NULL, 0, &stats) != ctx->save.p2m_size )
{
PERROR("Failed to retrieve logdirty bitmap");
@@ -593,7 +593,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
out:
xc_set_progress_prefix(xch, NULL);
free(progress_str);
- xc_hypercall_buffer_free_pages(xch, to_send,
+ xc_hypercall_buffer_free_pages(xch, dirty_bitmap,
NRPAGES(bitmap_size(ctx->save.p2m_size)));
free(ctx->save.deferred_pages);
free(ctx->save.batch_pfns);
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 09/14] libxc/save: adjust the memory allocation for migration
2015-05-13 1:53 [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (7 preceding siblings ...)
2015-05-13 1:53 ` [PATCH v5 08/14] libxc/save: rename to_send to dirty_bitmap Yang Hongyang
@ 2015-05-13 1:53 ` Yang Hongyang
2015-05-13 1:54 ` [PATCH v5 10/14] libxc/save: remove bitmap param from send_some_pages Yang Hongyang
` (5 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 1:53 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>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
tools/libxc/xc_sr_common.h | 1 +
tools/libxc/xc_sr_save.c | 60 ++++++++++++++++++++++------------------------
2 files changed, 29 insertions(+), 32 deletions(-)
diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index c0f90d4..276d00a 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -190,6 +190,7 @@ struct xc_sr_context
unsigned nr_batch_pfns;
unsigned long *deferred_pages;
unsigned long nr_deferred_pages;
+ xc_hypercall_buffer_t dirty_bitmap_hbuf;
} save;
struct /* Restore data. */
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index 5d08620..beb54c4 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -475,24 +475,12 @@ static int update_progress_string(struct xc_sr_context *ctx,
static int send_domain_memory_live(struct xc_sr_context *ctx)
{
xc_interface *xch = ctx->xch;
- DECLARE_HYPERCALL_BUFFER(unsigned long, dirty_bitmap);
xc_shadow_op_stats_t stats = { 0, ctx->save.p2m_size };
char *progress_str = NULL;
unsigned x;
int rc = -1;
-
- dirty_bitmap = xc_hypercall_buffer_alloc_pages(
- xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->save.p2m_size)));
-
- ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE *
- sizeof(*ctx->save.batch_pfns));
- ctx->save.deferred_pages = calloc(1, bitmap_size(ctx->save.p2m_size));
-
- if ( !ctx->save.batch_pfns || !dirty_bitmap || !ctx->save.deferred_pages )
- {
- ERROR("Unable to allocate memory for to_{send,fix}/batch bitmaps");
- goto out;
- }
+ DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
+ &ctx->save.dirty_bitmap_hbuf);
rc = enable_logdirty(ctx);
if ( rc )
@@ -593,10 +581,6 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
out:
xc_set_progress_prefix(xch, NULL);
free(progress_str);
- xc_hypercall_buffer_free_pages(xch, dirty_bitmap,
- NRPAGES(bitmap_size(ctx->save.p2m_size)));
- free(ctx->save.deferred_pages);
- free(ctx->save.batch_pfns);
return rc;
}
@@ -609,17 +593,6 @@ static int send_domain_memory_nonlive(struct xc_sr_context *ctx)
xc_interface *xch = ctx->xch;
int rc = -1;
- ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE *
- sizeof(*ctx->save.batch_pfns));
- ctx->save.deferred_pages = calloc(1, bitmap_size(ctx->save.p2m_size));
-
- if ( !ctx->save.batch_pfns || !ctx->save.deferred_pages )
- {
- PERROR("Failed to allocate memory for nonlive tracking structures");
- errno = ENOMEM;
- goto err;
- }
-
rc = suspend_domain(ctx);
if ( rc )
goto err;
@@ -631,15 +604,30 @@ static int send_domain_memory_nonlive(struct xc_sr_context *ctx)
goto err;
err:
- free(ctx->save.deferred_pages);
- free(ctx->save.batch_pfns);
-
return rc;
}
static int setup(struct xc_sr_context *ctx)
{
+ xc_interface *xch = ctx->xch;
int rc;
+ DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
+ &ctx->save.dirty_bitmap_hbuf);
+
+ dirty_bitmap = xc_hypercall_buffer_alloc_pages(
+ xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->save.p2m_size)));
+ ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE *
+ sizeof(*ctx->save.batch_pfns));
+ ctx->save.deferred_pages = calloc(1, bitmap_size(ctx->save.p2m_size));
+
+ if ( !ctx->save.batch_pfns || !dirty_bitmap || !ctx->save.deferred_pages )
+ {
+ ERROR("Unable to allocate memory for dirty bitmaps, batch pfns and"
+ " deferred pages");
+ rc = -1;
+ errno = ENOMEM;
+ goto err;
+ }
rc = ctx->save.ops.setup(ctx);
if ( rc )
@@ -654,12 +642,20 @@ static int setup(struct xc_sr_context *ctx)
static void cleanup(struct xc_sr_context *ctx)
{
xc_interface *xch = ctx->xch;
+ DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
+ &ctx->save.dirty_bitmap_hbuf);
+
xc_shadow_control(xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_OFF,
NULL, 0, NULL, 0, NULL);
if ( ctx->save.ops.cleanup(ctx) )
PERROR("Failed to clean up");
+
+ xc_hypercall_buffer_free_pages(xch, dirty_bitmap,
+ NRPAGES(bitmap_size(ctx->save.p2m_size)));
+ free(ctx->save.deferred_pages);
+ free(ctx->save.batch_pfns);
}
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 10/14] libxc/save: remove bitmap param from send_some_pages
2015-05-13 1:53 [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (8 preceding siblings ...)
2015-05-13 1:53 ` [PATCH v5 09/14] libxc/save: adjust the memory allocation for migration Yang Hongyang
@ 2015-05-13 1:54 ` Yang Hongyang
2015-05-13 1:54 ` [PATCH v5 11/14] libxc/save: rename send_some_pages to send_dirty_pages Yang Hongyang
` (4 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 1:54 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>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
tools/libxc/xc_sr_save.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index beb54c4..adb5cce 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -374,23 +374,24 @@ static int send_all_pages(struct xc_sr_context *ctx)
}
/*
- * Send a subset of pages in the guests p2m, according to the provided bitmap.
+ * Send a subset of pages in the guests p2m, according to the dirty bitmap.
* Used for each subsequent iteration of the live migration loop.
*
* Bitmap is bounded by p2m_size.
*/
static int send_some_pages(struct xc_sr_context *ctx,
- unsigned long *bitmap,
unsigned long entries)
{
xc_interface *xch = ctx->xch;
xen_pfn_t p;
unsigned long written;
int rc;
+ DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
+ &ctx->save.dirty_bitmap_hbuf);
for ( p = 0, written = 0; p < ctx->save.p2m_size; ++p )
{
- if ( !test_bit(p, bitmap) )
+ if ( !test_bit(p, dirty_bitmap) )
continue;
rc = add_to_batch(ctx, p);
@@ -515,7 +516,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
if ( rc )
goto out;
- rc = send_some_pages(ctx, dirty_bitmap, stats.dirty_count);
+ rc = send_some_pages(ctx, stats.dirty_count);
if ( rc )
goto out;
}
@@ -540,8 +541,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
bitmap_or(dirty_bitmap, ctx->save.deferred_pages, ctx->save.p2m_size);
- rc = send_some_pages(ctx, dirty_bitmap,
- stats.dirty_count + ctx->save.nr_deferred_pages);
+ rc = send_some_pages(ctx, stats.dirty_count + ctx->save.nr_deferred_pages);
if ( rc )
goto out;
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 11/14] libxc/save: rename send_some_pages to send_dirty_pages
2015-05-13 1:53 [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (9 preceding siblings ...)
2015-05-13 1:54 ` [PATCH v5 10/14] libxc/save: remove bitmap param from send_some_pages Yang Hongyang
@ 2015-05-13 1:54 ` Yang Hongyang
2015-05-13 1:54 ` [PATCH v5 12/14] libxc/save: reuse send_dirty_pages() in send_all_pages() Yang Hongyang
` (3 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 1:54 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 adb5cce..3768bda 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -379,8 +379,8 @@ static int send_all_pages(struct xc_sr_context *ctx)
*
* Bitmap is bounded by p2m_size.
*/
-static int send_some_pages(struct xc_sr_context *ctx,
- unsigned long entries)
+static int send_dirty_pages(struct xc_sr_context *ctx,
+ unsigned long entries)
{
xc_interface *xch = ctx->xch;
xen_pfn_t p;
@@ -516,7 +516,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
if ( rc )
goto out;
- rc = send_some_pages(ctx, stats.dirty_count);
+ rc = send_dirty_pages(ctx, stats.dirty_count);
if ( rc )
goto out;
}
@@ -541,7 +541,7 @@ static int send_domain_memory_live(struct xc_sr_context *ctx)
bitmap_or(dirty_bitmap, ctx->save.deferred_pages, ctx->save.p2m_size);
- rc = send_some_pages(ctx, stats.dirty_count + ctx->save.nr_deferred_pages);
+ rc = send_dirty_pages(ctx, stats.dirty_count + ctx->save.nr_deferred_pages);
if ( rc )
goto out;
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 12/14] libxc/save: reuse send_dirty_pages() in send_all_pages()
2015-05-13 1:53 [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (10 preceding siblings ...)
2015-05-13 1:54 ` [PATCH v5 11/14] libxc/save: rename send_some_pages to send_dirty_pages Yang Hongyang
@ 2015-05-13 1:54 ` Yang Hongyang
2015-05-13 1:54 ` [PATCH v5 13/14] libxc/restore: introduce process_record() Yang Hongyang
` (2 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 1:54 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 3768bda..1d0a46d 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -344,36 +344,6 @@ static int suspend_domain(struct xc_sr_context *ctx)
}
/*
- * Send all pages in the guests p2m. Used as the first iteration of the live
- * migration loop, and for a non-live save.
- */
-static int send_all_pages(struct xc_sr_context *ctx)
-{
- xc_interface *xch = ctx->xch;
- xen_pfn_t p;
- int rc;
-
- for ( p = 0; p < ctx->save.p2m_size; ++p )
- {
- rc = add_to_batch(ctx, p);
- if ( rc )
- return rc;
-
- /* Update progress every 4MB worth of memory sent. */
- if ( (p & ((1U << (22 - 12)) - 1)) == 0 )
- xc_report_progress_step(xch, p, ctx->save.p2m_size);
- }
-
- rc = flush_batch(ctx);
- if ( rc )
- return rc;
-
- xc_report_progress_step(xch, ctx->save.p2m_size,
- ctx->save.p2m_size);
- return 0;
-}
-
-/*
* Send a subset of pages in the guests p2m, according to the dirty bitmap.
* Used for each subsequent iteration of the live migration loop.
*
@@ -416,6 +386,20 @@ static int send_dirty_pages(struct xc_sr_context *ctx,
return 0;
}
+/*
+ * Send all pages in the guests p2m. Used as the first iteration of the live
+ * migration loop, and for a non-live save.
+ */
+static int send_all_pages(struct xc_sr_context *ctx)
+{
+ DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
+ &ctx->save.dirty_bitmap_hbuf);
+
+ bitmap_set(dirty_bitmap, ctx->save.p2m_size);
+
+ return send_dirty_pages(ctx, ctx->save.p2m_size);
+}
+
static int enable_logdirty(struct xc_sr_context *ctx)
{
xc_interface *xch = ctx->xch;
--
1.9.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 13/14] libxc/restore: introduce process_record()
2015-05-13 1:53 [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (11 preceding siblings ...)
2015-05-13 1:54 ` [PATCH v5 12/14] libxc/save: reuse send_dirty_pages() in send_all_pages() Yang Hongyang
@ 2015-05-13 1:54 ` Yang Hongyang
2015-05-13 1:54 ` [PATCH v5 14/14] libxc/restore: split read/handle qemu info Yang Hongyang
2015-05-13 7:54 ` [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Andrew Cooper
14 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 1:54 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 v5 14/14] libxc/restore: split read/handle qemu info
2015-05-13 1:53 [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (12 preceding siblings ...)
2015-05-13 1:54 ` [PATCH v5 13/14] libxc/restore: introduce process_record() Yang Hongyang
@ 2015-05-13 1:54 ` Yang Hongyang
2015-05-13 7:54 ` [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Andrew Cooper
14 siblings, 0 replies; 28+ messages in thread
From: Yang Hongyang @ 2015-05-13 1:54 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 v5 00/14] Misc patches to aid migration v2 Remus support
2015-05-13 1:53 [PATCH v5 00/14] Misc patches to aid migration v2 Remus support Yang Hongyang
` (13 preceding siblings ...)
2015-05-13 1:54 ` [PATCH v5 14/14] libxc/restore: split read/handle qemu info Yang Hongyang
@ 2015-05-13 7:54 ` Andrew Cooper
14 siblings, 0 replies; 28+ messages in thread
From: Andrew Cooper @ 2015-05-13 7:54 UTC (permalink / raw)
To: Yang Hongyang, xen-devel
Cc: wei.liu2, ian.campbell, wency, ian.jackson, yunhong.jiang,
eddie.dong, rshriram
On 13/05/2015 02:53, Yang Hongyang wrote:
> This is the combination of Andrew Cooper's misc patches and mine
> to aid migration v2 Remus support.
Everything looks in order now from my point of view!
~Andrew
>
> See individual patches for details.
>
> Git tree available at:
> https://github.com/macrosheep/xen/tree/misc-remus-v5
>
> v4->v5
> Most of the changes are trival, like drop brackets in
> DECLARE_HYPERCALL_BUFFER_SHADOW(), drop stray blank line etc.
> Except the 6th patch which add a do{}while (0) in
> xc_hypercall_buffer_free_pages macro.
>
> Summary of changes:
> M = modified
> A = acked
> no mark = unchanged from last round
>
> Andrew Cooper (4):
> libxc/migration: Be rather stricter with illformed callers
> libxc/save: Adjust stream-position callbacks for checkpointed streams
> M libxc/migration: Specification update for CHECKPOINT records
> libxc/migration: Pass checkpoint information into the save algorithm.
>
> Yang Hongyang (10):
> tools/libxc: unused attribute in DECLARE_HYPERCALL_BUFFER_SHADOW
> M tools/libxc: add a check in xc_hypercall_buffer_free_pages macro
> M libxc/save: introduce setup() and cleanup() on save
> libxc/save: rename to_send to dirty_bitmap
> M libxc/save: adjust the memory allocation for migration
> M libxc/save: remove bitmap param from send_some_pages
> libxc/save: rename send_some_pages to send_dirty_pages
> M libxc/save: reuse send_dirty_pages() in send_all_pages()
> libxc/restore: introduce process_record()
> libxc/restore: split read/handle qemu info
>
> docs/specs/libxc-migration-stream.pandoc | 29 +++++-
> tools/libxc/include/xenctrl.h | 11 +-
> tools/libxc/include/xenguest.h | 1 +
> tools/libxc/xc_bitops.h | 5 +
> tools/libxc/xc_sr_common.c | 1 +
> tools/libxc/xc_sr_common.h | 30 ++++--
> tools/libxc/xc_sr_restore.c | 89 ++++++++++------
> tools/libxc/xc_sr_restore_x86_hvm.c | 28 ++++-
> tools/libxc/xc_sr_save.c | 171 ++++++++++++++++---------------
> tools/libxc/xc_sr_save_x86_hvm.c | 30 +++---
> tools/libxc/xc_sr_save_x86_pv.c | 29 ++++--
> tools/libxc/xc_sr_stream_format.h | 1 +
> tools/libxl/libxl_dom.c | 1 +
> 13 files changed, 272 insertions(+), 154 deletions(-)
>
^ permalink raw reply [flat|nested] 28+ messages in thread