* [Qemu-devel] [PATCH v2 1/7] block: Drop aio/cache consistency check from qmp_blockdev_add()
2016-09-23 14:32 [Qemu-devel] [PATCH v2 0/7] block: Make more blockdev-add options work Kevin Wolf
@ 2016-09-23 14:32 ` Kevin Wolf
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 2/7] block/qapi: Use separate options type for curl driver Kevin Wolf
` (7 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-23 14:32 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel
The TODO comment has been addressed a while ago and this is now checked
in raw-posix, so we don't have to special case this in blockdev-add any
more.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
blockdev.c | 15 ---------------
tests/qemu-iotests/087.out | 2 +-
2 files changed, 1 insertion(+), 16 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 814d49f..3508ff3 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3832,21 +3832,6 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
QDict *qdict;
Error *local_err = NULL;
- /* TODO Sort it out in raw-posix and drive_new(): Reject aio=native with
- * cache.direct=false instead of silently switching to aio=threads, except
- * when called from drive_new().
- *
- * For now, simply forbidding the combination for all drivers will do. */
- if (options->has_aio && options->aio == BLOCKDEV_AIO_OPTIONS_NATIVE) {
- bool direct = options->has_cache &&
- options->cache->has_direct &&
- options->cache->direct;
- if (!direct) {
- error_setg(errp, "aio=native requires cache.direct=true");
- goto fail;
- }
- }
-
visit_type_BlockdevOptions(v, NULL, &options, &local_err);
if (local_err) {
error_propagate(errp, local_err);
diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
index bef6862..cd02eae 100644
--- a/tests/qemu-iotests/087.out
+++ b/tests/qemu-iotests/087.out
@@ -27,7 +27,7 @@ QMP_VERSION
Testing:
QMP_VERSION
{"return": {}}
-{"error": {"class": "GenericError", "desc": "aio=native requires cache.direct=true"}}
+{"error": {"class": "GenericError", "desc": "aio=native was specified, but it requires cache.direct=on, which was not specified."}}
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v2 2/7] block/qapi: Use separate options type for curl driver
2016-09-23 14:32 [Qemu-devel] [PATCH v2 0/7] block: Make more blockdev-add options work Kevin Wolf
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 1/7] block: Drop aio/cache consistency check from qmp_blockdev_add() Kevin Wolf
@ 2016-09-23 14:32 ` Kevin Wolf
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 3/7] block/qapi: Move 'aio' option to file driver Kevin Wolf
` (6 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-23 14:32 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel
We're going to add an option to the file drivers which doesn't apply to
the curl drivers, so give them a separate option type.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
qapi/block-core.json | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 92193ab..b5fdd42 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1721,8 +1721,7 @@
##
# @BlockdevOptionsFile
#
-# Driver specific block device options for the file backend and similar
-# protocols.
+# Driver specific block device options for the file backend.
#
# @filename: path to the image file
#
@@ -2211,6 +2210,18 @@
'*top-id': 'str' } }
##
+# @BlockdevOptionsCurl
+#
+# Driver specific block device options for the curl backend.
+#
+# @filename: path to the image file
+#
+# Since: 1.7
+##
+{ 'struct': 'BlockdevOptionsCurl',
+ 'data': { 'filename': 'str' } }
+
+##
# @BlockdevOptions
#
# Options for creating a block device. Many options are available for all
@@ -2248,13 +2259,13 @@
'cloop': 'BlockdevOptionsGenericFormat',
'dmg': 'BlockdevOptionsGenericFormat',
'file': 'BlockdevOptionsFile',
- 'ftp': 'BlockdevOptionsFile',
- 'ftps': 'BlockdevOptionsFile',
+ 'ftp': 'BlockdevOptionsCurl',
+ 'ftps': 'BlockdevOptionsCurl',
'gluster': 'BlockdevOptionsGluster',
'host_cdrom': 'BlockdevOptionsFile',
'host_device':'BlockdevOptionsFile',
- 'http': 'BlockdevOptionsFile',
- 'https': 'BlockdevOptionsFile',
+ 'http': 'BlockdevOptionsCurl',
+ 'https': 'BlockdevOptionsCurl',
# TODO iscsi: Wait for structured options
'luks': 'BlockdevOptionsLUKS',
# TODO nbd: Should take InetSocketAddress for 'host'?
@@ -2271,7 +2282,7 @@
'replication':'BlockdevOptionsReplication',
# TODO sheepdog: Wait for structured options
# TODO ssh: Should take InetSocketAddress for 'host'?
- 'tftp': 'BlockdevOptionsFile',
+ 'tftp': 'BlockdevOptionsCurl',
'vdi': 'BlockdevOptionsGenericFormat',
'vhdx': 'BlockdevOptionsGenericFormat',
'vmdk': 'BlockdevOptionsGenericCOWFormat',
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v2 3/7] block/qapi: Move 'aio' option to file driver
2016-09-23 14:32 [Qemu-devel] [PATCH v2 0/7] block: Make more blockdev-add options work Kevin Wolf
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 1/7] block: Drop aio/cache consistency check from qmp_blockdev_add() Kevin Wolf
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 2/7] block/qapi: Use separate options type for curl driver Kevin Wolf
@ 2016-09-23 14:32 ` Kevin Wolf
2016-09-23 14:40 ` Eric Blake
2016-09-26 16:49 ` Max Reitz
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 4/7] block: Parse 'detect-zeroes' in bdrv_open_common() Kevin Wolf
` (5 subsequent siblings)
8 siblings, 2 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-23 14:32 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel
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 <kwolf@redhat.com>
---
block/raw-posix.c | 44 ++++++++++++++++++++++++---------------
block/raw-win32.c | 56 +++++++++++++++++++++++++++++++++++++++++++++-----
qapi/block-core.json | 6 +++---
tests/qemu-iotests/087 | 4 ++--
4 files changed, 83 insertions(+), 27 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 6ed7547..166e9d1 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;
} 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
- */
- return (bdrv_flags & (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) ==
- (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO);
-}
-#endif
-
static void raw_parse_filename(const char *filename, QDict *options,
Error **errp)
{
@@ -399,6 +388,11 @@ static QemuOptsList raw_runtime_opts = {
.type = QEMU_OPT_STRING,
.help = "File name of the image",
},
+ {
+ .name = "aio",
+ .type = QEMU_OPT_STRING,
+ .help = "host AIO implementation (threads, native)",
+ },
{ /* end of list */ }
},
};
@@ -410,6 +404,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
QemuOpts *opts;
Error *local_err = NULL;
const char *filename = NULL;
+ BlockdevAioOptions aio, aio_default;
int fd, ret;
struct stat st;
@@ -429,6 +424,18 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
goto fail;
}
+ aio_default = (bdrv_flags & BDRV_O_NATIVE_AIO)
+ ? BLOCKDEV_AIO_OPTIONS_NATIVE
+ : BLOCKDEV_AIO_OPTIONS_THREADS;
+ aio = qapi_enum_parse(BlockdevAioOptions_lookup, qemu_opt_get(opts, "aio"),
+ BLOCKDEV_AIO_OPTIONS__MAX, aio_default, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ ret = -EINVAL;
+ goto fail;
+ }
+ s->use_linux_aio = (aio == BLOCKDEV_AIO_OPTIONS_NATIVE);
+
s->open_flags = open_flags;
raw_parse_flags(bdrv_flags, &s->open_flags);
@@ -444,14 +451,15 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
s->fd = fd;
#ifdef CONFIG_LINUX_AIO
- if (!raw_use_aio(bdrv_flags) && (bdrv_flags & BDRV_O_NATIVE_AIO)) {
+ /* Currently Linux does AIO only for files opened with O_DIRECT */
+ if (s->use_linux_aio && !(s->open_flags & O_DIRECT)) {
error_setg(errp, "aio=native was specified, but it requires "
"cache.direct=on, which was not specified.");
ret = -EINVAL;
goto fail;
}
#else
- if (bdrv_flags & BDRV_O_NATIVE_AIO) {
+ if (s->use_linux_aio) {
error_setg(errp, "aio=native was specified, but is not supported "
"in this build.");
ret = -EINVAL;
@@ -1256,7 +1264,7 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
if (!bdrv_qiov_is_aligned(bs, qiov)) {
type |= QEMU_AIO_MISALIGNED;
#ifdef CONFIG_LINUX_AIO
- } else if (bs->open_flags & BDRV_O_NATIVE_AIO) {
+ } else if (s->use_linux_aio) {
LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
assert(qiov->size == bytes);
return laio_co_submit(bs, aio, s->fd, offset, qiov, type);
@@ -1285,7 +1293,8 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, uint64_t offset,
static void raw_aio_plug(BlockDriverState *bs)
{
#ifdef CONFIG_LINUX_AIO
- if (bs->open_flags & BDRV_O_NATIVE_AIO) {
+ BDRVRawState *s = bs->opaque;
+ if (s->use_linux_aio) {
LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
laio_io_plug(bs, aio);
}
@@ -1295,7 +1304,8 @@ static void raw_aio_plug(BlockDriverState *bs)
static void raw_aio_unplug(BlockDriverState *bs)
{
#ifdef CONFIG_LINUX_AIO
- if (bs->open_flags & BDRV_O_NATIVE_AIO) {
+ BDRVRawState *s = bs->opaque;
+ if (s->use_linux_aio) {
LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
laio_io_unplug(bs, aio);
}
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 56f45fe..734bb10 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -32,6 +32,7 @@
#include "block/thread-pool.h"
#include "qemu/iov.h"
#include "qapi/qmp/qstring.h"
+#include "qapi/util.h"
#include <windows.h>
#include <winioctl.h>
@@ -252,7 +253,8 @@ static void raw_probe_alignment(BlockDriverState *bs, Error **errp)
}
}
-static void raw_parse_flags(int flags, int *access_flags, DWORD *overlapped)
+static void raw_parse_flags(int flags, bool use_aio, int *access_flags,
+ DWORD *overlapped)
{
assert(access_flags != NULL);
assert(overlapped != NULL);
@@ -264,7 +266,7 @@ static void raw_parse_flags(int flags, int *access_flags, DWORD *overlapped)
}
*overlapped = FILE_ATTRIBUTE_NORMAL;
- if (flags & BDRV_O_NATIVE_AIO) {
+ if (use_aio) {
*overlapped |= FILE_FLAG_OVERLAPPED;
}
if (flags & BDRV_O_NOCACHE) {
@@ -292,10 +294,35 @@ static QemuOptsList raw_runtime_opts = {
.type = QEMU_OPT_STRING,
.help = "File name of the image",
},
+ {
+ .name = "aio",
+ .type = QEMU_OPT_STRING,
+ .help = "host AIO implementation (threads, native)",
+ },
{ /* end of list */ }
},
};
+static bool get_aio_option(QemuOpts *opts, int flags, Error **errp)
+{
+ BlockdevAioOptions aio, aio_default;
+
+ aio_default = (flags & BDRV_O_NATIVE_AIO) ? BLOCKDEV_AIO_OPTIONS_NATIVE
+ : BLOCKDEV_AIO_OPTIONS_THREADS;
+ aio = qapi_enum_parse(BlockdevAioOptions_lookup, qemu_opt_get(opts, "aio"),
+ BLOCKDEV_AIO_OPTIONS__MAX, aio_default, errp);
+
+ switch (aio) {
+ case BLOCKDEV_AIO_OPTIONS_NATIVE:
+ return true;
+ case BLOCKDEV_AIO_OPTIONS_THREADS:
+ return false;
+ default:
+ error_setg(errp, "Invalid AIO option");
+ }
+ return false;
+}
+
static int raw_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
@@ -305,6 +332,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
QemuOpts *opts;
Error *local_err = NULL;
const char *filename;
+ bool use_aio;
int ret;
s->type = FTYPE_FILE;
@@ -319,7 +347,14 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
filename = qemu_opt_get(opts, "filename");
- raw_parse_flags(flags, &access_flags, &overlapped);
+ use_aio = get_aio_option(opts, flags, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ ret = -EINVAL;
+ goto fail;
+ }
+
+ raw_parse_flags(flags, use_aio, &access_flags, &overlapped);
if (filename[0] && filename[1] == ':') {
snprintf(s->drive_path, sizeof(s->drive_path), "%c:\\", filename[0]);
@@ -346,7 +381,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
goto fail;
}
- if (flags & BDRV_O_NATIVE_AIO) {
+ if (use_aio) {
s->aio = win32_aio_init();
if (s->aio == NULL) {
CloseHandle(s->hfile);
@@ -647,6 +682,7 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
Error *local_err = NULL;
const char *filename;
+ bool use_aio;
QemuOpts *opts = qemu_opts_create(&raw_runtime_opts, NULL, 0,
&error_abort);
@@ -659,6 +695,16 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
filename = qemu_opt_get(opts, "filename");
+ use_aio = get_aio_option(opts, flags, &local_err);
+ if (!local_err && use_aio) {
+ error_setg(&local_err, "AIO is not supported on Windows host devices");
+ }
+ if (local_err) {
+ error_propagate(errp, local_err);
+ ret = -EINVAL;
+ goto done;
+ }
+
if (strstart(filename, "/dev/cdrom", NULL)) {
if (find_cdrom(device_name, sizeof(device_name)) < 0) {
error_setg(errp, "Could not open CD-ROM drive");
@@ -677,7 +723,7 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
}
s->type = find_device_type(bs, filename);
- raw_parse_flags(flags, &access_flags, &overlapped);
+ raw_parse_flags(flags, use_aio, &access_flags, &overlapped);
create_flags = OPEN_EXISTING;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b5fdd42..0939a6b 100644
--- a/qapi/block-core.json
+++ 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' } }
##
# @BlockdevOptionsNull
@@ -2232,7 +2234,6 @@
# This option is required on the top level of blockdev-add.
# @discard: #optional discard-related options (default: ignore)
# @cache: #optional cache-related options
-# @aio: #optional AIO backend (default: threads)
# @read-only: #optional whether the block device should be read-only
# (default: false)
# @detect-zeroes: #optional detect and optimize zero writes (Since 2.1)
@@ -2247,7 +2248,6 @@
'*node-name': 'str',
'*discard': 'BlockdevDiscardOptions',
'*cache': 'BlockdevCacheOptions',
- '*aio': 'BlockdevAioOptions',
'*read-only': 'bool',
'*detect-zeroes': 'BlockdevDetectZeroesOptions' },
'discriminator': 'driver',
diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
index 5c04577..b1ac71f 100755
--- a/tests/qemu-iotests/087
+++ 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"
}
}
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v2 3/7] block/qapi: Move 'aio' option to file driver
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 3/7] block/qapi: Move 'aio' option to file driver Kevin Wolf
@ 2016-09-23 14:40 ` Eric Blake
2016-09-26 9:16 ` Kevin Wolf
2016-09-26 16:49 ` Max Reitz
1 sibling, 1 reply; 17+ messages in thread
From: Eric Blake @ 2016-09-23 14:40 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1520 bytes --]
On 09/23/2016 09:32 AM, 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 <kwolf@redhat.com>
> ---
> block/raw-posix.c | 44 ++++++++++++++++++++++++---------------
> block/raw-win32.c | 56 +++++++++++++++++++++++++++++++++++++++++++++-----
> qapi/block-core.json | 6 +++---
> tests/qemu-iotests/087 | 4 ++--
> 4 files changed, 83 insertions(+), 27 deletions(-)
>
> +++ 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)
Missed this last time, but probably worth a '(since 2.8)' marker.
Trivial enough that you can squash in during the pull request, so:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/7] block/qapi: Move 'aio' option to file driver
2016-09-23 14:40 ` Eric Blake
@ 2016-09-26 9:16 ` Kevin Wolf
0 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-26 9:16 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-block, mreitz, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]
Am 23.09.2016 um 16:40 hat Eric Blake geschrieben:
> On 09/23/2016 09:32 AM, 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 <kwolf@redhat.com>
> > ---
> > block/raw-posix.c | 44 ++++++++++++++++++++++++---------------
> > block/raw-win32.c | 56 +++++++++++++++++++++++++++++++++++++++++++++-----
> > qapi/block-core.json | 6 +++---
> > tests/qemu-iotests/087 | 4 ++--
> > 4 files changed, 83 insertions(+), 27 deletions(-)
> >
>
> > +++ 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)
>
> Missed this last time, but probably worth a '(since 2.8)' marker.
I'm not sure how useful this is when the whole blockdev-add command is
still experimental and we're going to break it incompatibly by removing
the "options" layer. But we have the annotation elsewhere, so I'll add
it.
Maybe the patch that breaks compatibility should remove the annotation
everywhere again.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/7] block/qapi: Move 'aio' option to file driver
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 3/7] block/qapi: Move 'aio' option to file driver Kevin Wolf
2016-09-23 14:40 ` Eric Blake
@ 2016-09-26 16:49 ` Max Reitz
2016-09-27 8:20 ` Kevin Wolf
1 sibling, 1 reply; 17+ messages in thread
From: Max Reitz @ 2016-09-26 16:49 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: eblake, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]
On 23.09.2016 16:32, 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 <kwolf@redhat.com>
> ---
> block/raw-posix.c | 44 ++++++++++++++++++++++++---------------
> block/raw-win32.c | 56 +++++++++++++++++++++++++++++++++++++++++++++-----
> qapi/block-core.json | 6 +++---
> tests/qemu-iotests/087 | 4 ++--
> 4 files changed, 83 insertions(+), 27 deletions(-)
[...]
> diff --git a/block/raw-win32.c b/block/raw-win32.c
> index 56f45fe..734bb10 100644
> --- a/block/raw-win32.c
> +++ b/block/raw-win32.c
[...]
> +static bool get_aio_option(QemuOpts *opts, int flags, Error **errp)
> +{
> + BlockdevAioOptions aio, aio_default;
> +
> + aio_default = (flags & BDRV_O_NATIVE_AIO) ? BLOCKDEV_AIO_OPTIONS_NATIVE
> + : BLOCKDEV_AIO_OPTIONS_THREADS;
> + aio = qapi_enum_parse(BlockdevAioOptions_lookup, qemu_opt_get(opts, "aio"),
> + BLOCKDEV_AIO_OPTIONS__MAX, aio_default, errp);
> +
> + switch (aio) {
> + case BLOCKDEV_AIO_OPTIONS_NATIVE:
> + return true;
> + case BLOCKDEV_AIO_OPTIONS_THREADS:
> + return false;
> + default:
> + error_setg(errp, "Invalid AIO option");
Any reason for catching this case here but not in raw-posix?
(Not that it really matters, though.)
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 480 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v2 3/7] block/qapi: Move 'aio' option to file driver
2016-09-26 16:49 ` Max Reitz
@ 2016-09-27 8:20 ` Kevin Wolf
2016-09-27 19:43 ` Max Reitz
0 siblings, 1 reply; 17+ messages in thread
From: Kevin Wolf @ 2016-09-27 8:20 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, eblake, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2308 bytes --]
Am 26.09.2016 um 18:49 hat Max Reitz geschrieben:
> On 23.09.2016 16:32, 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 <kwolf@redhat.com>
> > ---
> > block/raw-posix.c | 44 ++++++++++++++++++++++++---------------
> > block/raw-win32.c | 56 +++++++++++++++++++++++++++++++++++++++++++++-----
> > qapi/block-core.json | 6 +++---
> > tests/qemu-iotests/087 | 4 ++--
> > 4 files changed, 83 insertions(+), 27 deletions(-)
>
> [...]
>
> > diff --git a/block/raw-win32.c b/block/raw-win32.c
> > index 56f45fe..734bb10 100644
> > --- a/block/raw-win32.c
> > +++ b/block/raw-win32.c
>
> [...]
>
> > +static bool get_aio_option(QemuOpts *opts, int flags, Error **errp)
> > +{
> > + BlockdevAioOptions aio, aio_default;
> > +
> > + aio_default = (flags & BDRV_O_NATIVE_AIO) ? BLOCKDEV_AIO_OPTIONS_NATIVE
> > + : BLOCKDEV_AIO_OPTIONS_THREADS;
> > + aio = qapi_enum_parse(BlockdevAioOptions_lookup, qemu_opt_get(opts, "aio"),
> > + BLOCKDEV_AIO_OPTIONS__MAX, aio_default, errp);
> > +
> > + switch (aio) {
> > + case BLOCKDEV_AIO_OPTIONS_NATIVE:
> > + return true;
> > + case BLOCKDEV_AIO_OPTIONS_THREADS:
> > + return false;
> > + default:
> > + error_setg(errp, "Invalid AIO option");
>
> Any reason for catching this case here but not in raw-posix?
>
> (Not that it really matters, though.)
Nobody will forget raw-posix when adding a new AIO mode to win32, so I
didn't feel like the additional code was worth it there. But if we add a
new AIO mode to raw-posix, I'm pretty sure we will forget win32.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v2 3/7] block/qapi: Move 'aio' option to file driver
2016-09-27 8:20 ` Kevin Wolf
@ 2016-09-27 19:43 ` Max Reitz
0 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2016-09-27 19:43 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, eblake, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2385 bytes --]
On 27.09.2016 10:20, Kevin Wolf wrote:
> Am 26.09.2016 um 18:49 hat Max Reitz geschrieben:
>> On 23.09.2016 16:32, 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 <kwolf@redhat.com>
>>> ---
>>> block/raw-posix.c | 44 ++++++++++++++++++++++++---------------
>>> block/raw-win32.c | 56 +++++++++++++++++++++++++++++++++++++++++++++-----
>>> qapi/block-core.json | 6 +++---
>>> tests/qemu-iotests/087 | 4 ++--
>>> 4 files changed, 83 insertions(+), 27 deletions(-)
>>
>> [...]
>>
>>> diff --git a/block/raw-win32.c b/block/raw-win32.c
>>> index 56f45fe..734bb10 100644
>>> --- a/block/raw-win32.c
>>> +++ b/block/raw-win32.c
>>
>> [...]
>>
>>> +static bool get_aio_option(QemuOpts *opts, int flags, Error **errp)
>>> +{
>>> + BlockdevAioOptions aio, aio_default;
>>> +
>>> + aio_default = (flags & BDRV_O_NATIVE_AIO) ? BLOCKDEV_AIO_OPTIONS_NATIVE
>>> + : BLOCKDEV_AIO_OPTIONS_THREADS;
>>> + aio = qapi_enum_parse(BlockdevAioOptions_lookup, qemu_opt_get(opts, "aio"),
>>> + BLOCKDEV_AIO_OPTIONS__MAX, aio_default, errp);
>>> +
>>> + switch (aio) {
>>> + case BLOCKDEV_AIO_OPTIONS_NATIVE:
>>> + return true;
>>> + case BLOCKDEV_AIO_OPTIONS_THREADS:
>>> + return false;
>>> + default:
>>> + error_setg(errp, "Invalid AIO option");
>>
>> Any reason for catching this case here but not in raw-posix?
>>
>> (Not that it really matters, though.)
>
> Nobody will forget raw-posix when adding a new AIO mode to win32, so I
> didn't feel like the additional code was worth it there. But if we add a
> new AIO mode to raw-posix, I'm pretty sure we will forget win32.
:-)
Good point.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 480 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v2 4/7] block: Parse 'detect-zeroes' in bdrv_open_common()
2016-09-23 14:32 [Qemu-devel] [PATCH v2 0/7] block: Make more blockdev-add options work Kevin Wolf
` (2 preceding siblings ...)
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 3/7] block/qapi: Move 'aio' option to file driver Kevin Wolf
@ 2016-09-23 14:32 ` Kevin Wolf
2016-09-23 14:41 ` Eric Blake
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 5/7] block: Use 'detect-zeroes' option for 'blockdev-change-medium' Kevin Wolf
` (4 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Kevin Wolf @ 2016-09-23 14:32 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel
Amongst others, this means that you can now use the 'detect-zeroes'
option for non-top-level nodes in blockdev-add, like the QAPI schema
promises.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 33 +++++++++++++++++++++++++++++++++
blockdev.c | 9 +--------
2 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/block.c b/block.c
index 493ecf3..1f10457 100644
--- a/block.c
+++ b/block.c
@@ -42,6 +42,7 @@
#include "qapi-event.h"
#include "qemu/cutils.h"
#include "qemu/id.h"
+#include "qapi/util.h"
#ifdef CONFIG_BSD
#include <sys/ioctl.h>
@@ -954,6 +955,11 @@ static QemuOptsList bdrv_runtime_opts = {
.type = QEMU_OPT_BOOL,
.help = "Node is opened in read-only mode",
},
+ {
+ .name = "detect-zeroes",
+ .type = QEMU_OPT_STRING,
+ .help = "try to optimize zero writes (off, on, unmap)",
+ },
{ /* end of list */ }
},
};
@@ -970,6 +976,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
const char *filename;
const char *driver_name = NULL;
const char *node_name = NULL;
+ const char *detect_zeroes;
QemuOpts *opts;
BlockDriver *drv;
Error *local_err = NULL;
@@ -1038,6 +1045,32 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
}
}
+ detect_zeroes = qemu_opt_get(opts, "detect-zeroes");
+ if (detect_zeroes) {
+ BlockdevDetectZeroesOptions value =
+ qapi_enum_parse(BlockdevDetectZeroesOptions_lookup,
+ detect_zeroes,
+ BLOCKDEV_DETECT_ZEROES_OPTIONS__MAX,
+ BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ ret = -EINVAL;
+ goto fail_opts;
+ }
+
+ if (value == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP &&
+ !(bs->open_flags & BDRV_O_UNMAP))
+ {
+ error_setg(errp, "setting detect-zeroes to unmap is not allowed "
+ "without setting discard operation to unmap");
+ ret = -EINVAL;
+ goto fail_opts;
+ }
+
+ bs->detect_zeroes = value;
+ }
+
if (filename != NULL) {
pstrcpy(bs->filename, sizeof(bs->filename), filename);
} else {
diff --git a/blockdev.c b/blockdev.c
index 3508ff3..7baf5bb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -658,7 +658,6 @@ static BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp)
BlockDriverState *bs;
QemuOpts *opts;
Error *local_error = NULL;
- BlockdevDetectZeroesOptions detect_zeroes;
int bdrv_flags = 0;
opts = qemu_opts_create(&qemu_root_bds_opts, NULL, 1, errp);
@@ -673,7 +672,7 @@ static BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp)
}
extract_common_blockdev_options(opts, &bdrv_flags, NULL, NULL,
- &detect_zeroes, &local_error);
+ NULL, &local_error);
if (local_error) {
error_propagate(errp, local_error);
goto fail;
@@ -695,8 +694,6 @@ static BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp)
goto fail_no_bs_opts;
}
- bs->detect_zeroes = detect_zeroes;
-
fail_no_bs_opts:
qemu_opts_del(opts);
return bs;
@@ -4136,10 +4133,6 @@ static QemuOptsList qemu_root_bds_opts = {
.name = "copy-on-read",
.type = QEMU_OPT_BOOL,
.help = "copy read data from backing file into image file",
- },{
- .name = "detect-zeroes",
- .type = QEMU_OPT_STRING,
- .help = "try to optimize zero writes (off, on, unmap)",
},
{ /* end of list */ }
},
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v2 4/7] block: Parse 'detect-zeroes' in bdrv_open_common()
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 4/7] block: Parse 'detect-zeroes' in bdrv_open_common() Kevin Wolf
@ 2016-09-23 14:41 ` Eric Blake
0 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-09-23 14:41 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 558 bytes --]
On 09/23/2016 09:32 AM, Kevin Wolf wrote:
> Amongst others, this means that you can now use the 'detect-zeroes'
> option for non-top-level nodes in blockdev-add, like the QAPI schema
> promises.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 33 +++++++++++++++++++++++++++++++++
> blockdev.c | 9 +--------
> 2 files changed, 34 insertions(+), 8 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v2 5/7] block: Use 'detect-zeroes' option for 'blockdev-change-medium'
2016-09-23 14:32 [Qemu-devel] [PATCH v2 0/7] block: Make more blockdev-add options work Kevin Wolf
` (3 preceding siblings ...)
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 4/7] block: Parse 'detect-zeroes' in bdrv_open_common() Kevin Wolf
@ 2016-09-23 14:32 ` Kevin Wolf
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 6/7] block: Move 'discard' option to bdrv_open_common() Kevin Wolf
` (3 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-23 14:32 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel
Instead of modifying the new BDS after it has been opened, use the newly
supported 'detect-zeroes' option in bdrv_open_common() so that all
requirements are checked (detect-zeroes=unmap requires discard=unmap).
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/block-backend.c | 9 ++++-----
blockdev.c | 9 ++++++---
include/sysemu/block-backend.h | 2 +-
3 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 0bd19ab..bc14f96 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1592,13 +1592,12 @@ void blk_update_root_state(BlockBackend *blk)
}
/*
- * Applies the information in the root state to the given BlockDriverState. This
- * does not include the flags which have to be specified for bdrv_open(), use
- * blk_get_open_flags_from_root_state() to inquire them.
+ * Returns the detect-zeroes setting to be used for bdrv_open() of a
+ * BlockDriverState which is supposed to inherit the root state.
*/
-void blk_apply_root_state(BlockBackend *blk, BlockDriverState *bs)
+bool blk_get_detect_zeroes_from_root_state(BlockBackend *blk)
{
- bs->detect_zeroes = blk->root_state.detect_zeroes;
+ return blk->root_state.detect_zeroes;
}
/*
diff --git a/blockdev.c b/blockdev.c
index 7baf5bb..d84717d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2546,6 +2546,7 @@ void qmp_blockdev_change_medium(bool has_device, const char *device,
BlockBackend *blk;
BlockDriverState *medium_bs = NULL;
int bdrv_flags;
+ bool detect_zeroes;
int rc;
QDict *options = NULL;
Error *err = NULL;
@@ -2585,8 +2586,12 @@ void qmp_blockdev_change_medium(bool has_device, const char *device,
abort();
}
+ options = qdict_new();
+ detect_zeroes = blk_get_detect_zeroes_from_root_state(blk);
+ qdict_put(options, "detect-zeroes",
+ qstring_from_str(detect_zeroes ? "on" : "off"));
+
if (has_format) {
- options = qdict_new();
qdict_put(options, "driver", qstring_from_str(format));
}
@@ -2623,8 +2628,6 @@ void qmp_blockdev_change_medium(bool has_device, const char *device,
goto fail;
}
- blk_apply_root_state(blk, medium_bs);
-
qmp_blockdev_close_tray(has_device, device, has_id, id, errp);
fail:
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 3b29317..07339b7 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -199,7 +199,7 @@ void blk_io_unplug(BlockBackend *blk);
BlockAcctStats *blk_get_stats(BlockBackend *blk);
BlockBackendRootState *blk_get_root_state(BlockBackend *blk);
void blk_update_root_state(BlockBackend *blk);
-void blk_apply_root_state(BlockBackend *blk, BlockDriverState *bs);
+bool blk_get_detect_zeroes_from_root_state(BlockBackend *blk);
int blk_get_open_flags_from_root_state(BlockBackend *blk);
void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v2 6/7] block: Move 'discard' option to bdrv_open_common()
2016-09-23 14:32 [Qemu-devel] [PATCH v2 0/7] block: Make more blockdev-add options work Kevin Wolf
` (4 preceding siblings ...)
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 5/7] block: Use 'detect-zeroes' option for 'blockdev-change-medium' Kevin Wolf
@ 2016-09-23 14:32 ` Kevin Wolf
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 7/7] block: Remove qemu_root_bds_opts Kevin Wolf
` (2 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-23 14:32 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel
This enables its use for nested child nodes. The compatibility
between the 'discard' and 'detect-zeroes' setting is checked in
bdrv_open_common() now as the former setting isn't available before
calling bdrv_open() any more.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block.c | 17 ++++++++++++++++-
blockdev.c | 25 -------------------------
include/block/block.h | 1 +
3 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/block.c b/block.c
index 1f10457..bb1f1ec 100644
--- a/block.c
+++ b/block.c
@@ -765,7 +765,7 @@ static void bdrv_inherited_options(int *child_flags, QDict *child_options,
/* Our block drivers take care to send flushes and respect unmap policy,
* so we can default to enable both on lower layers regardless of the
* corresponding parent options. */
- flags |= BDRV_O_UNMAP;
+ qdict_set_default_str(child_options, BDRV_OPT_DISCARD, "unmap");
/* Clear flags that only apply to the top layer */
flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_COPY_ON_READ |
@@ -960,6 +960,11 @@ static QemuOptsList bdrv_runtime_opts = {
.type = QEMU_OPT_STRING,
.help = "try to optimize zero writes (off, on, unmap)",
},
+ {
+ .name = "discard",
+ .type = QEMU_OPT_STRING,
+ .help = "discard operation (ignore/off, unmap/on)",
+ },
{ /* end of list */ }
},
};
@@ -976,6 +981,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
const char *filename;
const char *driver_name = NULL;
const char *node_name = NULL;
+ const char *discard;
const char *detect_zeroes;
QemuOpts *opts;
BlockDriver *drv;
@@ -1045,6 +1051,15 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
}
}
+ discard = qemu_opt_get(opts, "discard");
+ if (discard != NULL) {
+ if (bdrv_parse_discard_flags(discard, &bs->open_flags) != 0) {
+ error_setg(errp, "Invalid discard option");
+ ret = -EINVAL;
+ goto fail_opts;
+ }
+ }
+
detect_zeroes = qemu_opt_get(opts, "detect-zeroes");
if (detect_zeroes) {
BlockdevDetectZeroesOptions value =
diff --git a/blockdev.c b/blockdev.c
index d84717d..a4bddbe 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -356,7 +356,6 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
const char **throttling_group, ThrottleConfig *throttle_cfg,
BlockdevDetectZeroesOptions *detect_zeroes, Error **errp)
{
- const char *discard;
Error *local_error = NULL;
const char *aio;
@@ -365,13 +364,6 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
*bdrv_flags |= BDRV_O_COPY_ON_READ;
}
- if ((discard = qemu_opt_get(opts, "discard")) != NULL) {
- if (bdrv_parse_discard_flags(discard, bdrv_flags) != 0) {
- error_setg(errp, "Invalid discard option");
- return;
- }
- }
-
if ((aio = qemu_opt_get(opts, "aio")) != NULL) {
if (!strcmp(aio, "native")) {
*bdrv_flags |= BDRV_O_NATIVE_AIO;
@@ -449,15 +441,6 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
error_propagate(errp, local_error);
return;
}
-
- if (bdrv_flags &&
- *detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP &&
- !(*bdrv_flags & BDRV_O_UNMAP))
- {
- error_setg(errp, "setting detect-zeroes to unmap is not allowed "
- "without setting discard operation to unmap");
- return;
- }
}
}
@@ -3990,10 +3973,6 @@ QemuOptsList qemu_common_drive_opts = {
.type = QEMU_OPT_BOOL,
.help = "enable/disable snapshot mode",
},{
- .name = "discard",
- .type = QEMU_OPT_STRING,
- .help = "discard operation (ignore/off, unmap/on)",
- },{
.name = "aio",
.type = QEMU_OPT_STRING,
.help = "host AIO implementation (threads, native)",
@@ -4125,10 +4104,6 @@ static QemuOptsList qemu_root_bds_opts = {
.head = QTAILQ_HEAD_INITIALIZER(qemu_root_bds_opts.head),
.desc = {
{
- .name = "discard",
- .type = QEMU_OPT_STRING,
- .help = "discard operation (ignore/off, unmap/on)",
- },{
.name = "aio",
.type = QEMU_OPT_STRING,
.help = "host AIO implementation (threads, native)",
diff --git a/include/block/block.h b/include/block/block.h
index e18233a..e572b31 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -108,6 +108,7 @@ typedef struct HDGeometry {
#define BDRV_OPT_CACHE_DIRECT "cache.direct"
#define BDRV_OPT_CACHE_NO_FLUSH "cache.no-flush"
#define BDRV_OPT_READ_ONLY "read-only"
+#define BDRV_OPT_DISCARD "discard"
#define BDRV_SECTOR_BITS 9
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v2 7/7] block: Remove qemu_root_bds_opts
2016-09-23 14:32 [Qemu-devel] [PATCH v2 0/7] block: Make more blockdev-add options work Kevin Wolf
` (5 preceding siblings ...)
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 6/7] block: Move 'discard' option to bdrv_open_common() Kevin Wolf
@ 2016-09-23 14:32 ` Kevin Wolf
2016-09-26 17:10 ` [Qemu-devel] [PATCH v2 0/7] block: Make more blockdev-add options work Max Reitz
2016-09-27 8:28 ` Kevin Wolf
8 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-23 14:32 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel
The remaining options in qemu_root_bds_opts (aio and copy-on-read)
aren't used any more, the QAPI schema doesn't contain them. Therefore
all the code processing qemu_root_bds_opts options is dead and can be
removed.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
blockdev.c | 54 +-----------------------------------------------------
1 file changed, 1 insertion(+), 53 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index a4bddbe..76b86ab 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -633,34 +633,11 @@ err_no_opts:
return NULL;
}
-static QemuOptsList qemu_root_bds_opts;
-
/* Takes the ownership of bs_opts */
static BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp)
{
- BlockDriverState *bs;
- QemuOpts *opts;
- Error *local_error = NULL;
int bdrv_flags = 0;
- opts = qemu_opts_create(&qemu_root_bds_opts, NULL, 1, errp);
- if (!opts) {
- goto fail;
- }
-
- qemu_opts_absorb_qdict(opts, bs_opts, &local_error);
- if (local_error) {
- error_propagate(errp, local_error);
- goto fail;
- }
-
- extract_common_blockdev_options(opts, &bdrv_flags, NULL, NULL,
- NULL, &local_error);
- if (local_error) {
- error_propagate(errp, local_error);
- goto fail;
- }
-
/* bdrv_open() defaults to the values in bdrv_flags (for compatibility
* with other callers) rather than what we want as the real defaults.
* Apply the defaults here instead. */
@@ -672,19 +649,7 @@ static BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp)
bdrv_flags |= BDRV_O_INACTIVE;
}
- bs = bdrv_open(NULL, NULL, bs_opts, bdrv_flags, errp);
- if (!bs) {
- goto fail_no_bs_opts;
- }
-
-fail_no_bs_opts:
- qemu_opts_del(opts);
- return bs;
-
-fail:
- qemu_opts_del(opts);
- QDECREF(bs_opts);
- return NULL;
+ return bdrv_open(NULL, NULL, bs_opts, bdrv_flags, errp);
}
void blockdev_close_all_bdrv_states(void)
@@ -4099,23 +4064,6 @@ QemuOptsList qemu_common_drive_opts = {
},
};
-static QemuOptsList qemu_root_bds_opts = {
- .name = "root-bds",
- .head = QTAILQ_HEAD_INITIALIZER(qemu_root_bds_opts.head),
- .desc = {
- {
- .name = "aio",
- .type = QEMU_OPT_STRING,
- .help = "host AIO implementation (threads, native)",
- },{
- .name = "copy-on-read",
- .type = QEMU_OPT_BOOL,
- .help = "copy read data from backing file into image file",
- },
- { /* end of list */ }
- },
-};
-
QemuOptsList qemu_drive_opts = {
.name = "drive",
.head = QTAILQ_HEAD_INITIALIZER(qemu_drive_opts.head),
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/7] block: Make more blockdev-add options work
2016-09-23 14:32 [Qemu-devel] [PATCH v2 0/7] block: Make more blockdev-add options work Kevin Wolf
` (6 preceding siblings ...)
2016-09-23 14:32 ` [Qemu-devel] [PATCH v2 7/7] block: Remove qemu_root_bds_opts Kevin Wolf
@ 2016-09-26 17:10 ` Max Reitz
2016-09-27 8:26 ` Kevin Wolf
2016-09-27 8:28 ` Kevin Wolf
8 siblings, 1 reply; 17+ messages in thread
From: Max Reitz @ 2016-09-26 17:10 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: eblake, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2134 bytes --]
On 23.09.2016 16:32, Kevin Wolf wrote:
> This series moves a few more options from flags to the appropriate place. This
> doesn't only result in cleaner code, but also allows using these options in
> nested node definitions.
>
> After this series, bds_tree_init() is only a thin wrapper around bdrv_open()
> which sets the right defaults for cache modes and the BDRV_O_INACTIVE flag if
> necessary.
>
> Depends on my block branch, specifically Berto's 'read-only' patches.
>
> v2:
> - Patch 3 ('block/qapi: Move 'aio' option to file driver')
> Use qapi_enum_parse() instead of parsing manually [Eric]
>
> - Patch 4 ('block: Parse 'detect-zeroes' in bdrv_open_common()')
> Fixed typo in commit message [Eric]
> Reuse local detect_zeroes variable instead of querying QemuOpts [Eric]
>
> Kevin Wolf (7):
> block: Drop aio/cache consistency check from qmp_blockdev_add()
> block/qapi: Use separate options type for curl driver
> block/qapi: Move 'aio' option to file driver
> block: Parse 'detect-zeroes' in bdrv_open_common()
> block: Use 'detect-zeroes' option for 'blockdev-change-medium'
> block: Move 'discard' option to bdrv_open_common()
> block: Remove qemu_root_bds_opts
>
> block.c | 50 ++++++++++++++++++-
> block/block-backend.c | 9 ++--
> block/raw-posix.c | 44 ++++++++++-------
> block/raw-win32.c | 56 +++++++++++++++++++--
> blockdev.c | 110 +++--------------------------------------
> include/block/block.h | 1 +
> include/sysemu/block-backend.h | 2 +-
> qapi/block-core.json | 31 ++++++++----
> tests/qemu-iotests/087 | 4 +-
> tests/qemu-iotests/087.out | 2 +-
> 10 files changed, 164 insertions(+), 145 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
It's a bit funny now how blockdev_init() and
extract_common_blockdev_options() are actually used by neither
blockdev-add nor -blockdev, but only by drive_new() (which happily
advertises blockdev_init() to be shared with blockev-add, though).
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 480 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/7] block: Make more blockdev-add options work
2016-09-23 14:32 [Qemu-devel] [PATCH v2 0/7] block: Make more blockdev-add options work Kevin Wolf
` (7 preceding siblings ...)
2016-09-26 17:10 ` [Qemu-devel] [PATCH v2 0/7] block: Make more blockdev-add options work Max Reitz
@ 2016-09-27 8:28 ` Kevin Wolf
8 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-27 8:28 UTC (permalink / raw)
To: qemu-block; +Cc: mreitz, eblake, qemu-devel
Am 23.09.2016 um 16:32 hat Kevin Wolf geschrieben:
> This series moves a few more options from flags to the appropriate place. This
> doesn't only result in cleaner code, but also allows using these options in
> nested node definitions.
>
> After this series, bds_tree_init() is only a thin wrapper around bdrv_open()
> which sets the right defaults for cache modes and the BDRV_O_INACTIVE flag if
> necessary.
>
> Depends on my block branch, specifically Berto's 'read-only' patches.
Applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 17+ messages in thread