Re: [Xen-devel] [PATCH v3 05/16] libxl: Load guest BIOS from file

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Tue, Mar 01, 2016 at 11:51:40AM +0000, Wei Liu wrote:
> On Thu, Feb 25, 2016 at 02:56:03PM +0000, Anthony PERARD wrote:
> > The path to the BIOS blob can be override by the xl's bios_override option,
> > or provided by u.hvm.bios_firmware in the domain_build_info struct by other
> > libxl user.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > 
> > ---
> > Change in V3:
> > - move seabios_path and ovmf_path to libxl_path.c (with renaming)
> > - fix some coding style
> > - warn for empty file
> > - remove rombios stuff (will still be built-in hvmloader)
> > - rename field bios_filename in domain_build_info to bios_firmware to
> >   follow naming of acpi and smbios.
> > - log an error after libxl_read_file_contents() only when it return ENOENT
> > - return an error on empty file.
> > - added #define LIBXL_HAVE_BUILDINFO_HVM_BIOS_FIRMWARE
> > ---
> >  tools/libxl/libxl.h          |  8 +++++++
> >  tools/libxl/libxl_dom.c      | 57 
> > ++++++++++++++++++++++++++++++++++++++++++++
> >  tools/libxl/libxl_internal.h |  2 ++
> >  tools/libxl/libxl_paths.c    | 10 ++++++++
> >  tools/libxl/libxl_types.idl  |  1 +
> >  tools/libxl/xl_cmdimpl.c     | 11 ++++++---
> 
> You also need to patch manpage for this new option.

Yes, I'll do that.

> How does this new option interacts with bios= option?

That would be the same interaction that there is between
device_model_version and device_model_override.

If someone provide bios_override without bios, the guest could fail to
boot.

> >  6 files changed, 86 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> > index fa87f53..d223c35 100644
> > --- a/tools/libxl/libxl.h
> > +++ b/tools/libxl/libxl.h
> > @@ -876,6 +876,14 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, 
> > libxl_mac *src);
> >   */
> >  #define LIBXL_HAVE_DEVICE_MODEL_VERSION_NONE 1
> >  
> > +/*
> > + * LIBXL_HAVE_BUILDINFO_HVM_BIOS_FIRMWARE
> > + *
> > + * libxl_domain_build_info has u.hvm.bios_firmware field which can be use
> > + * to provide a different bios blob (like SeaBIOS or OVMF).
> > + */
> > +#define LIBXL_HAVE_BUILDINFO_HVM_BIOS_FIRMWARE
> > +
> >  typedef char **libxl_string_list;
> >  void libxl_string_list_dispose(libxl_string_list *sl);
> >  int libxl_string_list_length(const libxl_string_list *sl);
> > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> > index 2269998..50abfbc 100644
> > --- a/tools/libxl/libxl_dom.c
> > +++ b/tools/libxl/libxl_dom.c
> > @@ -863,6 +863,38 @@ err:
> >      return ret;
> >  }
> >  
> > +static int libxl__load_hvm_firmware_module(libxl__gc *gc,
> > +                                           const char *filename,
> > +                                           const char *what,
> > +                                           struct xc_hvm_firmware_module 
> > *m)
> > +{
> > +    int datalen = 0;
> > +    void *data = NULL;
> > +    int e;
> > +
> > +    LOG(DEBUG, "Loading %s: %s", what, filename);
> > +    e = libxl_read_file_contents(CTX, filename, &data, &datalen);
> > +    if (e) {
> > +        /*
> > +         * Print a message only on ENOENT, other error are logged by the
> > +         * function libxl_read_file_contents().
> > +         */
> > +        if (e == ENOENT)
> > +            LOGEV(ERROR, e, "failed to read %s file", what);
> > +        return ERROR_FAIL;
> > +    }
> > +    libxl__ptr_add(gc, data);
> > +    if (datalen) {
> > +        /* Only accept non-empty files */
> > +        m->data = data;
> > +        m->length = datalen;
> > +    } else {
> > +        LOG(ERROR, "file %s for %s is empty", filename, what);
> > +        return ERROR_FAIL;
> 
> ERROR_INVAL is more appropriate.

OK.

-- 
Anthony PERARD

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