[Qemu-devel] [PATCH] blockdev: warn about aio=native if libaio is unavailable

[Qemu-devel] [PATCH] blockdev: warn about aio=native if libaio is unavailable

[Stefan Hajnoczi]

QEMU silently ignores aio=native if libaio is unavailable.  It is
confusing when aio=native performance is identical to aio=threads
because the binary was accidentally built without libaio.

Use error_report() because failing would break backward compatibility.
There are probably users using aio=native who would be inconvenienced if
QEMU suddenly refused to start their guests.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 blockdev.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 62a4586..f30828a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -405,10 +405,14 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
         bdrv_flags |= BDRV_O_NO_FLUSH;
     }
 
-#ifdef CONFIG_LINUX_AIO
     if ((buf = qemu_opt_get(opts, "aio")) != NULL) {
         if (!strcmp(buf, "native")) {
+#ifdef CONFIG_LINUX_AIO
             bdrv_flags |= BDRV_O_NATIVE_AIO;
+#else
+            error_report("warning: aio=native support unavailable, "
+                         "using default instead");
+#endif
         } else if (!strcmp(buf, "threads")) {
             /* this is the default */
         } else {
@@ -416,7 +420,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
            goto early_err;
         }
     }
-#endif
 
     if ((buf = qemu_opt_get(opts, "format")) != NULL) {
         if (is_help_option(buf)) {
-- 
2.4.3

Re: [Qemu-devel] [PATCH] blockdev: warn about aio=native if libaio is unavailable

[Kevin Wolf]

Am 17.07.2015 um 11:59 hat Stefan Hajnoczi geschrieben:
> QEMU silently ignores aio=native if libaio is unavailable.  It is
> confusing when aio=native performance is identical to aio=threads
> because the binary was accidentally built without libaio.
> 
> Use error_report() because failing would break backward compatibility.
> There are probably users using aio=native who would be inconvenienced if
> QEMU suddenly refused to start their guests.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

I hope not too many people are using aio=native without having libaio
compiled in... Can we make it a message like for the case with
aio=native,cache.direct=off, i.e. a deprecation warning that allows us
to make this an error in a few releases?

>  blockdev.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 62a4586..f30828a 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -405,10 +405,14 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
>          bdrv_flags |= BDRV_O_NO_FLUSH;
>      }
>  
> -#ifdef CONFIG_LINUX_AIO
>      if ((buf = qemu_opt_get(opts, "aio")) != NULL) {
>          if (!strcmp(buf, "native")) {
> +#ifdef CONFIG_LINUX_AIO
>              bdrv_flags |= BDRV_O_NATIVE_AIO;
> +#else
> +            error_report("warning: aio=native support unavailable, "
> +                         "using default instead");
> +#endif
>          } else if (!strcmp(buf, "threads")) {
>              /* this is the default */
>          } else {
> @@ -416,7 +420,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
>             goto early_err;
>          }
>      }
> -#endif

*sigh* It's sad to see that we still have such code in the generic block
layer. It has no business there, the raw-posix driver should check
whether it can support Linux AIO or not.

Paolo: You added some code to raw-win32 that implements aio=native.
Did it ever work in qemu proper, or only in the tools? It doesn't seem
that this code would even have added the flag on Windows builds.

Kevin

Re: [Qemu-devel] [PATCH] blockdev: warn about aio=native if libaio is unavailable

[Stefan Hajnoczi]

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

On Fri, Jul 17, 2015 at 12:56:15PM +0200, Kevin Wolf wrote:
> Am 17.07.2015 um 11:59 hat Stefan Hajnoczi geschrieben:
> > QEMU silently ignores aio=native if libaio is unavailable.  It is
> > confusing when aio=native performance is identical to aio=threads
> > because the binary was accidentally built without libaio.
> > 
> > Use error_report() because failing would break backward compatibility.
> > There are probably users using aio=native who would be inconvenienced if
> > QEMU suddenly refused to start their guests.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> I hope not too many people are using aio=native without having libaio
> compiled in... Can we make it a message like for the case with
> aio=native,cache.direct=off, i.e. a deprecation warning that allows us
> to make this an error in a few releases?

Yes, I'll move the warning to raw-posix.c so all callers benefit from
it.

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