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

[Jim Fehlig]

On 05/25/2016 08:37 AM, Michal Privoznik wrote:
> 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 at suse.com>
>>>> ---
>>>>  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.

Thanks. I've fixed the typos and this time ensured it compiles with or without
DEFAULT_LOADER_NVRAM defined.

But given the pending freeze, I think I'll be a bit conservative and wait to
push these until the other series has been reviewed. I'm hesitant to push this
work without the patches that need it. For completeness, I'll include these
patches in V2 of the "libxl: Add support for UEFI using OVMF" series.

Regards,
Jim