Re: [Qemu-devel] [PATCH] raw-posix: Make aio=native option binding

[Christian Borntraeger <borntraeger@de.ibm.com>]

On 12/15/2015 11:42 AM, Kevin Wolf wrote:


> Subject: [PATCH] raw-posix: Make aio=native option binding

bike-shedding, but is there a better wording for the subject line,
e.g. something like

raw-posix: error out on invalid configurations for aio=native

or something like that


> Traditionally, aio=native was treated as an advice that could simply be
> ignored if an error occurs while initialising Linux AIO or the feature
> wasn't compiled in. This behaviour was deprecated in commit 96518254
> (qemu 2.3; error during init) and commit 1501ecc1 (qemu 2.5; not
> compiled in).
> 
> This patch changes raw-posix to error out in these cases instead of
> printing a deprecation warning.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/raw-posix.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index d9162fd..cb26dcb 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -500,21 +500,17 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
>          goto fail;
>      }
>      if (!s->use_aio && (bdrv_flags & BDRV_O_NATIVE_AIO)) {
> -        error_printf("WARNING: aio=native was specified for '%s', but "
> -                     "it requires cache.direct=on, which was not "
> -                     "specified. Falling back to aio=threads.\n"
> -                     "         This will become an error condition in "
> -                     "future QEMU versions.\n",
> -                     bs->filename);
> +        error_setg(errp, "aio=native was specified, but it requires "
> +                         "cache.direct=on, which was not specified.");
> +        ret = -EINVAL;
> +        goto fail;

In the real life this is probably the right thing to do.
On the other hand its a Linux kernel implementation detail that aio does
only work with O_DIRECT. But anyway, if Linux aio gets fixed we can 
change QEMU again.


>      }
>  #else
>      if (bdrv_flags & BDRV_O_NATIVE_AIO) {
> -        error_printf("WARNING: aio=native was specified for '%s', but "
> -                     "is not supported in this build. Falling back to "
> -                     "aio=threads.\n"
> -                     "         This will become an error condition in "
> -                     "future QEMU versions.\n",
> -                     bs->filename);
> +        error_setg(errp, "aio=native was specified, but is not supported "
> +                         "in this build.");
> +        ret = -EINVAL;
> +        goto fail;


We certainly want to error out in this case.

Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>