libxenguest and xenguest.h

libxenguest and xenguest.h

[Jürgen Groß]

Andy has reported a libxenguest related build failure of qemu when
building qemu outside the Xen build environment. Problem is xenguest.h
now including xenctrl_dom.h, which is including xen/libelf/libelf.h.

The underlying problem is that libxenguest is basically offering some
"official" functions via xenguest.h, while some other functions are
only Xen internally usable and are defined in xenctrl_dom.h.

This is a rather weird construction and I'm seeing the following
solutions:

1. Make xen/include/xen/libelf.h a public header (or split the parts
    needed by xenguest.h into a public header)

2. Reflect the two parts of libxenguest by carving out the xenctrl_dom.h
    defined parts into a new library not made public

3. Make the xenctrl_dom.h interfaces internal again by not adding it to
    the installed headers

While variant 3 seems to be the easiest one I'd prefer variant 1.
Variant 2 seems to add complexity without any real gain IMO.

Thoughts?


Juergen

Re: libxenguest and xenguest.h

[Jan Beulich]

On 15.09.2020 07:18, Jürgen Groß wrote:
> Andy has reported a libxenguest related build failure of qemu when
> building qemu outside the Xen build environment. Problem is xenguest.h
> now including xenctrl_dom.h, which is including xen/libelf/libelf.h.
> 
> The underlying problem is that libxenguest is basically offering some
> "official" functions via xenguest.h, while some other functions are
> only Xen internally usable and are defined in xenctrl_dom.h.
> 
> This is a rather weird construction and I'm seeing the following
> solutions:
> 
> 1. Make xen/include/xen/libelf.h a public header (or split the parts
>     needed by xenguest.h into a public header)

Besides being conceptually wrong imo, this could (afaict) cause name
space issues to consumers. This definitely gets a -1 from me, if not
a -2.

> 2. Reflect the two parts of libxenguest by carving out the xenctrl_dom.h
>     defined parts into a new library not made public
> 
> 3. Make the xenctrl_dom.h interfaces internal again by not adding it to
>     the installed headers

This option would seem to imply that qemu has no real need to include
this header, as otherwise how would this address the build issue?

Jan

Re: libxenguest and xenguest.h

[Jürgen Groß]

On 15.09.20 09:55, Jan Beulich wrote:
> On 15.09.2020 07:18, Jürgen Groß wrote:
>> Andy has reported a libxenguest related build failure of qemu when
>> building qemu outside the Xen build environment. Problem is xenguest.h
>> now including xenctrl_dom.h, which is including xen/libelf/libelf.h.
>>
>> The underlying problem is that libxenguest is basically offering some
>> "official" functions via xenguest.h, while some other functions are
>> only Xen internally usable and are defined in xenctrl_dom.h.
>>
>> This is a rather weird construction and I'm seeing the following
>> solutions:
>>
>> 1. Make xen/include/xen/libelf.h a public header (or split the parts
>>      needed by xenguest.h into a public header)
> 
> Besides being conceptually wrong imo, this could (afaict) cause name
> space issues to consumers. This definitely gets a -1 from me, if not
> a -2.
> 
>> 2. Reflect the two parts of libxenguest by carving out the xenctrl_dom.h
>>      defined parts into a new library not made public
>>
>> 3. Make the xenctrl_dom.h interfaces internal again by not adding it to
>>      the installed headers
> 
> This option would seem to imply that qemu has no real need to include
> this header, as otherwise how would this address the build issue?

In fact qemu doesn't need to include xenguest.h at all, but this was
just how the problem was discovered.

So before my patches xenctrl_dom.h (or xc_dom.h as it was named at that
time) was included only from Xen sources (libxenguest, libxl, pvgrub).
Basically there was a rather large part of libxenguest ot really usable
by anyone outside the Xen build system. External users could use only
the interfaces which are declared in xenguest.h.


Juergen

Re: libxenguest and xenguest.h

[Jan Beulich]

