On 09/20/2016 04:08 PM, Kevin Wolf wrote:
> The option whether or not to use a native AIO interface really isn't a
> generic option for all drivers, but only applies to the native file
> protocols. This patch moves the option in blockdev-add to the
> appropriate places (raw-posix and raw-win32).
>
> We still have to keep the flag BDRV_O_NATIVE_AIO for compatibility
> because so far the AIO option was usually specified on the wrong layer
> (the top-level format driver, which didn't even look at it) and then
> inherited by the protocol driver (where it was actually used). We can't
> forbid this use except in new interfaces.
>
> Signed-off-by: Kevin Wolf <kw...@redhat.com>
> ---
> block/raw-posix.c | 45 ++++++++++++++++++++++++++++-----------------
> block/raw-win32.c | 50
> +++++++++++++++++++++++++++++++++++++++++++++-----
> qapi/block-core.json | 6 +++---
> tests/qemu-iotests/087 | 4 ++--
> 4 files changed, 78 insertions(+), 27 deletions(-)
>
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 6ed7547..beb86a3 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -143,6 +143,7 @@ typedef struct BDRVRawState {
> bool has_discard:1;
> bool has_write_zeroes:1;
> bool discard_zeroes:1;
> + bool use_linux_aio:1;
> bool has_fallocate;
> bool needs_alignment;
Not this patch's concern, but why are some of our bools packed in a
bitfield and others used with a full byte? Does the performance vs.
size tradeoff even matter for any of the members concerned?
> } BDRVRawState;
> @@ -367,18 +368,6 @@ static void raw_parse_flags(int bdrv_flags, int
> *open_flags)
> }
> }
>
> -#ifdef CONFIG_LINUX_AIO
> -static bool raw_use_aio(int bdrv_flags)
> -{
> - /*
> - * Currently Linux do AIO only for files opened with O_DIRECT
> - * specified so check NOCACHE flag too
Nice; we're getting rid of some awkward grammar.
> @@ -429,6 +424,19 @@ static int raw_open_common(BlockDriverState *bs, QDict
> *options,
> goto fail;
> }
>
> + aio = qemu_opt_get(opts, "aio");
> + if (!aio) {
> + s->use_linux_aio = !!(bdrv_flags & BDRV_O_NATIVE_AIO);
> + } else if (!strcmp(aio, "native")) {
> + s->use_linux_aio = true;
> + } else if (!strcmp(aio, "threads")) {
> + s->use_linux_aio = false;
> + } else {
> + error_setg(errp, "invalid aio option");
> + ret = -EINVAL;
> + goto fail;
> + }
> +
> +++ b/block/raw-win32.c
>
> +static bool get_aio_option(QemuOpts *opts, int flags, Error **errp)
> +{
> + const char *aio = qemu_opt_get(opts, "aio");
> + if (!aio) {
> + return !!(flags & BDRV_O_NATIVE_AIO);
> + } else if (!strcmp(aio, "native")) {
> + return true;
> + } else if (!strcmp(aio, "threads")) {
> + return false;
> + }
> +
> + error_setg(errp, "invalid aio option");
> + return false;
> +}
Is there somewhere common to share this, to avoid duplication?
> +++ b/qapi/block-core.json
> @@ -1724,11 +1724,13 @@
> # Driver specific block device options for the file backend.
> #
> # @filename: path to the image file
> +# @aio: #optional AIO backend (default: threads)
> #
> # Since: 1.7
> ##
> { 'struct': 'BlockdevOptionsFile',
> - 'data': { 'filename': 'str' } }
> + 'data': { 'filename': 'str',
> + '*aio': 'BlockdevAioOptions' } }
>
> +++ b/tests/qemu-iotests/087
> @@ -117,10 +117,10 @@ run_qemu <<EOF
> "options": {
> "driver": "$IMGFMT",
> "node-name": "disk",
> - "aio": "native",
> "file": {
> "driver": "file",
> - "filename": "$TEST_IMG"
> + "filename": "$TEST_IMG",
> + "aio": "native"
Looks like a reasonable relocation. And more evidence that blockdev-add
is not yet usable; hopefully we get there in 2.8.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
