[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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

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


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.