diff mbox

blockdev: warn about aio=native if libaio is unavailable

Message ID 1437127189-1137-1-git-send-email-stefanha@redhat.com
State New
Headers show

Commit Message

Stefan Hajnoczi July 17, 2015, 9:59 a.m. UTC
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(-)

Comments

Kevin Wolf July 17, 2015, 10:56 a.m. UTC | #1
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
Stefan Hajnoczi July 17, 2015, 1:57 p.m. UTC | #2
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.
diff mbox

Patch

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)) {