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

[Qemu-devel] [PATCH for-2.3] 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>
---
 block/raw-posix.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index f0b4488..929b6ae 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)) {
+        fprintf(stderr, "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] 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>
> ---
>  block/raw-posix.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index f0b4488..929b6ae 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)) {
> +        fprintf(stderr, "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;

error_report(), please, because:

* No warning in a monitor that isn't on the tty:

    $ socat UNIX:test-hmp STDIO
    QEMU 2.2.50 monitor - type 'help' for more information
    (qemu) drive_add "" if=none,aio=native,file=tmp.qcow2
    drive_add "" if=none,aio=native,file=tmp.qcow2
    OK

* The warning gives no clue on location.  Not nice when the culprit is
  actually deep in some -readconfig file.

A quick grep counts 42 instances of "warning:', 53 '"Warning:', and 20
'"WARNING:'.  We suck at consistency :)

For a COW image, I get the warning twice, and once for image, and once
for the backing image:

    $ qemu-system-x86_64 -nodefaults -drive if=none,aio=native,file=tmp.qcow2
    WARNING: aio=native was specified for 'tmp.qcow2', but it requires cache.direct=on, which was not specified. Falling back to aio=threads.
             This will become an error condition in future QEMU versions.
    WARNING: aio=native was specified for 'tmp.img', but it requires cache.direct=on, which was not specified. Falling back to aio=threads.
             This will become an error condition in future QEMU versions.

Not sure whether this can or should be avoided.

With error_report() instead of fprintf():

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

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

[Eric Blake]

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

On 03/17/2015 06:48 AM, Kevin Wolf wrote:
> 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>
> ---
>  block/raw-posix.c | 8 ++++++++
>  1 file changed, 8 insertions(+)

Silent behavior changes are always mean, so making this noisy for now
with the intent of an error in the future is good.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]