[Qemu-devel] [PATCH] Add --disable-bridge-helper configure flag

[Qemu-devel] [PATCH] Add --disable-bridge-helper configure flag

[Fabien Chouteau]

Bridge helper uses ioctl's not available on old Linux versions, we add
this flag to disable the build.

Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
 Makefile  |    2 +-
 configure |    8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 34d6a9e..b46c6b0 100644
--- a/Makefile
+++ b/Makefile
@@ -37,7 +37,7 @@ $(call set-vpath, $(SRC_PATH))
 
 LIBS+=-lz $(LIBS_TOOLS)
 
-HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
+HELPERS-$(CONFIG_HELPERS) = qemu-bridge-helper$(EXESUF)
 
 ifdef BUILD_DOCS
 DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 QMP/qmp-commands.txt
diff --git a/configure b/configure
index 500fe24..5566752 100755
--- a/configure
+++ b/configure
@@ -195,6 +195,7 @@ zlib="yes"
 guest_agent="yes"
 libiscsi=""
 coroutine=""
+bridge_helper="yes"
 
 # parse CC options first
 for opt do
@@ -824,6 +825,8 @@ for opt do
   ;;
   --disable-guest-agent) guest_agent="no"
   ;;
+  --disable-bridge-helper) bridge_helper="no"
+  ;;
   *) echo "ERROR: unknown option $opt"; show_help="yes"
   ;;
   esac
@@ -1110,6 +1113,7 @@ echo "  --disable-guest-agent    disable building of the QEMU Guest Agent"
 echo "  --enable-guest-agent     enable building of the QEMU Guest Agent"
 echo "  --with-coroutine=BACKEND coroutine backend. Supported options:"
 echo "                           gthread, ucontext, sigaltstack, windows"
+echo "  --disable-bridge-helper  disable building of the qemu-bridge-helper"
 echo ""
 echo "NOTE: The object files are built at the place where configure is launched"
 exit 1
@@ -3896,6 +3900,10 @@ if test "$tcg_interpreter" = "yes" ; then
   echo "CONFIG_TCI_DIS=y"  >> $libdis_config_mak
 fi
 
+if test "$bridge_helper" = "yes" && test "$linux" = "yes" ; then
+    echo "CONFIG_HELPERS=y" >> $config_host_mak
+fi
+
 case "$ARCH" in
 alpha)
   # Ensure there's only a single GP
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH] Add --disable-bridge-helper configure flag

[Paolo Bonzini]

Il 10/07/2012 12:43, Fabien Chouteau ha scritto:
> Bridge helper uses ioctl's not available on old Linux versions, we add
> this flag to disable the build.

Which ioctls?  Please detect them, so that we can also work around them
perhaps.

Paolo

Re: [Qemu-devel] [PATCH] Add --disable-bridge-helper configure flag

[Fabien Chouteau]

On 07/10/2012 12:44 PM, Paolo Bonzini wrote:
> Il 10/07/2012 12:43, Fabien Chouteau ha scritto:
>> Bridge helper uses ioctl's not available on old Linux versions, we add
>> this flag to disable the build.
> 
> Which ioctls?  Please detect them, so that we can also work around them
> perhaps.
> 

There SIOCBRADDIF at least, maybe it's the only one. 

-- 
Fabien Chouteau

Re: [Qemu-devel] [PATCH] Add --disable-bridge-helper configure flag

[Paolo Bonzini]

Il 10/07/2012 13:02, Fabien Chouteau ha scritto:
>>> >> Bridge helper uses ioctl's not available on old Linux versions, we add
>>> >> this flag to disable the build.
>> > 
>> > Which ioctls?  Please detect them, so that we can also work around them
>> > perhaps.
>> > 
> There SIOCBRADDIF at least, maybe it's the only one. 

So indeed you could also use SIOCDEVPRIVATE / BRCTL_ADD_IF if you were
inclined to do so...

Paolo

Re: [Qemu-devel] [PATCH] Add --disable-bridge-helper configure flag

