| Message ID | 1341917019-23224-1-git-send-email-chouteau@adacore.com |
|---|---|
| State | New |
| Headers | show |
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
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.
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
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?
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 >
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 >> > >
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.
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,
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
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.
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
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(-)