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

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

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.

> -    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.

The rest of this looks OK from a tools pov, AFAICT.

Thanks,
Ian.

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