Re: [PATCH 1/4] docs: Clarify the expected behaviour of zero length records

[David Vrabel <david.vrabel@citrix.com>]

On 25/07/16 11:38, Andrew Cooper wrote:
> On 25/07/16 11:35, David Vrabel wrote:
>> On 25/07/16 11:25, Andrew Cooper wrote:
>>> On 25/07/16 11:21, David Vrabel wrote:
>>>> On 21/07/16 18:17, Andrew Cooper wrote:
>>>>> The sending side shouldn't send any variable sized records which end up having
>>>>> zero content, but the receiving side will need to tolerate such records for
>>>>> compatibility purposes.
>>>>>
>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>> ---
>>>>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>>>>> CC: Wei Liu <wei.liu2@citrix.com>
>>>>> ---
>>>>>  docs/specs/libxc-migration-stream.pandoc | 16 +++++++++++++++-
>>>>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/docs/specs/libxc-migration-stream.pandoc b/docs/specs/libxc-migration-stream.pandoc
>>>>> index 31eba10..a90bc5d 100644
>>>>> --- a/docs/specs/libxc-migration-stream.pandoc
>>>>> +++ b/docs/specs/libxc-migration-stream.pandoc
>>>>> @@ -3,7 +3,7 @@
>>>>>    Andrew Cooper <<andrew.cooper3@citrix.com>>
>>>>>    Wen Congyang <<wency@cn.fujitsu.com>>
>>>>>    Yang Hongyang <<hongyang.yang@easystack.cn>>
>>>>> -% Revision 1
>>>>> +% Revision 2
>>>>>  
>>>>>  Introduction
>>>>>  ============
>>>>> @@ -631,6 +631,10 @@ The set of valid records depends on the guest architecture and type.  No
>>>>>  assumptions should be made about the ordering or interleaving of
>>>>>  independent records.  Record dependencies are noted below.
>>>>>  
>>>>> +Some records have an exactly specified size.  Some records have variable size
>>>>> +depending on their content.  A record with variable size which ends up being
>>>>> +zero should be omitted entirely from the stream by the sending side.
>>>> I disagree. I think the stream should include the records with the empty
>>>> content.  This gives better consistency and does not require changes to
>>>> the stream.
>>> There are already some which are properly omitted, like the vcpu records
>>> for offline vcpus.
>>>
>>> There is no point having empty records; omitting them is an optimisation
>>> which we absolutely shouldn't preclude.
>> The optimization doesn't matter since these records are so tiny.
>>
>> I've expanded on why these should be included in another reply.
> 
> We already omit most zero-length records.  That ship has already sailed.
> 
> This bug only presents itself because of two hypercalls in Xen returning
> different size characteristics.  We already checked for zero first, and
> fail to check for zero the second time.

Then document that those specific records may be omitted, but don't
spread the brokenness to other (future) records.

David


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