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

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

[Kevin Wolf]

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;
     }
 #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;
     }
 #endif /* !defined(CONFIG_LINUX_AIO) */
 
-- 
1.8.3.1

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

[Christian Borntraeger]

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>

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 691 bytes --]

On Tue, Dec 15, 2015 at 11:42:23AM +0100, Kevin Wolf wrote:
> 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(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]