[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 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.

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]