From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH] libxc: remove most of tools/libxc/xc_dom_compat_linux.c Date: Fri, 23 Oct 2015 11:53:49 +0200 Message-ID: <562A03AD.6000003@suse.com> References: <1444131327-22000-1-git-send-email-jgross@suse.com> <5613C400.4040209@citrix.com> <20151006125858.GB29124@zion.uk.xensource.com> <5613C773.5030901@citrix.com> <1444137461.5302.167.camel@citrix.com> <5624C7C3.2030007@suse.com> <22056.65314.301337.452773@mariner.uk.xensource.com> <1445528287.2374.15.camel@citrix.com> <5629DE9D.8000404@suse.com> <1445593341.2374.91.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1445593341.2374.91.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Ian Jackson Cc: Andrew Cooper , xen-devel@lists.xen.org, Wei Liu , stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org 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 >>> >>> 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