From 24df38b00e23f76558ac12d7055c2df8d4b05150 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 26 Sep 2016 15:03:00 +0200 Subject: block: Fix error path in qmp_blockdev_change_medium() Commit 00949bab incorrectly changed one instance of &err into errp while touching the line. Change it back. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- blockdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'blockdev.c') diff --git a/blockdev.c b/blockdev.c index 29c6561fd8..62d0dd016f 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2614,7 +2614,7 @@ void qmp_blockdev_change_medium(bool has_device, const char *device, error_free(err); err = NULL; - qmp_x_blockdev_remove_medium(has_device, device, has_id, id, errp); + qmp_x_blockdev_remove_medium(has_device, device, has_id, id, &err); if (err) { error_propagate(errp, err); goto fail; -- cgit v1.2.3-55-g7522 From 0ffcdd9c06343a3fe0b2b3e1ca93ce8aa5366f98 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 7 Sep 2016 15:26:42 +0200 Subject: block: Drop aio/cache consistency check from qmp_blockdev_add() 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 Reviewed-by: Eric Blake Reviewed-by: Max Reitz --- blockdev.c | 15 --------------- tests/qemu-iotests/087.out | 2 +- 2 files changed, 1 insertion(+), 16 deletions(-) (limited to 'blockdev.c') diff --git a/blockdev.c b/blockdev.c index 62d0dd016f..7820f42210 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 bef68626c8..cd02eaed4c 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"} -- cgit v1.2.3-55-g7522 From 692e01a27ccde4120f1371227d4011f668d67a76 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 12 Sep 2016 21:00:41 +0200 Subject: block: Parse 'detect-zeroes' in bdrv_open_common() 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 Reviewed-by: Eric Blake Reviewed-by: Max Reitz --- block.c | 33 +++++++++++++++++++++++++++++++++ blockdev.c | 9 +-------- 2 files changed, 34 insertions(+), 8 deletions(-) (limited to 'blockdev.c') diff --git a/block.c b/block.c index 493ecf3137..1f1045738d 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 @@ -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 7820f42210..511260ce93 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 */ } }, -- cgit v1.2.3-55-g7522 From b85114f8cfbede8b153db68875973ef0790bf296 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 12 Sep 2016 19:08:31 +0200 Subject: block: Use 'detect-zeroes' option for 'blockdev-change-medium' 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 Reviewed-by: Eric Blake Reviewed-by: Max Reitz --- block/block-backend.c | 9 ++++----- blockdev.c | 9 ++++++--- include/sysemu/block-backend.h | 2 +- 3 files changed, 11 insertions(+), 9 deletions(-) (limited to 'blockdev.c') diff --git a/block/block-backend.c b/block/block-backend.c index f34bad5840..639294b8e6 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 511260ce93..7b87bd8a71 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 24d1d85399..a7993afcda 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -198,7 +198,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, -- cgit v1.2.3-55-g7522 From 818584a43ab0ef52c131865128ef110f867726cd Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 12 Sep 2016 18:03:18 +0200 Subject: block: Move 'discard' option to bdrv_open_common() 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 Reviewed-by: Eric Blake Reviewed-by: Max Reitz --- block.c | 17 ++++++++++++++++- blockdev.c | 25 ------------------------- include/block/block.h | 1 + 3 files changed, 17 insertions(+), 26 deletions(-) (limited to 'blockdev.c') diff --git a/block.c b/block.c index 1f1045738d..bb1f1ec957 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 7b87bd8a71..e2ace04346 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; - } } } @@ -3989,10 +3972,6 @@ QemuOptsList qemu_common_drive_opts = { .name = "snapshot", .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, @@ -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 811b060f41..107c603605 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 -- cgit v1.2.3-55-g7522 From 74e1ae7c0b66d85d2b04c153ffdf6294e6a40798 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 20 Sep 2016 20:48:52 +0200 Subject: block: Remove qemu_root_bds_opts 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 Reviewed-by: Eric Blake Reviewed-by: Max Reitz --- blockdev.c | 54 +----------------------------------------------------- 1 file changed, 1 insertion(+), 53 deletions(-) (limited to 'blockdev.c') diff --git a/blockdev.c b/blockdev.c index e2ace04346..07ec733905 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), -- cgit v1.2.3-55-g7522