Re: [Xen-devel] [PATCH] tools/libxl: Reposition build_pre() logic between architectures

From: Wei Liu <wl@xen.org>
To: Jan Beulich <jbeulich@suse.com>
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>, "Wei Liu" <wl@xen.org>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	Xen-devel <xen-devel@lists.xenproject.org>,
	"Anthony Perard" <anthony.perard@citrix.com>,
	"Ian Jackson" <Ian.Jackson@citrix.com>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH] tools/libxl: Reposition build_pre() logic between architectures
Date: Mon, 30 Dec 2019 12:56:47 +0000	[thread overview]
Message-ID: <20191230125647.rthgr5llgy5hdybs@debian> (raw)
In-Reply-To: <5660e951-4fa3-a37d-31e9-30e9da97486e@suse.com>

On Fri, Dec 27, 2019 at 02:50:58PM +0100, Jan Beulich wrote:
> On 27.12.2019 14:45, Andrew Cooper wrote:
> > The call to xc_domain_disable_migrate() is made only from x86, while its
> > handling in Xen is common.  Move it to the libxl__build_pre().
> > 
> > hvm_set_conf_params(), hvm_set_viridian_features(),
> > hvm_set_mca_capabilities(), and the altp2m logic is all in common
> > code (parts ifdef'd) but despite this, is all actually x86 specific.
> > 
> > Move it into x86 specific code, and fold all of the xc_hvm_param_set() calls
> > together into hvm_set_conf_params() in a far more coherent way.
> > 
> > Finally - ensure that all hypercalls have their return values checked.
> > 
> > No practical change in constructed domains.  Fewer useless hypercalls now to
> > construct an ARM guest.
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > ---
> > CC: Ian Jackson <Ian.Jackson@citrix.com>
> > CC: Wei Liu <wl@xen.org>
> > CC: Anthony Perard <anthony.perard@citrix.com>
> > CC: Jan Beulich <JBeulich@suse.com>
> > CC: Wei Liu <wl@xen.org>
> > CC: Roger Pau Monné <roger.pau@citrix.com>
> > CC: Stefano Stabellini <sstabellini@kernel.org>
> > CC: Julien Grall <julien@xen.org>
> > CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> > ---
> >  tools/libxl/libxl_dom.c | 183 ++----------------------------------------------
> >  tools/libxl/libxl_x86.c | 181 ++++++++++++++++++++++++++++++++++++++++++++++-
> 
> I'll defer to the tool stack maintainers here. Imo this file would
> better be split - one to contain stuff better falling under x86
> maintainership, and the other for everything falling in the tool
> stack realm.

(Assuming you're talking about libxl_x86.c)

That works for me.

It is unclear to me how clean that split can be though...

Wei.

> 
> Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel