| Message ID | 1353414712-27072-3-git-send-email-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Nov 20, 2012 at 01:31:46PM +0100, Stefan Hajnoczi wrote: > The virtio-blk-data-plane feature only works with Linux AIO. Therefore > add a ./configure option and necessary checks to implement this > dependency. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > configure | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/configure b/configure > index 780b19a..633ba6d 100755 > --- a/configure > +++ b/configure > @@ -223,6 +223,7 @@ libiscsi="" > coroutine="" > seccomp="" > glusterfs="" > +virtio_blk_data_plane="" > > # parse CC options first > for opt do > @@ -871,6 +872,10 @@ for opt do > ;; > --enable-glusterfs) glusterfs="yes" > ;; > + --disable-virtio-blk-data-plane) virtio_blk_data_plane="no" > + ;; > + --enable-virtio-blk-data-plane) virtio_blk_data_plane="yes" > + ;; > *) echo "ERROR: unknown option $opt"; show_help="yes" > ;; > esac > @@ -2233,6 +2238,17 @@ EOF > fi > > ########################################## > +# adjust virtio-blk-data-plane based on linux-aio > + > +if test "$virtio_blk_data_plane" = "yes" -a \ > + "$linux_aio" != "yes" ; then > + echo "Error: virtio-blk-data-plane requires Linux AIO, please try --enable-linux-aio" > + exit 1 > +elif test -z "$virtio_blk_data_plane" ; then > + virtio_blk_data_plane=$linux_aio > +fi $linux_aio gets set automatically if the user has libaio installed and doesn't specify --disable-linux-aio, so this ends up enabling dataplane by default in a lot of situations. Since it's experimental I think it should only be enabled if we pass --enable-virtio-blk-data-plane explicitly.
On Wed, Nov 21, 2012 at 7:17 PM, mdroth <mdroth@linux.vnet.ibm.com> wrote: > On Tue, Nov 20, 2012 at 01:31:46PM +0100, Stefan Hajnoczi wrote: >> @@ -2233,6 +2238,17 @@ EOF >> fi >> >> ########################################## >> +# adjust virtio-blk-data-plane based on linux-aio >> + >> +if test "$virtio_blk_data_plane" = "yes" -a \ >> + "$linux_aio" != "yes" ; then >> + echo "Error: virtio-blk-data-plane requires Linux AIO, please try --enable-linux-aio" >> + exit 1 >> +elif test -z "$virtio_blk_data_plane" ; then >> + virtio_blk_data_plane=$linux_aio >> +fi > > $linux_aio gets set automatically if the user has libaio installed and > doesn't specify --disable-linux-aio, so this ends up enabling dataplane by > default in a lot of situations. Since it's experimental I think it should only > be enabled if we pass --enable-virtio-blk-data-plane explicitly. I expect downstreams to enable this feature. Requiring package maintainers to add --enable-virtio-blk-data-one explicitly is probably going to cause more work than any benefits of disabling it by default. The feature has no effect unless -device virtio-blk-pci,x-data-plane-on is used. Code size is <12 KB on x86_64 and contains nothing especially risky from a security perspective. That said, if there is a strong feeling this should be disabled by default, I can switch it to default off. Stefan
On Wed, Nov 21, 2012 at 07:29:21PM +0100, Stefan Hajnoczi wrote: > On Wed, Nov 21, 2012 at 7:17 PM, mdroth <mdroth@linux.vnet.ibm.com> wrote: > > On Tue, Nov 20, 2012 at 01:31:46PM +0100, Stefan Hajnoczi wrote: > >> @@ -2233,6 +2238,17 @@ EOF > >> fi > >> > >> ########################################## > >> +# adjust virtio-blk-data-plane based on linux-aio > >> + > >> +if test "$virtio_blk_data_plane" = "yes" -a \ > >> + "$linux_aio" != "yes" ; then > >> + echo "Error: virtio-blk-data-plane requires Linux AIO, please try --enable-linux-aio" > >> + exit 1 > >> +elif test -z "$virtio_blk_data_plane" ; then > >> + virtio_blk_data_plane=$linux_aio > >> +fi > > > > $linux_aio gets set automatically if the user has libaio installed and > > doesn't specify --disable-linux-aio, so this ends up enabling dataplane by > > default in a lot of situations. Since it's experimental I think it should only > > be enabled if we pass --enable-virtio-blk-data-plane explicitly. > > I expect downstreams to enable this feature. Requiring package > maintainers to add --enable-virtio-blk-data-one explicitly is probably > going to cause more work than any benefits of disabling it by default. > > The feature has no effect unless -device > virtio-blk-pci,x-data-plane-on is used. Code size is <12 KB on x86_64 > and contains nothing especially risky from a security perspective. > > That said, if there is a strong feeling this should be disabled by > default, I can switch it to default off. No, sorry for the noise. I was playing around with it locally and noticed it enabled by default, but wasn't accounting for the fact that you still need to enable the x-data-plane option to use it. I don't think it hurts to compile in the support by default. > > Stefan >
diff --git a/configure b/configure index 780b19a..633ba6d 100755 --- a/configure +++ b/configure @@ -223,6 +223,7 @@ libiscsi="" coroutine="" seccomp="" glusterfs="" +virtio_blk_data_plane="" # parse CC options first for opt do @@ -871,6 +872,10 @@ for opt do ;; --enable-glusterfs) glusterfs="yes" ;; + --disable-virtio-blk-data-plane) virtio_blk_data_plane="no" + ;; + --enable-virtio-blk-data-plane) virtio_blk_data_plane="yes" + ;; *) echo "ERROR: unknown option $opt"; show_help="yes" ;; esac @@ -2233,6 +2238,17 @@ EOF fi ########################################## +# adjust virtio-blk-data-plane based on linux-aio + +if test "$virtio_blk_data_plane" = "yes" -a \ + "$linux_aio" != "yes" ; then + echo "Error: virtio-blk-data-plane requires Linux AIO, please try --enable-linux-aio" + exit 1 +elif test -z "$virtio_blk_data_plane" ; then + virtio_blk_data_plane=$linux_aio +fi + +########################################## # attr probe if test "$attr" != "no" ; then @@ -3235,6 +3251,7 @@ echo "build guest agent $guest_agent" echo "seccomp support $seccomp" echo "coroutine backend $coroutine_backend" echo "GlusterFS support $glusterfs" +echo "virtio-blk-data-plane $virtio_blk_data_plane" if test "$sdl_too_old" = "yes"; then echo "-> Your SDL version is too old - please upgrade to have SDL support" @@ -3581,6 +3598,10 @@ if test "$glusterfs" = "yes" ; then echo "CONFIG_GLUSTERFS=y" >> $config_host_mak fi +if test "$virtio_blk_data_plane" = "yes" ; then + echo "CONFIG_VIRTIO_BLK_DATA_PLANE=y" >> $config_host_mak +fi + # USB host support case "$usb" in linux)
The virtio-blk-data-plane feature only works with Linux AIO. Therefore add a ./configure option and necessary checks to implement this dependency. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- configure | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)