[Fabien Chouteau]

On 07/10/2012 01:06 PM, Paolo Bonzini wrote:
> Il 10/07/2012 13:02, Fabien Chouteau ha scritto:
>>>>>> Bridge helper uses ioctl's not available on old Linux versions, we add
>>>>>> this flag to disable the build.
>>>>
>>>> Which ioctls?  Please detect them, so that we can also work around them
>>>> perhaps.
>>>>
>> There SIOCBRADDIF at least, maybe it's the only one. 
> 
> So indeed you could also use SIOCDEVPRIVATE / BRCTL_ADD_IF if you were
> inclined to do so...
> 

Unfortunately I don't have time to go deep into the code. It's not a
feature I plan to use, so the quickest solution for me is to disable it.

Corey, I can see that you developed the bridge helper, can you please
take a look at this issue?

-- 
Fabien Chouteau

Re: [Qemu-devel] [PATCH] Add --disable-bridge-helper configure flag

[Corey Bryant]

On 07/10/2012 09:37 AM, Fabien Chouteau wrote:
> On 07/10/2012 01:06 PM, Paolo Bonzini wrote:
>> Il 10/07/2012 13:02, Fabien Chouteau ha scritto:
>>>>>>> Bridge helper uses ioctl's not available on old Linux versions, we add
>>>>>>> this flag to disable the build.
>>>>>
>>>>> Which ioctls?  Please detect them, so that we can also work around them
>>>>> perhaps.
>>>>>
>>> There SIOCBRADDIF at least, maybe it's the only one.
>>
>> So indeed you could also use SIOCDEVPRIVATE / BRCTL_ADD_IF if you were
>> inclined to do so...
>>
>
> Unfortunately I don't have time to go deep into the code. It's not a
> feature I plan to use, so the quickest solution for me is to disable it.
>
> Corey, I can see that you developed the bridge helper, can you please
> take a look at this issue?
>

Apologies, I was out yesterday.  I see your new patch series is 
progressing.  I'll take over if/when you want.

-- 
Regards,
Corey

Re: [Qemu-devel] [PATCH] Add --disable-bridge-helper configure flag

[Fabien Chouteau]

On 07/11/2012 04:22 PM, Corey Bryant wrote:
> 
> 
> On 07/10/2012 09:37 AM, Fabien Chouteau wrote:
>> On 07/10/2012 01:06 PM, Paolo Bonzini wrote:
>>> Il 10/07/2012 13:02, Fabien Chouteau ha scritto:
>>>>>>>> Bridge helper uses ioctl's not available on old Linux versions, we add
>>>>>>>> this flag to disable the build.
>>>>>>
>>>>>> Which ioctls?  Please detect them, so that we can also work around them
>>>>>> perhaps.
>>>>>>
>>>> There SIOCBRADDIF at least, maybe it's the only one.
>>>
>>> So indeed you could also use SIOCDEVPRIVATE / BRCTL_ADD_IF if you were
>>> inclined to do so...
>>>
>>
>> Unfortunately I don't have time to go deep into the code. It's not a
>> feature I plan to use, so the quickest solution for me is to disable it.
>>
>> Corey, I can see that you developed the bridge helper, can you please
>> take a look at this issue?
>>
> 
> Apologies, I was out yesterday.  I see your new patch series is progressing.  I'll take over if/when you want.
> 

Beside the --enable/--disable patch, there's another question for you.
Is it possible to implement qemu-bridge-adapter without SIOCBRADDIF?

Thanks,

-- 
Fabien Chouteau

Re: [Qemu-devel] [PATCH] Add --disable-bridge-helper configure flag

[Paolo Bonzini]

