[Qemu-devel] [PATCH for-2.3 v2] raw-posix: Deprecate aio=threads fallback without O_DIRECT

[Qemu-devel] [PATCH for-2.3 v2] raw-posix: Deprecate aio=threads fallback without O_DIRECT

[Kevin Wolf]

Currently, if the user requests aio=native, but forgets to choose a
cache mode that sets O_DIRECT, that request is silently ignored and raw
falls back to aio=threads.

Deprecate that behaviour so we can make it an error in future qemu
versions.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
v2:
- Use error_printf() instead of fprintf(stderr, ...) so that the warning will
  appear on the HMP monitor instead of stderr for drive_add.

  Markus suggested error_report(), which would have the advantage of displaying
  the option string and position in the config file. However, the result was a
  line that was so full that it wasn't very prominent as a warning any more.
  Making the warning very visible was the reason for using fprintf in v1.

  After thinking more about it, I came to the conclusion that for a deprecation
  warning like this, being visible is more important than finding the bad
  option quickly. The hope is anyway that nobody runs a setup like this, and
  even then the warning will last only for one release before we'll get a real
  error.

 block/raw-posix.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 844ac21..24d8582 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -503,6 +503,14 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
         error_setg_errno(errp, -ret, "Could not set AIO state");
         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);
+    }
 #endif
 
     s->has_discard = true;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for-2.3 v2] raw-posix: Deprecate aio=threads fallback without O_DIRECT

[Markus Armbruster]

Kevin Wolf <kwolf@redhat.com> writes:

> Currently, if the user requests aio=native, but forgets to choose a
> cache mode that sets O_DIRECT, that request is silently ignored and raw
> falls back to aio=threads.
>
> Deprecate that behaviour so we can make it an error in future qemu
> versions.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> v2:
> - Use error_printf() instead of fprintf(stderr, ...) so that the warning will
>   appear on the HMP monitor instead of stderr for drive_add.
>
>   Markus suggested error_report(), which would have the advantage of displaying
>   the option string and position in the config file. However, the result was a
>   line that was so full that it wasn't very prominent as a warning any more.
>   Making the warning very visible was the reason for using fprintf in v1.
>
>   After thinking more about it, I came to the conclusion that for a deprecation
>   warning like this, being visible is more important than finding the bad
>   option quickly. The hope is anyway that nobody runs a setup like this, and
>   even then the warning will last only for one release before we'll get a real
>   error.

Judgement call, obviously yours to make.

Reviewed-by: Markus Armbruster <armbru@redhat.com>