Re: [PATCH] libxl: check nesthvm and altp2m in libxl level

[Wei Liu <wei.liu2@citrix.com>]

On Mon, Jul 27, 2015 at 01:34:04PM +0100, Julien Grall wrote:
> Hi Wei,
> 
> On 24/07/15 16:39, Wei Liu wrote:
> > In ea214001 ("x86/altp2m: add altp2mhvm HVM domain parameter"), a
> > check was added to ensure nestedhvm and altp2m cannot be enabled at
> > the same time. That check was added in xl, but in fact it should be in
> > libxl because it should be the entity that decides whether
> > the provided configuration is valid.
> > 
> > This patch moves the check to libxl. The code snippet is moved after
> > calling libxl__domain_build_info_setdefault so that we can:
> > 1. remove libxl_defbool_is_default in `if()';
> > 2. detect mistake in libxl__domain_build_info_setdefault.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > Cc: Ed White <edmund.h.white@intel.com>
> > Cc: "Sahita, Ravi" <ravi.sahita@intel.com>
> > 
> > I said I discovered an issue during review but also volunteered to fix
> > it after the series is merged. Here is the patch to do that.
> > ---
> >  tools/libxl/libxl_create.c | 6 ++++++
> >  tools/libxl/xl_cmdimpl.c   | 8 --------
> >  2 files changed, 6 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> > index 855b42c..de536ba 100644
> > --- a/tools/libxl/libxl_create.c
> > +++ b/tools/libxl/libxl_create.c
> > @@ -883,6 +883,12 @@ static void initiate_domain_create(libxl__egc *egc,
> >          goto error_out;
> >      }
> >  
> > +    if (libxl_defbool_val(d_config->b_info.u.hvm.nested_hvm) &&
> > +        libxl_defbool_val(d_config->b_info.u.hvm.altp2m)) {
> > +        LOG(ERROR, "nestedhvm and altp2mhvm cannot be used together");
> > +        goto error_out;
> > +    }
> > +
> 
> The u.hvm.{nested_hvm,altp2m} can only be checked when the created
> domain is an HVM.
> 
> But initiate_domain_create is called with either a PV or HVM domain. So
> you may need to check if we are creating a HVM one.
> 

Good point. I will respin.

Wei.

> Regards,
> 
> -- 
> Julien Grall