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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 25/07/16 11:44, David Vrabel wrote:
> 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@xxxxxxxxxx>
>>>>>> ---
>>>>>> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
>>>>>> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
>>>>>> ---
>>>>>>  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@xxxxxxxxxx>>
>>>>>>    Wen Congyang <<wency@xxxxxxxxxxxxxx>>
>>>>>>    Yang Hongyang <<hongyang.yang@xxxxxxxxxxxx>>
>>>>>> -% 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.

I disagree, and do not consider this broken.

~Andrew

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