* [PATCH] Only advertise aio=io_uring if support is actually available @ 2022-04-19 17:19 Dirk Müller 2022-04-19 19:23 ` Eric Blake 2022-04-20 8:33 ` Daniel P. Berrangé 0 siblings, 2 replies; 5+ messages in thread From: Dirk Müller @ 2022-04-19 17:19 UTC (permalink / raw) To: qemu-block; +Cc: eblake, qemu-devel, Dirk Müller This allows $qemu --help runtime configure checks for detecting the host support. Signed-off-by: Dirk Müller <dmueller@suse.de> --- block/file-posix.c | 4 ++++ qemu-nbd.c | 4 ++++ qemu-options.hx | 6 +++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/block/file-posix.c b/block/file-posix.c index 39a3d6dbe6..aec4763862 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -544,7 +544,11 @@ static QemuOptsList raw_runtime_opts = { { .name = "aio", .type = QEMU_OPT_STRING, +#ifdef CONFIG_LINUX_IO_URING .help = "host AIO implementation (threads, native, io_uring)", +#else + .help = "host AIO implementation (threads, native)", +#endif }, { .name = "aio-max-batch", diff --git a/qemu-nbd.c b/qemu-nbd.c index 713e7557a9..4634a0fc42 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -147,7 +147,11 @@ static void usage(const char *name) " --cache=MODE set cache mode used to access the disk image, the\n" " valid options are: 'none', 'writeback' (default),\n" " 'writethrough', 'directsync' and 'unsafe'\n" +#ifdef CONFIG_LINUX_IO_URING " --aio=MODE set AIO mode (native, io_uring or threads)\n" +#else +" --aio=MODE set AIO mode (native or threads)\n" +#endif " --discard=MODE set discard mode (ignore, unmap)\n" " --detect-zeroes=MODE set detect-zeroes mode (off, on, unmap)\n" " --image-opts treat FILE as a full set of image options\n" diff --git a/qemu-options.hx b/qemu-options.hx index 34e9b32a5c..973125cfca 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1338,7 +1338,11 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive, " [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n" " [,snapshot=on|off][,rerror=ignore|stop|report]\n" " [,werror=ignore|stop|report|enospc][,id=name]\n" - " [,aio=threads|native|io_uring]\n" + " [,aio=threads|native" +#if defined(CONFIG_LINUX_IO_URING) + "|io_uring" +#endif + "]\n" " [,readonly=on|off][,copy-on-read=on|off]\n" " [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n" " [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n" -- 2.35.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Only advertise aio=io_uring if support is actually available 2022-04-19 17:19 [PATCH] Only advertise aio=io_uring if support is actually available Dirk Müller @ 2022-04-19 19:23 ` Eric Blake 2022-04-20 8:33 ` Daniel P. Berrangé 1 sibling, 0 replies; 5+ messages in thread From: Eric Blake @ 2022-04-19 19:23 UTC (permalink / raw) To: Dirk Müller; +Cc: qemu-devel, qemu-block On Tue, Apr 19, 2022 at 07:19:31PM +0200, Dirk Müller wrote: > This allows $qemu --help runtime configure checks for detecting > the host support. --help is human-parseable, and thus not stable text. Relying on it for detecting host support is not always ideal, but anything is better than nothing, so I'm not opposed to this patch. Reviewed-by: Eric Blake <eblake@redhat.com> Since qemu-nbd is affected, I'm happy to queue this through my NBD tree now that 7.1 development has opened, if no one else queues it elsewhere first. > > Signed-off-by: Dirk Müller <dmueller@suse.de> > --- > block/file-posix.c | 4 ++++ > qemu-nbd.c | 4 ++++ > qemu-options.hx | 6 +++++- > 3 files changed, 13 insertions(+), 1 deletion(-) > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Only advertise aio=io_uring if support is actually available 2022-04-19 17:19 [PATCH] Only advertise aio=io_uring if support is actually available Dirk Müller 2022-04-19 19:23 ` Eric Blake @ 2022-04-20 8:33 ` Daniel P. Berrangé 2022-04-21 16:51 ` Dirk Müller 2022-04-25 11:06 ` Dirk Müller 1 sibling, 2 replies; 5+ messages in thread From: Daniel P. Berrangé @ 2022-04-20 8:33 UTC (permalink / raw) To: Dirk Müller; +Cc: eblake, qemu-devel, qemu-block On Tue, Apr 19, 2022 at 07:19:31PM +0200, Dirk Müller wrote: > This allows $qemu --help runtime configure checks for detecting > the host support. Note: detecting features by parsing --help output is something that is explicitly a non-goal for QEMU. The only supported interface for detecting features is QMP. We can't actively prevent people writing code that parses --help, but if such parsing breaks on arrival of new QEMU releases that is not considered a bug on the QEMU side. That all said, the patch itself is OK, because for human targetted interactive usage, it is desirable for --help to be representative of what's available. IOW, I'm just complaining about the commit message justification here & warning against writing tools to use --help :-) > Signed-off-by: Dirk Müller <dmueller@suse.de> > --- > block/file-posix.c | 4 ++++ > qemu-nbd.c | 4 ++++ > qemu-options.hx | 6 +++++- > 3 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/block/file-posix.c b/block/file-posix.c > index 39a3d6dbe6..aec4763862 100644 > --- a/block/file-posix.c > +++ b/block/file-posix.c > @@ -544,7 +544,11 @@ static QemuOptsList raw_runtime_opts = { > { > .name = "aio", > .type = QEMU_OPT_STRING, > +#ifdef CONFIG_LINUX_IO_URING > .help = "host AIO implementation (threads, native, io_uring)", > +#else > + .help = "host AIO implementation (threads, native)", > +#endif If we're going to conditionalize this, then we really ought to be address it fully, because 'native' is also platform specific. IOW, we would end up needing something more like this: .help = "host AIO implementation (threads" #if defined(WIN32) || defined(CONFIG_LINUX_AIO) ", native" #endif #if defined(CONFIG_LINUX_IO_URING) ", io_uring" #else ")," admittedly pretty ugly > }, > { > .name = "aio-max-batch", > diff --git a/qemu-nbd.c b/qemu-nbd.c > index 713e7557a9..4634a0fc42 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -147,7 +147,11 @@ static void usage(const char *name) > " --cache=MODE set cache mode used to access the disk image, the\n" > " valid options are: 'none', 'writeback' (default),\n" > " 'writethrough', 'directsync' and 'unsafe'\n" > +#ifdef CONFIG_LINUX_IO_URING > " --aio=MODE set AIO mode (native, io_uring or threads)\n" > +#else > +" --aio=MODE set AIO mode (native or threads)\n" > +#endif > " --discard=MODE set discard mode (ignore, unmap)\n" > " --detect-zeroes=MODE set detect-zeroes mode (off, on, unmap)\n" > " --image-opts treat FILE as a full set of image options\n" > diff --git a/qemu-options.hx b/qemu-options.hx > index 34e9b32a5c..973125cfca 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -1338,7 +1338,11 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive, > " [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n" > " [,snapshot=on|off][,rerror=ignore|stop|report]\n" > " [,werror=ignore|stop|report|enospc][,id=name]\n" > - " [,aio=threads|native|io_uring]\n" > + " [,aio=threads|native" > +#if defined(CONFIG_LINUX_IO_URING) > + "|io_uring" > +#endif > + "]\n" > " [,readonly=on|off][,copy-on-read=on|off]\n" > " [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n" > " [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n" > -- > 2.35.3 > > With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Only advertise aio=io_uring if support is actually available 2022-04-20 8:33 ` Daniel P. Berrangé @ 2022-04-21 16:51 ` Dirk Müller 2022-04-25 11:06 ` Dirk Müller 1 sibling, 0 replies; 5+ messages in thread From: Dirk Müller @ 2022-04-21 16:51 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: eblake, qemu-devel, qemu-block On Mittwoch, 20. April 2022 10:33:38 CEST Daniel P. Berrangé wrote: Hi Daniel, > That all said, the patch itself is OK, because for human targetted > interactive usage, it is desirable for --help to be representative > of what's available. > > IOW, I'm just complaining about the commit message justification > here & warning against writing tools to use --help :-) understood, thank you for the good feedback, I've sent a v2 with a reworded commit message. Greetings, Dirk ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Only advertise aio=io_uring if support is actually available 2022-04-20 8:33 ` Daniel P. Berrangé 2022-04-21 16:51 ` Dirk Müller @ 2022-04-25 11:06 ` Dirk Müller 1 sibling, 0 replies; 5+ messages in thread From: Dirk Müller @ 2022-04-25 11:06 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: eblake, qemu-devel, qemu-block On Mittwoch, 20. April 2022 10:33:38 CEST Daniel P. Berrangé wrote: Hi Daniel, > If we're going to conditionalize this, then we really ought to be > address it fully, because 'native' is also platform specific. Good point. I can do that as well. > IOW, we would end up needing something more like this: > > .help = "host AIO implementation (threads" > #if defined(WIN32) || defined(CONFIG_LINUX_AIO) > ", native" > #endif > #if defined(CONFIG_LINUX_IO_URING) > ", io_uring" > #else > ")," > > admittedly pretty ugly the only other option I came up with so far is something like this: +#if defined(CONFIG_LINUX_AIO) +#define IF_CONFIG_LINUX_AIO(X) X +#else +#define IF_CONFIG_LINUX_AIO(X) +#endif + +#if defined(CONFIG_LINUX_IO_URING) +#define IF_CONFIG_LINUX_IO_URING(X) X +#else +#define IF_CONFIG_LINUX_IO_URING(X) +#endif + and then use it like - .help = "host AIO implementation (threads" -#ifdef CONFIG_LINUX_AIO - ", native" -#endif -#ifdef CONFIG_LINUX_IO_URING - ", io_uring" -#endif - ")", + .help = "host AIO implementation (threads" IF_CONFIG_LINUX_AIO(", native") IF_CONFIG_LINUX_IO_URING(", io_uring") ")", WDYT, is that worth it? Thanks, Dirk ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-04-25 11:07 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-19 17:19 [PATCH] Only advertise aio=io_uring if support is actually available Dirk Müller 2022-04-19 19:23 ` Eric Blake 2022-04-20 8:33 ` Daniel P. Berrangé 2022-04-21 16:51 ` Dirk Müller 2022-04-25 11:06 ` Dirk Müller
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.