Re: [PATCH v8 --for 4.6 COLO 02/25] docs/libxl: Introduce COLO_CONTEXT to support migration v2 colo streams

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 16/07/15 11:11, Yang Hongyang wrote:
>
>
> On 07/16/2015 05:45 PM, Andrew Cooper wrote:
>> On 16/07/15 07:32, Yang Hongyang wrote:
>>>
>>>
>>> On 07/16/2015 12:52 AM, Andrew Cooper wrote:
>>>> On 15/07/15 10:18, Yang Hongyang wrote:
>>>>> From: Wen Congyang <wency@cn.fujitsu.com>
>>>>>
>>>>> 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
>>>>>
>>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>>>> ---
>>>>>    docs/specs/libxl-migration-stream.pandoc | 22
>>>>> +++++++++++++++++++++-
>>>>>    tools/libxl/libxl_sr_stream_format.h     | 11 +++++++++++
>>>>>    tools/python/xen/migration/libxl.py      |  9 +++++++++
>>>>>    3 files changed, 41 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/docs/specs/libxl-migration-stream.pandoc
>>>>> b/docs/specs/libxl-migration-stream.pandoc
>>>>> index c24a434..5986273 100644
>>>>> --- a/docs/specs/libxl-migration-stream.pandoc
>>>>> +++ b/docs/specs/libxl-migration-stream.pandoc
>>>>> @@ -121,7 +121,9 @@ type         0x00000000: END
>>>>>
>>>>>                 0x00000004: CHECKPOINT_END
>>>>>
>>>>> -             0x00000005 - 0x7FFFFFFF: Reserved for future
>>>>> _mandatory_
>>>>> +             0x00000005: COLO_CONTEXT
>>>>> +
>>>>> +             0x00000006 - 0x7FFFFFFF: Reserved for future
>>>>> _mandatory_
>>>>>                 records.
>>>>>
>>>>>                 0x80000000 - 0xFFFFFFFF: Reserved for future
>>>>> _optional_
>>>>> @@ -215,3 +217,21 @@ A checkpoint end record marks the end of a
>>>>> checkpoint in the image.
>>>>>        +-------------------------------------------------+
>>>>>
>>>>>    The end record contains no fields; its body_length is 0.
>>>>> +
>>>>> +COLO\_CONTEXT
>>>>> +--------------
>>>>> +
>>>>> +A COLO context record contains the control information for COLO.
>>>>> +
>>>>> +     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
>>>>> +                 0x00000001: Secondary VM is suspended
>>>>> +                 0x00000002: Secondary VM is ready
>>>>> +                 0x00000003: Secondary VM is resumed
>>>>
>>>> This style of table in pandoc need to be terminated with a line of
>>>> -------, just like the head of the table.
>>>
>>> Ok
>>>
>>>>
>>>> Also, I wonder at the name "COLO_CONTEXT".  CONTEXT implies an
>>>> associated blob of data, but this is not the case here.  Here, it is
>>>> more of a status update, with expected actions on some states.
>>>
>>> True, could you suggest a better name? sorry for my bad English...
>>
>> In hindsight, I would also avoid putting COLO in the name.
>>
>> How about CHECKPOINT_SECONDARY_STATE ?
>
> From my another mail, I explained the COLO_CONTEXT, do you still
> suggest CHECKPOINT_SECONDARY_STATE? IMO, it's more like a sync
> command?

Right, but what it contains is information concerning the current state
of the secondary VM.

The real question is how you see this record being extended with further
control_id's in the future.

(naming stuff is hard :( )

~Andrew