[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC PATCH v2 05/16] libxl: Load guest BIOS from file
On Mon, 2015-10-26 at 16:03 +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_filename in the domain_build_info struct by > other > libxl user. > > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx> > --- > Âtools/libxl/libxl_dom.cÂÂÂÂÂ| 66 > +++++++++++++++++++++++++++++++++++++++++++++ > Âtools/libxl/libxl_types.idl |ÂÂ1 + > Âtools/libxl/xl_cmdimpl.cÂÂÂÂ| 11 +++++--- > Â3 files changed, 75 insertions(+), 3 deletions(-) > > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index b40d744..27a0021 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -858,6 +858,42 @@ err: > ÂÂÂÂÂreturn ret; > Â} >  > +static const char *seabios_path(libxl__gc *gc) > +{ > +ÂÂÂÂreturn SEABIOS_PATH; > +} > + > +static const char *ovmf_path(libxl__gc *gc) > +{ > +ÂÂÂÂreturn OVMF_PATH; > +} Can you put these in libxl_paths.c (for consistency) please. > + > +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) { > +ÂÂÂÂÂÂÂÂLOGEV(ERROR, e, "failed to read %s file %s", "e" here should be an errno val, not a libxl ERROR_*. So you need to print manually via the format string using either LOG or LOGE (if libxl_read_file_contents also sets errno, probably not). > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂwhat, > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂfilename); > +ÂÂÂÂÂÂÂÂreturn ERROR_FAIL; > +ÂÂÂÂ} > +ÂÂÂÂlibxl__ptr_add(gc, data); > +ÂÂÂÂif (datalen) { > +ÂÂÂÂÂÂÂÂ/* Only accept non-empty files */ Error or logging otherwise? Silently ignoring seems likely to surprise someone eventually. > +ÂÂÂÂÂÂÂÂm->data = data; > +ÂÂÂÂÂÂÂÂm->length = (uint32_t)datalen; I hope that cast isn't really necessary. > +ÂÂÂÂ} > +ÂÂÂÂreturn 0; > +} > + > Âstatic int libxl__domain_firmware(libxl__gc *gc, > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂlibxl_domain_build_info *info, > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂstruct xc_dom_image *dom) > @@ -910,6 +946,36 @@ static int libxl__domain_firmware(libxl__gc *gc, > ÂÂÂÂÂÂÂÂÂgoto out; > ÂÂÂÂÂ} >  > +ÂÂÂÂif (info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE) { > +ÂÂÂÂÂÂÂÂconst char *bios_filename; > +ÂÂÂÂÂÂÂÂ// Look for BIOS and load it Proper comment style please. > +ÂÂÂÂÂÂÂÂif (info->u.hvm.bios_filename) { > +ÂÂÂÂÂÂÂÂÂÂÂÂbios_filename = info->u.hvm.bios_filename; > +ÂÂÂÂÂÂÂÂ} else { > +ÂÂÂÂÂÂÂÂÂÂÂÂswitch (info->u.hvm.bios) > +ÂÂÂÂÂÂÂÂÂÂÂÂ{ > +ÂÂÂÂÂÂÂÂÂÂÂÂcase LIBXL_BIOS_TYPE_ROMBIOS: > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂbios_filename = libxl__abs_path(gc, "rombios.bin", > + > libxl__xenfirmwaredir_path()); The cover letter implied that rombios was still built in, this suggests not? > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index 082fed8..a3fbcab 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -468,6 +468,7 @@ libxl_domain_build_info = > Struct("domain_build_info",[ > ÂÂÂÂÂ("u", KeyedUnion(None, libxl_domain_type, "type", > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ[("hvm", Struct(None, [("firmware",ÂÂÂÂÂÂÂÂÂstring), > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ("bios",ÂÂÂÂÂÂÂÂÂÂÂÂÂlibxl_bios_type), > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ("bios_filename",ÂÂÂÂstring), Such new additions require a #define LIBXL_HAVE_* in libxl.h so applications know they can use it. > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 840d73f..27d7c25 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_filename, 0); > +ÂÂÂÂÂÂÂÂif (!xlu_cfg_get_string(config, "bios", &buf, 0)) { > +ÂÂÂÂÂÂÂÂÂÂÂÂif (libxl_bios_type_from_string(buf, &b_info->u.hvm.bios)) { Please update the xl.cfg man page to reflect this. > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂfprintf(stderr, "ERROR: invalid value \"%s\" for \"bios\"\n", > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂbuf); > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂexit (1); > -ÂÂÂÂÂÂÂÂ} > +ÂÂÂÂÂÂÂÂÂÂÂÂ} > +ÂÂÂÂÂÂÂÂ} else if (b_info->u.hvm.bios_filename) > +ÂÂÂÂÂÂÂÂÂÂÂÂfprintf(stderr, "WARNING: " > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ"bios_override given without specific bios name\n"); What I'm about to say is probably not a good idea, but: Given that we might keep rombios in-hvmloader (hence bios=rombios bios_override!=NULL could be considered an error) can we get away with saying that any use of bios_filename implies some sort of common non- specific BIOS version? Eyeballing the diff between tools/firmware/hvmloader/{seabios,ovmf}.c I think the answer is already no and in any case doing this may tie our hands in the future with some fancy new thing. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |