Re: [Xen-devel] [PATCH v11 07/27] docs/libxl: Introduce CHECKPOINT_CONTEXT to support migration v2 colo streams

[Wen Congyang <wency@xxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxx>
> 
> 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.
> 
> 
> .
> 




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel