Re: [PATCH for-4.15 v2] xen/dmop: Strip __XEN_TOOLS__ header guard from public API

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 11/03/2021 14:44, Jan Beulich wrote:
> On 11.03.2021 14:43, Andrew Cooper wrote:
>> On 11/03/2021 13:28, Jan Beulich wrote:
>>> On 11.03.2021 14:00, Andrew Cooper wrote:
>>>> However, having laid things out in this way today, it occurs to me that
>>>> we should consider further cleanup as well.
>>>>
>>>> I do agree that code wanting to use the libxendevicemodel.h API almost
>>>> certainly don't want/need the dmop ABI.  (i.e. an individual consumer
>>>> will want one, or the other, but almost certainly not both together).
>>>>
>>>> Should libxendevicemodel.h really be including dm_op.h?
>>> I was indeed wondering.
>>>
>>>>   AFAICT, it is
>>>> only the ioserverid_t typedef which is API shared between the two
>>>> contexts, and we can trivially typedef that locally.
>>> Hmm, a local typedef isn't nice - there should be one central point.
>>> Granted there's no risk for this to change in anywhere halfway
>>> foreseeable future, but still. Also neither C89 nor C99 allow a
>>> typedef to be repeated - in those versions we'd then rely on an
>>> extension.
>> I wonder if we're depending on that extension elsewhere.  As far as the
>> stable libraries go, we are dependent on a Linux or BSD environment
>> currently.
> Right, but we'd like the headers to be consumable by any environment.
>
>> Alternatively we can drop the typedef and use uint16_t instead without
>> breaking things in practice.  (As long as we make the change in 4.15 and
>> we lose the wiggle room afforded us by the entire interface being behind
>> __XEN_TOOLS__ previously).
>>
>> Thoughts?  I can't think of any ifdefary which would help, and swapping
>> to uint16_t would reduce the use of an improperly namespaced identifier.
> I'm not outright against, but this might inspire people to use
> uint16_t elsewhere too, when they should use the typedef. How
> about a transient #define (suitably commented)?

Having actually experimented, it is more complicated.


/local/xen.git/tools/libs/devicemodel/../../../tools/include/xendevicemodel.h:283:45:
error: array type has incomplete element type ‘struct
xen_dm_op_modified_memory_extent’
     struct xen_dm_op_modified_memory_extent extents[], uint32_t nr);

so the dm_op include is necessary to use the libxendevicemodel API.

Also hvmmem_type_t and evtchn_port_t get used.

~Andrew