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

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

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.

Roger.

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