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

On 25.05.2016 15:28, Jim Fehlig wrote: > 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? > Ah, haven't seen those while reviewing this one. ACK to this one then too. Now it makes perfect sense.