Re: [PATCH] libxc: remove most of tools/libxc/xc_dom_compat_linux.c

[Juergen Gross <jgross@suse.com>]

On 10/23/2015 11:42 AM, Ian Campbell wrote:
> On Fri, 2015-10-23 at 09:15 +0200, Juergen Gross wrote:
>> On 10/22/2015 05:38 PM, Ian Campbell wrote:
>>> On Thu, 2015-10-22 at 16:22 +0100, Ian Jackson wrote:
>>>> Juergen Gross writes ("Re: [Xen-devel] [PATCH] libxc: remove most of
>>>> tools/libxc/xc_dom_compat_linux.c"):
>>>>> On 10/06/2015 03:17 PM, Ian Campbell wrote:
>>>>>> xc_dom_linux_build is implemented in terms of the non-compat
>>>>>> xc_dom_*
>>>>>> functions, so it should be possible to do what you want with out
>>>>>> using the
>>>>>> compat wrapper.
>>>>>>
>>>>>> If there is some obscure reason this isn't the case then we
>>>>>> should
>>>>>> fix
>>>>>> that, not carry around the compat options for ever as a
>>>>>> workaround
>>>>>> (fixes
>>>>>> include but are not limited to promoting xc_dom_linux_build into
>>>>>> a
>>>>>> non
>>>>>> -compat helper).
>>>>
>>>> I agree with this approach.
>>>>
>>>>> Any further comments?
>>>>>
>>>>> Andrew, are you okay with Ian's statement?
>>>>>
>>>>> Ian, does this mean you are Ack-ing the patch?
>>>>
>>>> Accordingly, in the absence of renewed objections, or alternative
>>>> proposals, the original patch is:
>>>>
>>>> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
>>>
>>> There was a conflict with "libxc: unify xc_dom_p2m_{host/guest}", where
>>> xc_dom_p2m_host became xc_dom_p2m. I tried to resolve in what I thought
>>> was
>>> the obvious way, but then I got many instances of:
>>>
>>>       In file included from libxl.c:19:0:
>>>       libxl_internal.h:1612:43: error: 'struct xc_dom_image' declared
>>> inside parameter list [-Werror]
>>>                                           struct xc_dom_image *dom);
>>>                                                  ^
>>>       libxl_internal.h:1612:43: error: its scope is only this definition
>>> or declaration, which is probably not what you want [-Werror]
>>>
>>> Not sure if the original patch was wrong, has bit-rotted, or I messed
>>> up
>>> the conflict resolution. This happens on all arches.
>>>
>>> Actually, looking back at it, the added "struct xc_dom_image" in
>>> libxl_arch.h is surely wrong, the right answer would be to include
>>> xc_dom.h
>>> somewhere appropriate it might be tolerable to just leave it in
>>> xenguest.h.
>>>
>>> Juergen, please investigate the build failure, fix the above and
>>> resubmit.
>>
>> That was easy. Just removing the definition for libxl_arch.h, include
>> xc_dom.h from libxl_internal.h and modify xc_dom.h to tolerate including
>> it multiple times.
>>
>> I've stumbled over another issue:
>>
>> I don't know what I did wrong, but obviously the patch was built on top
>> of the libxc python wrappers removal patch. Without that there are still
>> some functions in use which I wanted to remove in xc_dom_compat_linux.c
>>
>> As there was no objection for the intention of removing most of the
>> wrappers I'll resend the xc_dom_compat_linux.c cleanup patch together
>> with the libxc python wrappers cleanup in a series.
>
> OK. Please put the xc_dom_compat_linux.c parts towards the head of the
> series, such that they don't get blocked by any subsequent kvetching about
> any specific Python removal. (Except you should remove the Python wrappers
> for anything in xc_dom_compat_linux.c in the same patch as the removal of
> the C version).

Just to get it right: You are suggesting I do two patches:

- Patch 1: cleanup of xc_dom_compat_linux.c + removal of all python
   wrappers affected by this cleanup (this would be xc.linux_build() and
   xc.getBitSize() ).

- Patch 2: removal of the rest of the python wrappers

Is this your preferred approach?


Juergen