Re: [Xen-devel] [PATCH V2] common/mem_access: merged mem_access setting interfaces

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 03/20/2017 06:40 PM, Jan Beulich wrote:
>>>> On 20.03.17 at 17:16, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>> On 03/20/2017 06:14 PM, Razvan Cojocaru wrote:
>>> On 03/20/2017 06:07 PM, Jan Beulich wrote:
>>>>>>> On 20.03.17 at 10:50, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>>>> --- a/xen/include/public/memory.h
>>>>> +++ b/xen/include/public/memory.h
>>>>> @@ -444,6 +444,8 @@ struct xen_mem_access_op {
>>>>>      /* xenmem_access_t */
>>>>>      uint8_t access;
>>>>>      domid_t domid;
>>>>> +    uint16_t view_id;
>>>>> +    uint16_t pad[3];
>>>>
>>>> Irrespective of Andrew's valid general objection, the change above
>>>> wouldn't be valid either: How would you guarantee compatibility
>>>> with old callers? Other than in e.g. domctl/sysctl there's no
>>>> interface version here which can be bumped, so simply adding
>>>> fields to a structure and re-using an existing sub-op won't do.
>>>
>>> I wouldn't - I thought simply bumping the DOMCTL version macro would be
>>> enough, but obviously I could just add other DOMCTLs and return an error
>>> for the old ones.
> 
> I miss the connection to domctl here - this is a mem-op, isn't it?

Yes, sorry, I meant mem-ops.

>>> In any case, back when I've added xc_set_mem_access_multi() I've also
>>> modified struct xen_mem_access_op in the same manner:
>>>
>>>
>> http://xenbits.xenproject.org/gitweb/?p=xen.git;a=commitdiff;h=1ef5056bd6274e
>>  
>> cbe065387b6cf45657d6d700cd
>>
>> Oh, nevermind, I think you're referring to the fact that I had back then
>> added members to the end of the structure, and so the old layout had
>> remained compatible. Point taken.
> 
> Not just that - there you've also introduced a new sub-op.

Yes, but by modifying the structure for use with
XENMEM_access_op_set_access_multi, it's now also changed for, e.g.,
XENMEM_access_op_set_access - since it's common to them. Other than the
place where the new data has been added, I believe that the same
critique would apply to the old patch, unless I'm misunderstanding
something.


Thanks,
Razvan


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