On 15.09.2020 10:06, Jürgen Groß wrote:
> On 15.09.20 09:55, Jan Beulich wrote:
>> On 15.09.2020 07:18, Jürgen Groß wrote:
>>> Andy has reported a libxenguest related build failure of qemu when
>>> building qemu outside the Xen build environment. Problem is xenguest.h
>>> now including xenctrl_dom.h, which is including xen/libelf/libelf.h.
>>>
>>> The underlying problem is that libxenguest is basically offering some
>>> "official" functions via xenguest.h, while some other functions are
>>> only Xen internally usable and are defined in xenctrl_dom.h.
>>>
>>> This is a rather weird construction and I'm seeing the following
>>> solutions:
>>>
>>> 1. Make xen/include/xen/libelf.h a public header (or split the parts
>>>      needed by xenguest.h into a public header)
>>
>> Besides being conceptually wrong imo, this could (afaict) cause name
>> space issues to consumers. This definitely gets a -1 from me, if not
>> a -2.
>>
>>> 2. Reflect the two parts of libxenguest by carving out the xenctrl_dom.h
>>>      defined parts into a new library not made public
>>>
>>> 3. Make the xenctrl_dom.h interfaces internal again by not adding it to
>>>      the installed headers
>>
>> This option would seem to imply that qemu has no real need to include
>> this header, as otherwise how would this address the build issue?
> 
> In fact qemu doesn't need to include xenguest.h at all, but this was
> just how the problem was discovered.
> 
> So before my patches xenctrl_dom.h (or xc_dom.h as it was named at that
> time) was included only from Xen sources (libxenguest, libxl, pvgrub).
> Basically there was a rather large part of libxenguest ot really usable
> by anyone outside the Xen build system. External users could use only
> the interfaces which are declared in xenguest.h.

But then - what did change?

In any event, to me it looks like headers shouldn't have mixed purpose.
Either they're internal (and don't get installed), or they're meant for
public consumption (and then everything that's in there).

Jan

Re: libxenguest and xenguest.h

[Jürgen Groß]

On 15.09.20 10:11, Jan Beulich wrote:
> On 15.09.2020 10:06, Jürgen Groß wrote:
>> On 15.09.20 09:55, Jan Beulich wrote:
>>> On 15.09.2020 07:18, Jürgen Groß wrote:
>>>> Andy has reported a libxenguest related build failure of qemu when
>>>> building qemu outside the Xen build environment. Problem is xenguest.h
>>>> now including xenctrl_dom.h, which is including xen/libelf/libelf.h.
>>>>
>>>> The underlying problem is that libxenguest is basically offering some
>>>> "official" functions via xenguest.h, while some other functions are
>>>> only Xen internally usable and are defined in xenctrl_dom.h.
>>>>
>>>> This is a rather weird construction and I'm seeing the following
>>>> solutions:
>>>>
>>>> 1. Make xen/include/xen/libelf.h a public header (or split the parts
>>>>       needed by xenguest.h into a public header)
>>>
>>> Besides being conceptually wrong imo, this could (afaict) cause name
>>> space issues to consumers. This definitely gets a -1 from me, if not
>>> a -2.
>>>
>>>> 2. Reflect the two parts of libxenguest by carving out the xenctrl_dom.h
>>>>       defined parts into a new library not made public
>>>>
>>>> 3. Make the xenctrl_dom.h interfaces internal again by not adding it to
>>>>       the installed headers
>>>
>>> This option would seem to imply that qemu has no real need to include
>>> this header, as otherwise how would this address the build issue?
>>
>> In fact qemu doesn't need to include xenguest.h at all, but this was
>> just how the problem was discovered.
>>
>> So before my patches xenctrl_dom.h (or xc_dom.h as it was named at that
>> time) was included only from Xen sources (libxenguest, libxl, pvgrub).
>> Basically there was a rather large part of libxenguest ot really usable
>> by anyone outside the Xen build system. External users could use only
>> the interfaces which are declared in xenguest.h.
> 
> But then - what did change?

I included xenctrl_dom.h from xenguest.h as it contains interfaces of
libxenguest.

> In any event, to me it looks like headers shouldn't have mixed purpose.
> Either they're internal (and don't get installed), or they're meant for
> public consumption (and then everything that's in there).

Right.

And my thinking for libxenguest was that all interfaces it offers should
be in a public header.


Juergen