Patchwork [v2,2/8] configure: add CONFIG_VIRTIO_BLK_DATA_PLANE

login
register
mail settings
Submitter Stefan Hajnoczi
Date Nov. 20, 2012, 12:31 p.m.
Message ID <1353414712-27072-3-git-send-email-stefanha@redhat.com>
Download mbox | patch
Permalink /patch/200318/
State New
Headers show

Comments

Stefan Hajnoczi - Nov. 20, 2012, 12:31 p.m.
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(+)
Michael Roth - Nov. 21, 2012, 6:17 p.m.
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.
Stefan Hajnoczi - Nov. 21, 2012, 6:29 p.m.
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
Michael Roth - Nov. 21, 2012, 6:45 p.m.
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
>

Patch

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)