Re: [PATCH v2] tools: allow user to specify a system seabios binary

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 04/09/13 14:34, Ian Campbell wrote:
> On Fri, 2013-08-30 at 12:22 +0200, Fabio Fantoni wrote:
>> If this option is given don't bother building seabios ourselves.
>> Likely to be handy for distros who have an existing seabios
>> package which they want to reuse.
>>
>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>> ---
>>  config/Tools.mk.in                |    2 ++
>>  tools/configure                   |   28 ++++++++++++++++++++++++++++
>>  tools/configure.ac                |   16 ++++++++++++++++
>>  tools/firmware/Makefile           |    4 +++-
>>  tools/firmware/hvmloader/Makefile |    6 +++++-
>>  5 files changed, 54 insertions(+), 2 deletions(-)
>>
>> diff --git a/config/Tools.mk.in b/config/Tools.mk.in
>> index e388e42..eeca36b 100644
>> --- a/config/Tools.mk.in
>> +++ b/config/Tools.mk.in
>> @@ -23,6 +23,7 @@ LD86                := @LD86@
>>  BCC                 := @BCC@
>>  IASL                := @IASL@
>>  FETCHER             := @FETCHER@
>> +SEABIOS_PATH        := @seabios_path@
>>  
>>  # Extra folder for libs/includes
>>  PREPEND_INCLUDES    := @PREPEND_INCLUDES@
>> @@ -52,6 +53,7 @@ CONFIG_ROMBIOS      := @rombios@
>>  CONFIG_SEABIOS      := @seabios@
>>  CONFIG_QEMU_TRAD    := @qemu_traditional@
>>  CONFIG_QEMU_XEN     := @qemu_xen@
>> +CONFIG_SEABIOS_COMP := @seabios_compile@
> I don't think you need both SEABIOS_PATH and CONFIG_SEABIOS_COMP. Simply
> make the compilation of SeaBIOS conditional on SEABIOS_PATH being empty.
>
>>  CONFIG_XEND         := @xend@
>>  CONFIG_BLKTAP1      := @blktap1@
>>  
>> diff --git a/tools/configure.ac b/tools/configure.ac
>> index 1f57681..67b21bb 100644
>> --- a/tools/configure.ac
>> +++ b/tools/configure.ac
>> @@ -97,6 +97,22 @@ AS_IF([test "x$qemu_xen" = "xn"], [
>>  ])
>>  AC_SUBST(qemu_xen)
>>  
>> +AC_ARG_WITH([system-seabios],
>> +    AS_HELP_STRING([--with-system-seabios@<:@=PATH@:>@],
>> +       [Use system supplied seabios PATH instead of building and installing
>> +        our own version]),[
>> +    case $withval in
>> +    no) seabios_compile=y ; seabios_path= ;;
>> +    *)   seabios_compile=n ; seabios_path=$withval ;;
> please line these up </nitpick>
>
>> diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile
>> index aff8e56..fe56036 100644
>> --- a/tools/firmware/Makefile
>> +++ b/tools/firmware/Makefile
>> @@ -7,7 +7,9 @@ INST_DIR := $(DESTDIR)$(XENFIRMWAREDIR)
>>  
>>  SUBDIRS-y :=
>>  SUBDIRS-$(CONFIG_OVMF) += ovmf
>> -SUBDIRS-$(CONFIG_SEABIOS) += seabios-dir
>> +ifeq ($(CONFIG_SEABIOS),y)
>> +SUBDIRS-$(CONFIG_SEABIOS_COMP) += seabios-dir
>> +endif
> Perhaps:
>
> ifeq ($(CONFIG_SEABIOS_PATH),)

ifneq ?

> SUBDIRS-$(CONFIG_SEABIOS) += seabios-dir
> endif
>
> TBH I'm not sure that being able to disable SeaBIOS at build time is
> even useful.

Speaking as a downstream distro which doesn't currently use SeaBIOS, the
ability to disable is very useful to prevent having the xen build system
download yet more code.

While it is valid to have some combination of components built without
using SeaBIOS, an option to disable is more useful than not, even if
everyone is expected to use the default.

~Andrew