Re: [Xen-devel] [PATCH 1/5] xen: improve changes to xen_add_to_physmap

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.08.12 at 14:27, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
wrote:
> On Mon, 6 Aug 2012, Jan Beulich wrote:
>> >>> On 06.08.12 at 17:43, Stefano Stabellini 
>> >>> <stefano.stabellini@xxxxxxxxxxxxx>
>> wrote:
>> > On Mon, 6 Aug 2012, Jan Beulich wrote:
>> >> >>> On 06.08.12 at 16:12, Stefano Stabellini 
>> >> >>> <stefano.stabellini@xxxxxxxxxxxxx>
>> >> wrote:
>> >> > This is an incremental patch on top of
>> >> > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary
>> >> > compatibility, it is better to introduce foreign_domid as part of a
>> >> > union containing both size and foreign_domid.
>> >> > 
>> >> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
>> >> > ---
>> >> >  xen/include/public/memory.h |   11 +++++++----
>> >> >  1 files changed, 7 insertions(+), 4 deletions(-)
>> >> > 
>> >> > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
>> >> > index b2adfbe..b0af2fd 100644
>> >> > --- a/xen/include/public/memory.h
>> >> > +++ b/xen/include/public/memory.h
>> >> > @@ -208,8 +208,12 @@ struct xen_add_to_physmap {
>> >> >      /* Which domain to change the mapping for. */
>> >> >      domid_t domid;
>> >> >  
>> >> > -    /* Number of pages to go through for gmfn_range */
>> >> > -    uint16_t    size;
>> >> > +    union {
>> >> > +        /* Number of pages to go through for gmfn_range */
>> >> > +        uint16_t    size;
>> >> > +        /* IFF gmfn_foreign */
>> >> > +        domid_t foreign_domid;
>> >> > +    };
>> >> 
>> >> But you're clear that this isn't standard C, and hence can't go
>> >> in this way?
>> >> 
>> > 
>> > Why? It is c11 if I am not mistaken.
>> 
>> Yes. But the common baseline is C89.
> 
> Do we need to keep it C89?

Yes, I think so. That's what we indeed can (and already do) expect
of compilers used on these headers. Even C99 would be too much
already.

> If I am not mistaken anonymous unions have been in GCC for more than 10
> years now.

With a few quirks here and there, yes. But gcc is not the measure
here anyway, we need to settle on the lowest common standard
that's largely accepted/implemented industry wide.

> If we do want to keep it C89, considering that size was introduced only
> recently, do you think that it would be OK for me to change the
> interface and just add size to a union?
> Like this:
> 
> union {
>     /* Number of pages to go through for gmfn_range */
>     uint16_t    size;
>     /* IFF gmfn_foreign */
>     domid_t foreign_domid;
> } u;
> 
> Also CC'ing Jean.

If that's okay with Jean (and implying eventual other users of the
interface - I hope he would know), then yes; the addition isn't in
anything that we have released so far.

The only other reasonable alternative I see would be to bump
__XEN_LATEST_INTERFACE_VERSION__ again, and have this
coded as (beware, ugly!)

#if __XEN_LATEST_INTERFACE_VERSION__ > 0x040200
union {
#endif
    /* Number of pages to go through for gmfn_range */
    uint16_t    size;
#if __XEN_LATEST_INTERFACE_VERSION__ > 0x040200
    /* IFF gmfn_foreign */
    domid_t foreign_domid;
} u;
#endif

Jan


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