Re: [PATCH for-4.10 1/3] libxc: panic when trying to create a PVH guest without kernel support

["Roger Pau Monné" <roger.pau@citrix.com>]

On Fri, Jan 19, 2018 at 06:11:00AM +0000, HW42 wrote:
> Roger Pau Monne:
> > Previously when trying to boot a PV capable but not PVH capable kernel
> > inside of a PVH container xc_dom_guest_type would succeed and return a
> > PV guest type, which would lead to failures later on in the build
> > process.
> > 
> > Instead provide a clear error message when trying to create a PVH
> > guest using a kernel that doesn't support PVH.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  tools/libxc/xc_dom_elfloader.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
> > index 62d421a5e3..568d7f370c 100644
> > --- a/tools/libxc/xc_dom_elfloader.c
> > +++ b/tools/libxc/xc_dom_elfloader.c
> > @@ -59,6 +59,13 @@ static char *xc_dom_guest_type(struct xc_dom_image *dom,
> >      if ( dom->container_type == XC_DOM_HVM_CONTAINER &&
> >           dom->parms.phys_entry != UNSET_ADDR32 )
> >          return "hvm-3.0-x86_32";
> > +    if ( dom->container_type == XC_DOM_HVM_CONTAINER )
> > +    {
> > +        xc_dom_panic(dom->xch, XC_INVALID_KERNEL,
> > +                     "%s: image not capable of booting inside a HVM container",
> > +                     __FUNCTION__);
> > +        return "xen-3.0-unknown";
> > +    }
> >  
> >      switch ( machine )
> >      {
> 
> With this xc_dom_parse_elf_kernel() still returns success and the domain
> build fails only later because of the "xen-3.0-unknown" type. Why not
> fail directly in xc_dom_parse_elf_kernel() like for elf files which miss
> the DomU feature?

This doesn't seem to be how xc_dom_guest_type works ATM. There's also
a case below that also returns "xen-3.0-unknown", so iff you really
want to change the first return of "xen-3.0-unknown" you should also
change the one below AFAICT.

> And while we are at it: libxl__build_dom() expects that
> xc_dom_parse_elf_kernel() sets an errno. Currently this can contain an
> unrelated error since it does not get set when returning -EINVAL.

Error handling in libxc is all fuzzy, as noted at the top of
xc_dom_parse_elf_kernel:

/*
 * This function sometimes returns -1 for error and sometimes
 * an errno value.  ?!?!
 */

The general consensus is that either you set errno = -EFOO and return
-1, or simnply return -EFOO.

I can review the patches, but you will have to formally post them.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel