Re: [Xen-devel] [PATCH v4 10/12] livepatch: Handle arbitrary size names with the list operation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.09.2019 12:58,  Wieczorkiewicz, Pawel  wrote:
> 
> 
>> On 30. Sep 2019, at 10:50, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>
>> On 28.09.2019 17:13, Pawel Wieczorkiewicz wrote:
>>> --- a/xen/include/public/sysctl.h
>>> +++ b/xen/include/public/sysctl.h
>>> @@ -925,10 +925,11 @@ struct xen_sysctl_livepatch_get {
>>>  *
>>>
> snip
>>>     uint32_t pad;                           /* IN: Must be zero. */
>>> +    uint32_t name_total_size;               /* OUT: Total size of all 
>>> transfer names */
>>>     XEN_GUEST_HANDLE_64(xen_livepatch_status_t) status;  /* OUT. Must have 
>>> enough
>>>                                                space allocate for nr of 
>>> them. */
>>>     XEN_GUEST_HANDLE_64(char) name;         /* OUT: Array of names. Each 
>>> member
>>> -                                               MUST 
>>> XEN_LIVEPATCH_NAME_SIZE in size.
>>> -                                               Must have nr of them. */
>>> +                                               may have an arbitrary 
>>> length up to
>>> +                                               XEN_LIVEPATCH_NAME_SIZE 
>>> bytes. Must have
>>> +                                               nr of them. */
>>>     XEN_GUEST_HANDLE_64(uint32) len;        /* OUT: Array of lengths of 
>>> name's.
>>>                                                Must have nr of them. */
>>> };
>>
>> Non-binary-compatible changes require an interface version bump.
> 
> The bump happens with this patch of the patchset:
> https://patchwork.kernel.org/patch/11165427/
> 
>> I wonder though why you don't use the "pad" field; in fact the
>> way you make the change you'd have to introduce a 2nd padding
>> field, to make padding explicit _and_ check it's zero on input
>> (for future extensibility _without_ having to bump the interface
>> version).
>>
> 
> I do not use the pad field because I introduce another field with the
> next patch of the patchset: https://patchwork.kernel.org/patch/11165433/
> Then I would have to re-add the pad field again I suppose.

Yes indeed; this may seem a little cumbersome, but you want your
series structured that a large time gap between any two parts of
which is not going to result in not well formed code.

> Also, I was (false?) impression that the pad field is dedicated to
> the future input parameters, so I should not touch it.

With its padding function it's IN only (and I believe is being
checked to have been set to zero by the caller). Once assigned
a new purpose, it can as well become OUT, provided the prior
meaning of zero in the field doesn't have a chance of confusing
callers.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel