| Message ID | 20160622171545.5304-6-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 22.06.16 at 19:15, <anthony.perard@citrix.com> wrote: > --- 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; > +} With an earlier version of this series pulled into one of our branches, I've run into a problem with this: The paths you return here are the configured paths, and that's intended. Yet it breaks running the tools out of the build area (i.e. without any "make install"), which so far has been working fine (as apparently in all other relevant cases where paths are needed, relative ones are being used), and which I much prefer over the hassle of scattering around half a dozen of different Xen tools versions in custom directories under, say, /usr/local. I guess if I'm the only one using this, I'll have to find my own local solution for this, but of course I'd prefer for this currently working case not to get broken. Jan
On Fri, Jun 24, 2016 at 01:23:19AM -0600, Jan Beulich wrote: > >>> On 22.06.16 at 19:15, <anthony.perard@citrix.com> wrote: > > --- 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; > > +} > > With an earlier version of this series pulled into one of our branches, > I've run into a problem with this: The paths you return here are the > configured paths, and that's intended. Yet it breaks running the > tools out of the build area (i.e. without any "make install"), which so > far has been working fine (as apparently in all other relevant cases > where paths are needed, relative ones are being used), and which I'm not sure that true about the relative paths, I think most, if not all are full path and happen to match what is already install on the system. I've tried to run xl from the build dir (and also dist dir) and with a different --prefix, xl can not find qemu and if I run xl, this time configure with the same --prefix and I remove hvmloader from my system, xl can not find hvmloader. You could copy both firmware manually to be in the same directory as hvmloader. A better solution would be to use all the _override xl config, I've added `bios_path_override' so one can supply a different bios/firmware to libxl. To be honest, I don't know which relative path to use since it depend on where is xl executed from. And I don't think the path to the firmwares (like hvmloader) can be change at execution time unless change for each of them. Is that answer your question? > I much prefer over the hassle of scattering around half a dozen of > different Xen tools versions in custom directories under, say, > /usr/local. I guess if I'm the only one using this, I'll have to find my > own local solution for this, but of course I'd prefer for this currently > working case not to get broken. > > Jan >
On Wed, Jun 22, 2016 at 06:15:36PM +0100, Anthony PERARD wrote: > The path to the BIOS blob can be overriden by the xl's > bios_path_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> > > --- > Changes in V5: > - man page, use B<> to highlight config option in description. > - rename config option from `bios_override` to `bios_path_override` > - store libxl_read_file_contents() return value into r instead of e > (just renamed the variable) > - rename domain_build_info.u.hvm.bios_firmware to system_firmware > > Changes in V4: > - updating man page to have bios_override described. > - return ERROR_INVAL in libxl__load_hvm_firmware_module when the file is > empty. > > Changes 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 > --- > docs/man/xl.cfg.pod.5.in | 9 +++++++ > 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 ++++++--- > 7 files changed, 95 insertions(+), 3 deletions(-) > > diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in > index 3bb27d0..a685b83 100644 > --- a/docs/man/xl.cfg.pod.5.in > +++ b/docs/man/xl.cfg.pod.5.in > @@ -1212,6 +1212,15 @@ Requires device_model_version=qemu-xen. > > =back > > +=item B<bios_path_override="PATH"> > + > +Override the path to the blob to be used as BIOS. The blob provided here MUST > +be consistent with the B<bios=> which you have specified. You should not > +normally need to specify this option. > + > +This options does not have any effect if using B<bios="rombios"> or > +B<device_model_version="qemu-xen-traditional">. > + > =item B<pae=BOOLEAN> > > Hide or expose the IA32 Physical Address Extensions. These extensions > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > index 2c0f868..2b1f678 100644 > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -928,6 +928,14 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src); > #define LIBXL_HAVE_CHECKPOINTED_STREAM 1 > > /* > + * LIBXL_HAVE_BUILDINFO_HVM_SYSTEM_FIRMWARE > + * > + * libxl_domain_build_info has u.hvm.system_firmware field which can be use > + * to provide a different firmware blob (like SeaBIOS or OVMF). > + */ > +#define LIBXL_HAVE_BUILDINFO_HVM_SYSTEM_FIRMWARE > + > +/* > * ERROR_REMUS_XXX error code only exists from Xen 4.5, Xen 4.6 and it > * is changed to ERROR_CHECKPOINT_XXX in Xen 4.7 > */ > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index ec29060..c341a29 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -862,6 +862,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 r; > + > + LOG(DEBUG, "Loading %s: %s", what, filename); > + r = libxl_read_file_contents(CTX, filename, &data, &datalen); > + if (r) { > + /* > + * Print a message only on ENOENT, other errors are logged by the > + * function libxl_read_file_contents(). > + */ > + if (r == ENOENT) > + LOGEV(ERROR, r, "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_INVAL; > + } > + return 0; > +} > + Please use goto style error handling to be consistent with other code. Wei.
diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in index 3bb27d0..a685b83 100644 --- a/docs/man/xl.cfg.pod.5.in +++ b/docs/man/xl.cfg.pod.5.in @@ -1212,6 +1212,15 @@ Requires device_model_version=qemu-xen. =back +=item B<bios_path_override="PATH"> + +Override the path to the blob to be used as BIOS. The blob provided here MUST +be consistent with the B<bios=> which you have specified. You should not +normally need to specify this option. + +This options does not have any effect if using B<bios="rombios"> or +B<device_model_version="qemu-xen-traditional">. + =item B<pae=BOOLEAN> Hide or expose the IA32 Physical Address Extensions. These extensions diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 2c0f868..2b1f678 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -928,6 +928,14 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src); #define LIBXL_HAVE_CHECKPOINTED_STREAM 1 /* + * LIBXL_HAVE_BUILDINFO_HVM_SYSTEM_FIRMWARE + * + * libxl_domain_build_info has u.hvm.system_firmware field which can be use + * to provide a different firmware blob (like SeaBIOS or OVMF). + */ +#define LIBXL_HAVE_BUILDINFO_HVM_SYSTEM_FIRMWARE + +/* * ERROR_REMUS_XXX error code only exists from Xen 4.5, Xen 4.6 and it * is changed to ERROR_CHECKPOINT_XXX in Xen 4.7 */ diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index ec29060..c341a29 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -862,6 +862,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 r; + + LOG(DEBUG, "Loading %s: %s", what, filename); + r = libxl_read_file_contents(CTX, filename, &data, &datalen); + if (r) { + /* + * Print a message only on ENOENT, other errors are logged by the + * function libxl_read_file_contents(). + */ + if (r == ENOENT) + LOGEV(ERROR, r, "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_INVAL; + } + return 0; +} + static int libxl__domain_firmware(libxl__gc *gc, libxl_domain_build_info *info, struct xc_dom_image *dom) @@ -871,6 +903,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; @@ -914,6 +947,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.system_firmware) { + bios_filename = info->u.hvm.system_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->system_firmware_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 e7ab85d..e6a199b 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -2317,6 +2317,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 ef614be..98bfc3a 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -513,6 +513,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("timer_mode", libxl_timer_mode), ("nested_hvm", libxl_defbool), ("altp2m", libxl_defbool), + ("system_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 6459eec..18da7ab 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -1562,12 +1562,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_path_override", + &b_info->u.hvm.system_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.system_firmware) + fprintf(stderr, "WARNING: " + "bios_path_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);
The path to the BIOS blob can be overriden by the xl's bios_path_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> --- Changes in V5: - man page, use B<> to highlight config option in description. - rename config option from `bios_override` to `bios_path_override` - store libxl_read_file_contents() return value into r instead of e (just renamed the variable) - rename domain_build_info.u.hvm.bios_firmware to system_firmware Changes in V4: - updating man page to have bios_override described. - return ERROR_INVAL in libxl__load_hvm_firmware_module when the file is empty. Changes 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 --- docs/man/xl.cfg.pod.5.in | 9 +++++++ 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 ++++++--- 7 files changed, 95 insertions(+), 3 deletions(-)