Il 11/07/2012 16:56, Fabien Chouteau ha scritto:
>>>> >>> So indeed you could also use SIOCDEVPRIVATE / BRCTL_ADD_IF if you were
>>>> >>> inclined to do so...
>>>> >>>
>>> >>
>>> >> Unfortunately I don't have time to go deep into the code. It's not a
>>> >> feature I plan to use, so the quickest solution for me is to disable it.
>>> >>
>>> >> Corey, I can see that you developed the bridge helper, can you please
>>> >> take a look at this issue?
>>> >>
>> > 
>> > Apologies, I was out yesterday.  I see your new patch series is progressing.  I'll take over if/when you want.
>> > 
> Beside the --enable/--disable patch, there's another question for you.
> Is it possible to implement qemu-bridge-adapter without SIOCBRADDIF?

Yes, with the (obsolete) SIOCDEVPRIVATE ioctl.  It has multiple
subcommands, one of which is BRCTL_ADD_IF.

Paolo

Re: [Qemu-devel] [PATCH] Add --disable-bridge-helper configure flag

[Corey Bryant]

On 07/11/2012 10:57 AM, Paolo Bonzini wrote:
> Il 11/07/2012 16:56, Fabien Chouteau ha scritto:
>>>>>>>> So indeed you could also use SIOCDEVPRIVATE / BRCTL_ADD_IF if you were
>>>>>>>> inclined to do so...
>>>>>>>>
>>>>>>
>>>>>> Unfortunately I don't have time to go deep into the code. It's not a
>>>>>> feature I plan to use, so the quickest solution for me is to disable it.
>>>>>>
>>>>>> Corey, I can see that you developed the bridge helper, can you please
>>>>>> take a look at this issue?
>>>>>>
>>>>
>>>> Apologies, I was out yesterday.  I see your new patch series is progressing.  I'll take over if/when you want.
>>>>
>> Beside the --enable/--disable patch, there's another question for you.
>> Is it possible to implement qemu-bridge-adapter without SIOCBRADDIF?
>
> Yes, with the (obsolete) SIOCDEVPRIVATE ioctl.  It has multiple
> subcommands, one of which is BRCTL_ADD_IF.
>
> Paolo
>

I'll test this out and provide a patch that uses BRCTL_ADD_IF if 
SIOCBRADDIF is undefined.

-- 
Regards,
Corey

Re: [Qemu-devel] [PATCH] Add --disable-bridge-helper configure flag

[Andreas Färber]

Am 10.07.2012 12:43, schrieb Fabien Chouteau:
> Bridge helper uses ioctl's not available on old Linux versions, we add
> this flag to disable the build.
> 
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
> ---
>  Makefile  |    2 +-
>  configure |    8 ++++++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 34d6a9e..b46c6b0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -37,7 +37,7 @@ $(call set-vpath, $(SRC_PATH))
>  
>  LIBS+=-lz $(LIBS_TOOLS)
>  
> -HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
> +HELPERS-$(CONFIG_HELPERS) = qemu-bridge-helper$(EXESUF)
>  
>  ifdef BUILD_DOCS
>  DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 QMP/qmp-commands.txt

Three quick comments:

We should also disable build of all helpers when compiling with --static.

I don't like the bridge helper being disabled based on CONFIG_HELPERS,
suggest specific CONFIG_BRIDGE_HELPER or so.

All --disable-* options should probably have an --enable-* counterpart.

Andreas

