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

[Markus Armbruster]

Kevin Wolf <address@hidden> 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 <address@hidden>
> ---
>  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 <address@hidden>