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

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

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@xxxxxxxxxxxxx>
> > 
> > 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).

Ian.

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