Re: [libvirt] [PATCH 2/2] libxl: add default firmwares to driver config object

On 05/25/2016 03:32 AM, Michal Privoznik wrote: > On 18.05.2016 18:38, Jim Fehlig wrote: >> Prefer firmwares specified via --with-loader-nvram configure >> option. If none are specified, use the Xen-provided default >> firmwares found in LIBXL_FIRMWARE_DIR. >> >> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; >> --- >> src/libxl/libxl_conf.c | 21 +++++++++++++++++++++ >> src/libxl/libxl_conf.h | 4 ++++ >> 2 files changed, 25 insertions(+) >> >> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c >> index c399f5c..54bed6b 100644 >> --- a/src/libxl/libxl_conf.c >> +++ b/src/libxl/libxl_conf.c >> @@ -104,6 +104,7 @@ libxlDriverConfigDispose(void *obj) >> VIR_FREE(cfg->saveDir); >> VIR_FREE(cfg->autoDumpDir); >> VIR_FREE(cfg->lockManagerName); >> + virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares); >> } >> >> >> @@ -1774,6 +1775,26 @@ libxlDriverConfigNew(void) >> goto error; >> } >> >> +#ifdef DEFAULT_LOADER_NVRAM >> + if (virFirmwareParseList(DEFAULT_LOADER_NVRAM, >> + &cfg->firmwares, >> + &cfg->nfirmwares) < 0) >> + goto error; >> + >> +#else >> + if (VIR_ALLOC_N(cfg->firmwares, 2) < 0) >> + goto error; >> + cfg->nfirmwares = 2; >> + if (VIR_ALLOC(cfg->firmwares[0]) < 0 || VIR_ALLOC(cfg->firmwares[1]) < 0) >> + goto error; >> + >> + if (VIR_STRDUP(cfg->firwares[0]->name, >> + LIBXL_FIRMWARE_DIR "/hvmloader") < 0 || >> + VIR_STRDUP(cfg->firwares[1]->name, >> + LIBXL_FIRMWARE_DIR "/ovmf.bin") < 0) >> + goto error; > Yet again, copy & paste error :) > s/firwares/firmwares/g > >> +#endif >> + >> return cfg; >> >> error: >> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h >> index c5b9429..e55717a 100644 >> --- a/src/libxl/libxl_conf.h >> +++ b/src/libxl/libxl_conf.h >> @@ -39,6 +39,7 @@ >> # include "virchrdev.h" >> # include "virhostdev.h" >> # include "locking/lock_manager.h" >> +# include "virfirmware.h" >> >> # define LIBXL_DRIVER_NAME "xenlight" >> # define LIBXL_VNC_PORT_MIN 5900 >> @@ -107,6 +108,9 @@ struct _libxlDriverConfig { >> char *libDir; >> char *saveDir; >> char *autoDumpDir; >> + >> + virFirmwarePtr *firmwares; >> + size_t nfirmwares; >> }; >> >> >> > Well, there's technically nothing wrong with this patch, but perhaps we > want to report the information we parse here? I mean, so far this is > just a setter and no getter. Are those patches yet to come? Yes. I've rebased the below series on top of this one and well post a V2 later today. https://www.redhat.com/archives/libvir-list/2016-April/msg01358.html Is it fine to push this preparatory work as is (after fixing comments of course), or should I include all the patches together in a V2?