> diff --git a/configure b/configure
> index 500fe24..5566752 100755
> --- a/configure
> +++ b/configure
> @@ -195,6 +195,7 @@ zlib="yes"
>  guest_agent="yes"
>  libiscsi=""
>  coroutine=""
> +bridge_helper="yes"
>  
>  # parse CC options first
>  for opt do
> @@ -824,6 +825,8 @@ for opt do
>    ;;
>    --disable-guest-agent) guest_agent="no"
>    ;;
> +  --disable-bridge-helper) bridge_helper="no"
> +  ;;
>    *) echo "ERROR: unknown option $opt"; show_help="yes"
>    ;;
>    esac
> @@ -1110,6 +1113,7 @@ echo "  --disable-guest-agent    disable building of the QEMU Guest Agent"
>  echo "  --enable-guest-agent     enable building of the QEMU Guest Agent"
>  echo "  --with-coroutine=BACKEND coroutine backend. Supported options:"
>  echo "                           gthread, ucontext, sigaltstack, windows"
> +echo "  --disable-bridge-helper  disable building of the qemu-bridge-helper"
>  echo ""
>  echo "NOTE: The object files are built at the place where configure is launched"
>  exit 1
> @@ -3896,6 +3900,10 @@ if test "$tcg_interpreter" = "yes" ; then
>    echo "CONFIG_TCI_DIS=y"  >> $libdis_config_mak
>  fi
>  
> +if test "$bridge_helper" = "yes" && test "$linux" = "yes" ; then
> +    echo "CONFIG_HELPERS=y" >> $config_host_mak
> +fi
> +
>  case "$ARCH" in
>  alpha)
>    # Ensure there's only a single GP
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] Add --disable-bridge-helper configure flag

[Fabien Chouteau]

On 07/10/2012 07:29 PM, Andreas Färber wrote:
> Am 10.07.2012 12:43, schrieb Fabien Chouteau:
>> Bridge helper uses ioctl's not available on old Linux versions, we add
>> this flag to disable the build.
>>
>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>> ---
>>  Makefile  |    2 +-
>>  configure |    8 ++++++++
>>  2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 34d6a9e..b46c6b0 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -37,7 +37,7 @@ $(call set-vpath, $(SRC_PATH))
>>  
>>  LIBS+=-lz $(LIBS_TOOLS)
>>  
>> -HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
>> +HELPERS-$(CONFIG_HELPERS) = qemu-bridge-helper$(EXESUF)
>>  
>>  ifdef BUILD_DOCS
>>  DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 QMP/qmp-commands.txt
> 
> Three quick comments:
> 
> We should also disable build of all helpers when compiling with --static.
> 
> I don't like the bridge helper being disabled based on CONFIG_HELPERS,
> suggest specific CONFIG_BRIDGE_HELPER or so.
>
> All --disable-* options should probably have an --enable-* counterpart.

3 easy fixes, I'll send a new version.


>> diff --git a/configure b/configure
>> index 500fe24..5566752 100755
>> --- a/configure
>> +++ b/configure
>> @@ -195,6 +195,7 @@ zlib="yes"
>>  guest_agent="yes"
>>  libiscsi=""
>>  coroutine=""
>> +bridge_helper="yes"
>>  
>>  # parse CC options first
>>  for opt do
>> @@ -824,6 +825,8 @@ for opt do
>>    ;;
>>    --disable-guest-agent) guest_agent="no"
>>    ;;
>> +  --disable-bridge-helper) bridge_helper="no"
>> +  ;;
>>    *) echo "ERROR: unknown option $opt"; show_help="yes"
>>    ;;
>>    esac
>> @@ -1110,6 +1113,7 @@ echo "  --disable-guest-agent    disable building of the QEMU Guest Agent"
>>  echo "  --enable-guest-agent     enable building of the QEMU Guest Agent"
>>  echo "  --with-coroutine=BACKEND coroutine backend. Supported options:"
>>  echo "                           gthread, ucontext, sigaltstack, windows"
>> +echo "  --disable-bridge-helper  disable building of the qemu-bridge-helper"
>>  echo ""
>>  echo "NOTE: The object files are built at the place where configure is launched"
>>  exit 1
>> @@ -3896,6 +3900,10 @@ if test "$tcg_interpreter" = "yes" ; then
>>    echo "CONFIG_TCI_DIS=y"  >> $libdis_config_mak
>>  fi
>>  
>> +if test "$bridge_helper" = "yes" && test "$linux" = "yes" ; then
>> +    echo "CONFIG_HELPERS=y" >> $config_host_mak
>> +fi
>> +
>>  case "$ARCH" in
>>  alpha)
>>    # Ensure there's only a single GP
>>
> 
> 


-- 
Fabien Chouteau