Re: [PATCH 2 of 2] Add the ability to specify the option "bios_override" in the guest

[Attilio Rao <attilio.rao@citrix.com>]

On 23/02/12 10:13, Ian Campbell wrote:
> On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
>    
>> configuration file.
>> An example is given by:
>> bios_override="ovmf-x64"
>>      
> I'm not sure I would use the override suffix here -- that is generally
> for settings where we recommend that the user accepts the default. In
> this case though a user has perfectly valid reasons for selecting
> ovmf-{32,64} or seabios etc.
>
> Also please update docs/man/xl.cfg.pod.5 to document the new setting.
>    

It makes sense, thanks.

>> diff -r 032fea10f8d1 -r 520ba1527b7a tools/libxl/libxl_dm.c
>> --- a/tools/libxl/libxl_dm.c	Wed Feb 22 18:54:03 2012 +0000
>> +++ b/tools/libxl/libxl_dm.c	Wed Feb 22 18:56:22 2012 +0000
>> @@ -66,6 +66,8 @@ const char *libxl__domain_device_model(l
>>   static const char *libxl__domain_bios(libxl__gc *gc,
>>                                   const libxl_domain_build_info *info)
>>   {
>> +    if (info->u.hvm.bios)
>> +       return info->u.hvm.bios;
>>       switch (info->device_model_version) {
>>       case 1: return "rombios";
>>       case 2: return "seabios";
>>      
> Should integrate this function into libxl__domain_build_info_setdefaults
> from my "libxl: improved handling for default values in API" series.
>
>    

I really wanted to do this type of integration in a separate step.

> Also it would be good to use that same infrastructure to enforce that
> qemu-xen-traditional can only use rombios and that seabios and ovmf-*
> are only valid with qemu-xen.
>
>    
>> diff -r 032fea10f8d1 -r 520ba1527b7a tools/libxl/libxl_types.idl
>> --- a/tools/libxl/libxl_types.idl	Wed Feb 22 18:54:03 2012 +0000
>> +++ b/tools/libxl/libxl_types.idl	Wed Feb 22 18:56:22 2012 +0000
>> @@ -228,6 +228,7 @@ libxl_domain_build_info = Struct("domain
>>
>>       ("u", KeyedUnion(None, libxl_domain_type, "type",
>>                   [("hvm", Struct(None, [("firmware", string),
>> +                                       ("bios", string),
>>      
> I think an Enumeration would be better here.
> libxl__domain_bios

I just found it quicker/cleaner to just store the string and pass it 
down, rather than translating to an Enumeration and then translate it 
back to a string into libxl__domain_bios(), don't you think?
However I'm open to do what you feel is better.

Thanks,
Attilio