diff mbox

[v3,05/16] libxl: Load guest BIOS from file

Message ID 1456412174-20162-6-git-send-email-anthony.perard@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Anthony PERARD Feb. 25, 2016, 2:56 p.m. UTC
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@citrix.com>

---
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 ++++++---
 6 files changed, 86 insertions(+), 3 deletions(-)

Comments

Wei Liu March 1, 2016, 11:51 a.m. UTC | #1
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@citrix.com>
> 
> ---
> 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.

How does this new option interacts with bios= option?

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


Wei.
Anthony PERARD March 3, 2016, 5:16 p.m. UTC | #2
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@citrix.com>
> > 
> > ---
> > 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.
diff mbox

Patch

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;
+    }
+    return 0;
+}
+
 static int libxl__domain_firmware(libxl__gc *gc,
                                   libxl_domain_build_info *info,
                                   struct xc_dom_image *dom)
@@ -872,6 +904,7 @@  static int libxl__domain_firmware(libxl__gc *gc,
     int e, rc;
     int datalen = 0;
     void *data;
+    const char *bios_filename = NULL;
 
     if (info->u.hvm.firmware)
         firmware = info->u.hvm.firmware;
@@ -915,6 +948,30 @@  static int libxl__domain_firmware(libxl__gc *gc,
         goto out;
     }
 
+    if (info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
+        if (info->u.hvm.bios_firmware) {
+            bios_filename = info->u.hvm.bios_firmware;
+        } else {
+            switch (info->u.hvm.bios) {
+            case LIBXL_BIOS_TYPE_SEABIOS:
+                bios_filename = libxl__seabios_path();
+                break;
+            case LIBXL_BIOS_TYPE_OVMF:
+                bios_filename = libxl__ovmf_path();
+                break;
+            case LIBXL_BIOS_TYPE_ROMBIOS:
+            default:
+                abort();
+            }
+        }
+    }
+
+    if (bios_filename) {
+        rc = libxl__load_hvm_firmware_module(gc, bios_filename, "BIOS",
+                                             &dom->bios_module);
+        if (rc) goto out;
+    }
+
     if (info->u.hvm.smbios_firmware) {
         data = NULL;
         e = libxl_read_file_contents(ctx, info->u.hvm.smbios_firmware,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 650a958..0dbff27 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2265,6 +2265,8 @@  _hidden const char *libxl__xen_config_dir_path(void);
 _hidden const char *libxl__xen_script_dir_path(void);
 _hidden const char *libxl__lock_dir_path(void);
 _hidden const char *libxl__run_dir_path(void);
+_hidden const char *libxl__seabios_path(void);
+_hidden const char *libxl__ovmf_path(void);
 
 /*----- subprocess execution with timeout -----*/
 
diff --git a/tools/libxl/libxl_paths.c b/tools/libxl/libxl_paths.c
index 9b7b0d5..6972b90 100644
--- a/tools/libxl/libxl_paths.c
+++ b/tools/libxl/libxl_paths.c
@@ -35,6 +35,16 @@  const char *libxl__run_dir_path(void)
     return XEN_RUN_DIR;
 }
 
+const char *libxl__seabios_path(void)
+{
+    return SEABIOS_PATH;
+}
+
+const char *libxl__ovmf_path(void)
+{
+    return OVMF_PATH;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 9ad7eba..95bacd2 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -487,6 +487,7 @@  libxl_domain_build_info = Struct("domain_build_info",[
                                        ("timer_mode",       libxl_timer_mode),
                                        ("nested_hvm",       libxl_defbool),
                                        ("altp2m",           libxl_defbool),
+                                       ("bios_firmware",    string),
                                        ("smbios_firmware",  string),
                                        ("acpi_firmware",    string),
                                        ("hdtype",           libxl_hdtype),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index f40af51..201cff6 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1500,12 +1500,17 @@  static void parse_config_data(const char *config_source,
 
         xlu_cfg_replace_string (config, "firmware_override",
                                 &b_info->u.hvm.firmware, 0);
-        if (!xlu_cfg_get_string(config, "bios", &buf, 0) &&
-            libxl_bios_type_from_string(buf, &b_info->u.hvm.bios)) {
+        xlu_cfg_replace_string (config, "bios_override",
+                                &b_info->u.hvm.bios_firmware, 0);
+        if (!xlu_cfg_get_string(config, "bios", &buf, 0)) {
+            if (libxl_bios_type_from_string(buf, &b_info->u.hvm.bios)) {
                 fprintf(stderr, "ERROR: invalid value \"%s\" for \"bios\"\n",
                     buf);
                 exit (1);
-        }
+            }
+        } else if (b_info->u.hvm.bios_firmware)
+            fprintf(stderr, "WARNING: "
+                    "bios_override given without specific bios name\n");
 
         xlu_cfg_get_defbool(config, "pae", &b_info->u.hvm.pae, 0);
         xlu_cfg_get_defbool(config, "apic", &b_info->u.hvm.apic, 0);