From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: Regression building HVM domains following "x86: add bitmap of enabled emulated devices" Date: Thu, 12 Nov 2015 10:07:34 +0000 Message-ID: <564464E6.8050502@citrix.com> References: <56439355.6000609@citrix.com> <20151111205702.GP21495@char.us.oracle.com> <5643AE16.1060709@citrix.com> <1447320344.18450.32.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1447320344.18450.32.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 , Konrad Rzeszutek Wilk Cc: Juergen Gross , Wei Liu , Ian Jackson , "xen-devel@lists.xen.org" , Jan Beulich , David Scott , Roger Pau Monne List-Id: xen-devel@lists.xenproject.org On 12/11/15 09:25, Ian Campbell wrote: > On Wed, 2015-11-11 at 21:07 +0000, Andrew Cooper wrote: >> On 11/11/2015 20:57, Konrad Rzeszutek Wilk wrote: >>> On Wed, Nov 11, 2015 at 07:13:25PM +0000, Andrew Cooper wrote: >>>> Hello, >>>> >>>> Xapi uses the Ocaml stub_xc_domain_create() which uses >>>> xc_domain_create(). xc_domain_create() itself zeros the arch >>>> configuration but passes flags straight through. >>> Ooops. >>>> As a result of c/s 171946ab "x86: add bitmap of enabled emulated >>>> devices", xc_domain_create() can no longer be used to construct HVM >>>> domains, failing the hypervisor-side sanity check. >>>> >>>> Needless to say, this has put a dent in XenServer's automated >>>> testing. >>>> >>>> >>>> There are a couple of options, but neither of them are fantastic. >>>> >>>> 1) Have xc_domain_create() fill in XEN_X86_EMU_ALL based on >>>> XEN_DOMCTL_CDF_hvm_guest and XEN_DOMCTL_CDF_pvh_guest >>>> >>>> or >>>> >>>> 2) Mandate that all callers provide a valid arch configuration, >>>> essentially turning xc_domain_create() into xc_domain_create_config() >>>> >>>> >>>> Longterm, what is the plan wrt guest construction? With my x86 >>>> maintainership hat on, I don't want to keep XEN_DOMCTL_CDF_pvh_guest >>>> in >>>> the interface, so I do not like 1) as an option. >>>> >>>> `git grep` indicates that the 3 users of xc_domain_create() are the >>>> Ocaml/Python stubs and init-xenstore-domain.c which only constructs a >>>> PV >>>> guest (which bypasses the issue), whereas libxl uses >>>> xc_domain_create_config(). (For the python stubs, I expect this will >>>> hit Oracle who are still using Xend to my knowledge). >>> We are moving to 'xl'.. and there are no Xend bits anymore. >>>> Option 2) is a better alternative, but will have a knock-on effect >>>> for >>>> downstream consumers of the stubs. >>> But aren't xc_* calls not-release-stable? >> They are indeed not, which offers the option to change the API. >> >>> Here is a third idea:: >>> >>> Make 'xc_domain_create' call 'xc_domain_create_config'. The >>> xc_domain_create >>> would synthesis the flags and we would put an 'deprecated' flag on it >>> (whatever that means?) and remove 'xc_domain_create' in 4.7? >> This is option 1. xc_domain_create() already calls >> xc_domain_create_config() but with a zeroed arch configuration. >> >> The issue is that modifying xc_domain_create() will preclude their >> construction of DMLite domains. > I think that's ok, we would essentially be declaring that > xc_domain_create_config() is the formally supported interface which can do > everything while xc_domain_create() is essentially a compat shim which can > only do things up to a certain point. > > Users who want more would need to switch to the _config variant. > > I'm half considering suggesting removing xc_domain_create(), renaming > xc_domain_create_config() without the _config() and having it do some > compat thing if the passed config==NULL, such that existing callers of > xc_domain_create() just need to add NULL to retain their current behaviour. That sounds best, with one modification. In the case that NULL is passed, there should be some default filled in locally (as currently done for ARM). For x86, this should now be flags & hvm => X86_EMU_ALL, so they retain their ability to build HMV guests by passing a NULL config. ~Andrew