Re: [Xen-devel] [PATCH v2 1/3] x86: remove PVHv1 code

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Wed, Mar 01, 2017 at 02:10:53PM +0000, Roger Pau Monne wrote:
> On Wed, Mar 01, 2017 at 01:53:29PM +0000, Ian Jackson wrote:
> > Roger Pau Monne writes ("Re: [PATCH v2 1/3] x86: remove PVHv1 code"):
> > > On Tue, Feb 28, 2017 at 05:44:51PM +0000, Ian Jackson wrote:
> > > > Are those explicit settings even a supported way to create a PVHv2
> > > > domain ?  I think they probably shouldn't be.
> > > 
> > > Those are the only ways to create a PVHv2 domain ATM (without this
> > > patch, obviously). I don't mind making pvh the canonical way to
> > > create a PVHv2 guest.
> > 
> > Well, PVHv2 is in the process of becoming properly supported, so now
> > is the time to decide the "official" way.
> 
> I prefer builder="hvm" device_model_version="none" because I think it's 
> clearer
> from a user PoV that a HVM guest it being created. OTOH, using pvh=1 it's more
> obscure, and it isn't clear which kind of guest you are creating, and which
> options apply to it. Although all that can be fixed in the man page, I think
> it's less intuitive.
> 
> TBH, I'm not even sure we should keep the "pvh" option, the same kernel that
> previously worked with pvh=1 might not work anymore when this patch is 
> applied.
> 
> > > > > +        c_info->type = LIBXL_DOMAIN_TYPE_HVM;
> > > > > +        b_info->device_model_version = 
> > > > > LIBXL_DEVICE_MODEL_VERSION_NONE;
> > ...
> > > > I think this should probably be done in libxl, not xl.
> > > 
> > > That was my first attempt, but it's not trivial. PVHv1 assumes that
> > > the domain type is PV, so it will basically fill the PV side of the
> > > libxl_domain_build_info union, and that's bad. Because options like
> > > "hap" or "nestedhvm" are not even considered valid for PV guests,
> > > and there's no way for libxl to re-parse the configuration, so
> > > AFAICT the only proper way to solve this is to set the domain type
> > > correctly as soon as the "pvh" option is detected.
> > 
> > When you say "it will basically fill the PV side", what is "it" ?
> > Do you mean xl_parse.c ?
> 
> Yes, parse_config_data. With the current code in parse_config_data domain type
> (c_info->type) is set to PV when pvh=1 is set in the config file. Then in the
> same function, further below, options like nestedhvm are simply ignored.
> 
> I don't any other way to solve this rather than forcing domain type to HVM in
> parse_config_data when pvh=1 is set.
> 
> > Isn't this what libxl_domain_build_info_init_type is for ?
> 
> libxl_domain_build_info_init_type is not be able to re-parse the config file.

Nevermind, I see what you mean. libxl_domain_build_info_init_type should set
domain type to HVM and device model to none if pvh is set. I guess that could
work, but see my comment above about pvh=1 and compatibility.

Roger.

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