From mboxrd@z Thu Jan 1 00:00:00 1970 From: Attilio Rao Subject: Re: [PATCH 2 of 2] Add the ability to specify the option "bios_override" in the guest Date: Thu, 23 Feb 2012 10:31:26 +0000 Message-ID: <4F46157E.5000705@citrix.com> References: <520ba1527b7ad2c73c41.1329938234@dhcp-3-145.uk.xensource.com> <1329992002.8557.58.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1329992002.8557.58.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: "xen-devel@lists.xensource.com" , "gbtju85@gmail.com" List-Id: xen-devel@lists.xenproject.org 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