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

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

On Tue, Feb 28, 2017 at 05:44:51PM +0000, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH v2 1/3] x86: remove PVHv1 code"):
> > This removal applies to both the hypervisor and the toolstack side of PVHv1.
> > 
> > Note that on the toolstack side there's one hiccup: on xl the "pvh"
> > configuration option is translated to builder="hvm",
> > device_model_version="none".  This is done because otherwise xl would start
> > parsing PV like options, and filling the PV struct at 
> > libxl_domain_build_info
> > (which in turn pollutes the HVM one because it's a union).
> ...
> 
> Thanks.
> 
> I have no argument with the principle, obviously, but I think the
> libxl API needs a bit of adjustment.
> 
> > +=item B<pvh=BOOLEAN>
> > +
> > +Selects whether to run this PV guest in an HVM container. Default is 0.
> > +Note that this option is equivalent to setting builder="hvm" and
> > +device_model_version="none"
> 
> I think this note needs to be reworded or de-emphasised.
> 
> 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.

> > -    xlu_cfg_get_defbool(config, "pvh", &c_info->pvh, 0);
> > +    if (!xlu_cfg_get_defbool(config, "pvh", &c_info->pvh, 0)) {
> > +        /* NB: we need to set the type here, or else we will fall into
> > +         * the PV path, and the set of options will be completely wrong
> > +         * (even more because the PV and HVM options are inside an union).
> > +         */
> > +        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.

Roger.

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