Re: [Qemu-devel] [PATCH v2 2/8] configure: add CONFIG_VIRTIO_BLK_DATA_PLANE

[mdroth]

On Wed, Nov 21, 2012 at 07:29:21PM +0100, Stefan Hajnoczi wrote:
> On Wed, Nov 21, 2012 at 7:17 PM, mdroth <address@hidden> 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
>