| Message ID | 1457080891-26054-8-git-send-email-xiecl.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Changlong Xie writes ("[PATCH v11 07/27] docs/libxl: Introduce CHECKPOINT_CONTEXT to support migration v2 colo streams"):
> From: Wen Congyang <wency@cn.fujitsu.com>
I think we will want to see an ack from Andy Cooper on this, in due
course.
> It is the negotiation record for COLO.
> Primary->Secondary:
> control_id 0x00000000: Secondary VM is out of sync, start a new checkpoint
> Secondary->Primary:
> 0x00000001: Secondary VM is suspended
> 0x00000002: Secondary VM is ready
> 0x00000003: Secondary VM is resumed
I don't think it is necessary to repeat the enum assignment here in
the commit message.
> +CHECKPOINT\_STATE
> +--------------
This documentation patch ought to explicitly mention COLO, and have
cross-references to the various documents (eg, the README added in the
previous patch).
> +A checkpoint state record contains the control information for checkpoint.
> +
> + 0 1 2 3 4 5 6 7 octet
> + +------------------------+------------------------+
> + | control_id | padding |
> + +------------------------+------------------------+
> +
> +--------------------------------------------------------------------
> +Field Description
> +------------ ---------------------------------------------------
> +control_id 0x00000000: Secondary VM is out of sync, start a new checkpoint
> + (Primary -> Secondary)
> +
> + 0x00000001: Secondary VM is suspended (Secondary -> Primary)
> +
> + 0x00000002: Secondary VM is ready (Secondary -> Primary)
> +
> + 0x00000003: Secondary VM is resumed (Secondary -> Primary)
I think this should be accompanied by an explanation of what order
these messages are sent in, and what both ends may or may not do
during that time.
> @@ -212,6 +214,11 @@ class VerifyLibxl(VerifyBase):
> if len(content) != 0:
> raise RecordError("Checkpoint end record with non-zero length")
>
> + def verify_record_checkpoint_state(self, content):
> + """ Checkpoint state """
> + if len(content) == 0:
> + raise RecordError("Checkpoint state record with zero length")
> +
I'm not verify familiar with this area of the code, but I think that
this should probably check that the control_id is as expected. Can it
know what the right sequencing is ?
Ian.
On Fri, Mar 04, 2016 at 04:51:20PM +0000, Ian Jackson wrote: [...] > > > @@ -212,6 +214,11 @@ class VerifyLibxl(VerifyBase): > > if len(content) != 0: > > raise RecordError("Checkpoint end record with non-zero length") > > > > + def verify_record_checkpoint_state(self, content): > > + """ Checkpoint state """ > > + if len(content) == 0: > > + raise RecordError("Checkpoint state record with zero length") > > + > > I'm not verify familiar with this area of the code, but I think that > this should probably check that the control_id is as expected. Can it > know what the right sequencing is ? > FWIW this script is not used in live system -- so it probably doesn't have information on the control id and the sequence on a live system. Wei. > Ian.
On 03/05/2016 12:51 AM, Ian Jackson wrote: > Changlong Xie writes ("[PATCH v11 07/27] docs/libxl: Introduce CHECKPOINT_CONTEXT to support migration v2 colo streams"): >> From: Wen Congyang <wency@cn.fujitsu.com> > > I think we will want to see an ack from Andy Cooper on this, in due > course. > >> It is the negotiation record for COLO. >> Primary->Secondary: >> control_id 0x00000000: Secondary VM is out of sync, start a new checkpoint >> Secondary->Primary: >> 0x00000001: Secondary VM is suspended >> 0x00000002: Secondary VM is ready >> 0x00000003: Secondary VM is resumed > > I don't think it is necessary to repeat the enum assignment here in > the commit message. OK, will fix it in the next version. > > >> +CHECKPOINT\_STATE >> +-------------- > > This documentation patch ought to explicitly mention COLO, and have > cross-references to the various documents (eg, the README added in the > previous patch). > >> +A checkpoint state record contains the control information for checkpoint. >> + >> + 0 1 2 3 4 5 6 7 octet >> + +------------------------+------------------------+ >> + | control_id | padding | >> + +------------------------+------------------------+ >> + >> +-------------------------------------------------------------------- >> +Field Description >> +------------ --------------------------------------------------- >> +control_id 0x00000000: Secondary VM is out of sync, start a new checkpoint >> + (Primary -> Secondary) >> + >> + 0x00000001: Secondary VM is suspended (Secondary -> Primary) >> + >> + 0x00000002: Secondary VM is ready (Secondary -> Primary) >> + >> + 0x00000003: Secondary VM is resumed (Secondary -> Primary) > > I think this should be accompanied by an explanation of what order > these messages are sent in, and what both ends may or may not do > during that time. OK, will fix it in the next version. > > >> @@ -212,6 +214,11 @@ class VerifyLibxl(VerifyBase): >> if len(content) != 0: >> raise RecordError("Checkpoint end record with non-zero length") >> >> + def verify_record_checkpoint_state(self, content): >> + """ Checkpoint state """ >> + if len(content) == 0: >> + raise RecordError("Checkpoint state record with zero length") >> + > > I'm not verify familiar with this area of the code, but I think that > this should probably check that the control_id is as expected. Can it > know what the right sequencing is ? To Andrew Cooper: What is the purpost of this script? If it is not used for live system, I think the stream should not contain checkpoint state record. Thanks Wen Congyang > > Ian. > > > . >
diff --git a/docs/specs/libxl-migration-stream.pandoc b/docs/specs/libxl-migration-stream.pandoc index 2c97d86..5d577ac 100644 --- a/docs/specs/libxl-migration-stream.pandoc +++ b/docs/specs/libxl-migration-stream.pandoc @@ -1,6 +1,8 @@ % LibXenLight Domain Image Format % Andrew Cooper <<andrew.cooper3@citrix.com>> -% Revision 1 + Wen Congyang <<wency@cn.fujitsu.com>> + Yang Hongyang <<hongyang.yang@easystack.cn>> +% Revision 2 Introduction ============ @@ -119,7 +121,9 @@ type 0x00000000: END 0x00000004: CHECKPOINT_END - 0x00000005 - 0x7FFFFFFF: Reserved for future _mandatory_ + 0x00000005: CHECKPOINT_STATE + + 0x00000006 - 0x7FFFFFFF: Reserved for future _mandatory_ records. 0x80000000 - 0xFFFFFFFF: Reserved for future _optional_ @@ -249,6 +253,29 @@ A checkpoint end record marks the end of a checkpoint in the image. The end record contains no fields; its body_length is 0. +CHECKPOINT\_STATE +-------------- + +A checkpoint state record contains the control information for checkpoint. + + 0 1 2 3 4 5 6 7 octet + +------------------------+------------------------+ + | control_id | padding | + +------------------------+------------------------+ + +-------------------------------------------------------------------- +Field Description +------------ --------------------------------------------------- +control_id 0x00000000: Secondary VM is out of sync, start a new checkpoint + (Primary -> Secondary) + + 0x00000001: Secondary VM is suspended (Secondary -> Primary) + + 0x00000002: Secondary VM is ready (Secondary -> Primary) + + 0x00000003: Secondary VM is resumed (Secondary -> Primary) +-------------------------------------------------------------------- + Future Extensions ================= diff --git a/tools/libxl/libxl_sr_stream_format.h b/tools/libxl/libxl_sr_stream_format.h index 54da360..75f5190 100644 --- a/tools/libxl/libxl_sr_stream_format.h +++ b/tools/libxl/libxl_sr_stream_format.h @@ -36,6 +36,7 @@ typedef struct libxl__sr_rec_hdr #define REC_TYPE_EMULATOR_XENSTORE_DATA 0x00000002U #define REC_TYPE_EMULATOR_CONTEXT 0x00000003U #define REC_TYPE_CHECKPOINT_END 0x00000004U +#define REC_TYPE_CHECKPOINT_STATE 0x00000005U typedef struct libxl__sr_emulator_hdr { @@ -47,6 +48,16 @@ typedef struct libxl__sr_emulator_hdr #define EMULATOR_QEMU_TRADITIONAL 0x00000001U #define EMULATOR_QEMU_UPSTREAM 0x00000002U +typedef struct libxl_sr_checkpoint_state +{ + uint32_t id; +} libxl_sr_checkpoint_state; + +#define CHECKPOINT_NEW 0x00000000U +#define CHECKPOINT_SVM_SUSPENDED 0x00000001U +#define CHECKPOINT_SVM_READY 0x00000002U +#define CHECKPOINT_SVM_RESUMED 0x00000003U + #endif /* LIBXL__SR_STREAM_FORMAT_H */ /* diff --git a/tools/python/xen/migration/libxl.py b/tools/python/xen/migration/libxl.py index fc0acf6..d5f54dc 100644 --- a/tools/python/xen/migration/libxl.py +++ b/tools/python/xen/migration/libxl.py @@ -37,6 +37,7 @@ REC_TYPE_libxc_context = 0x00000001 REC_TYPE_emulator_xenstore_data = 0x00000002 REC_TYPE_emulator_context = 0x00000003 REC_TYPE_checkpoint_end = 0x00000004 +REC_TYPE_checkpoint_state = 0x00000005 rec_type_to_str = { REC_TYPE_end : "End", @@ -44,6 +45,7 @@ rec_type_to_str = { REC_TYPE_emulator_xenstore_data : "Emulator xenstore data", REC_TYPE_emulator_context : "Emulator context", REC_TYPE_checkpoint_end : "Checkpoint end", + REC_TYPE_checkpoint_state : "Checkpoint state" } # emulator_* header @@ -212,6 +214,11 @@ class VerifyLibxl(VerifyBase): if len(content) != 0: raise RecordError("Checkpoint end record with non-zero length") + def verify_record_checkpoint_state(self, content): + """ Checkpoint state """ + if len(content) == 0: + raise RecordError("Checkpoint state record with zero length") + record_verifiers = { REC_TYPE_end: @@ -224,4 +231,6 @@ record_verifiers = { VerifyLibxl.verify_record_emulator_context, REC_TYPE_checkpoint_end: VerifyLibxl.verify_record_checkpoint_end, + REC_TYPE_checkpoint_state: + VerifyLibxl.verify_record_checkpoint_state, }