Re: [PATCH 5/5] raisin: Add XEN_CONFIG_EXTRA to config file

[George Dunlap <george.dunlap@citrix.com>]

On 19/10/15 12:37, Stefano Stabellini wrote:
> On Mon, 19 Oct 2015, George Dunlap wrote:
>> On 16/10/15 14:53, Stefano Stabellini wrote:
>>> On Wed, 14 Oct 2015, George Dunlap wrote:
>>>> Allowing the user to enable or disable specific functionality, such as
>>>> stubdoms.
>>>>
>>>> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>>>
>>> I don't like this very much: if we want to disable stubdoms by default
>>> with all configs, then I would prefer to simply add --disable-stubdom to
>>> components/xen without introducing XEN_CONFIG_EXTRA.
>>>
>>> Otherwise if we want to give users the chance to add an extra config
>>> option, I would introduce a generic way to do that, so that people can
>>> use it with any components they want, QEMU, libvirt, etc.
>>
>> You did notice that this config option is disabled by default, right?  I
>> added the line commented-out as an example of the sort of thing you
>> might want to do.
>>
>> I don't think we want to disable stubdoms by default across the board.
>>
>> I wouldn't object to adding empty config variables to the other
>> components; I just don't have a good way to test them at the moment.
>  
> What if we just add an environmental variable named as
> COMPONENT_BUILD_CONFIGURE at the end of the configure line when
> appropriate?  Of course it would need to be documented in the README.

Isn't this exactly the same as my patch, but
1) With a different name, and
2) No examples in the config files?

I don't mind #1, but #2 seems like the most logical place to document it.

 -George

> 
> Something like:
> 
> 
> diff --git a/components/qemu b/components/qemu
> index dce4ce0..0e57bdb 100644
> --- a/components/qemu
> +++ b/components/qemu
> @@ -27,7 +27,7 @@ function qemu_build() {
>          --disable-docs \
>          --bindir=$PREFIX/lib/xen/bin \
>          --datadir=$PREFIX/share/qemu-xen \
> -        --disable-guest-agent
> +        --disable-guest-agent $QEMU_BUILD_CONFIGURE
>      $RAISIN_MAKE all
>      $RAISIN_MAKE install DESTDIR="$INST_DIR"
>      cd "$BASEDIR"
> diff --git a/components/xen b/components/xen
> index 6b700e5..01527ec 100644
> --- a/components/xen
> +++ b/components/xen
> @@ -39,7 +39,7 @@ function xen_build() {
>          ovmf_opt="--enable-ovmf --with-system-ovmf="$BASEDIR"/ovmf-dir/ovmf.bin"
>      fi
>      ./configure --prefix=$PREFIX --with-system-qemu=$PREFIX/lib/xen/bin/qemu-system-i386 \
> -        --disable-qemu-traditional --enable-rombios $seabios_opt $ovmf_opt
> +        --disable-qemu-traditional --enable-rombios $seabios_opt $ovmf_opt $XEN_BUILD_CONFIGURE
>      $RAISIN_MAKE
>      $RAISIN_MAKE install DESTDIR="$INST_DIR"
>      cd "$BASEDIR"
>