From f2cd875d54e95f87a4c994cfef082f09b4953aa9 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 21 Mar 2016 11:27:04 +0100 Subject: block: Introduce BlockBackendPublic Some features, like I/O throttling, are implemented outside block-backend.c, but still want to keep information in BlockBackend, e.g. list entries that allow keeping a list of BlockBackends. In order to avoid exposing the whole struct layout in the public header file, this patch introduces an embedded public struct where such information can be added and a pair of functions to convert between BlockBackend and BlockBackendPublic. Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Reviewed-by: Eric Blake Acked-by: Stefan Hajnoczi --- include/sysemu/block-backend.h | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'include') diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 26736ed84e..a771603bd0 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -59,6 +59,13 @@ typedef struct BlockDevOps { void (*resize_cb)(void *opaque); } BlockDevOps; +/* This struct is embedded in (the private) BlockBackend struct and contains + * fields that must be public. This is in particular for QLIST_ENTRY() and + * friends so that BlockBackends can be kept in lists outside block-backend.c */ +typedef struct BlockBackendPublic { + int dummy; /* empty structs are illegal */ +} BlockBackendPublic; + BlockBackend *blk_new(Error **errp); BlockBackend *blk_new_with_bs(Error **errp); BlockBackend *blk_new_open(const char *filename, const char *reference, @@ -74,6 +81,9 @@ BlockDriverState *blk_next_root_bs(BlockDriverState *bs); bool monitor_add_blk(BlockBackend *blk, const char *name, Error **errp); void monitor_remove_blk(BlockBackend *blk); +BlockBackendPublic *blk_get_public(BlockBackend *blk); +BlockBackend *blk_by_public(BlockBackendPublic *public); + BlockDriverState *blk_bs(BlockBackend *blk); void blk_remove_bs(BlockBackend *blk); void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs); -- cgit v1.2.3-55-g7522 From 31dce3ccca98bc9f9eb57f8b08b008edd07661ba Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 21 Mar 2016 11:30:57 +0100 Subject: block: throttle-groups: Use BlockBackend pointers internally As a first step towards moving I/O throttling to the BlockBackend level, this patch changes all pointers in struct ThrottleGroup from referencing a BlockDriverState to referencing a BlockBackend. This change is valid because we made sure that throttling can only be enabled on BDSes which have a BB attached. Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Acked-by: Stefan Hajnoczi --- block/io.c | 4 +- block/throttle-groups.c | 134 ++++++++++++++++++++-------------------- include/block/block_int.h | 1 - include/block/throttle-groups.h | 4 +- include/sysemu/block-backend.h | 5 +- tests/test-throttle.c | 13 ++-- 6 files changed, 83 insertions(+), 78 deletions(-) (limited to 'include') diff --git a/block/io.c b/block/io.c index cd6d71a503..ede74c5c03 100644 --- a/block/io.c +++ b/block/io.c @@ -70,7 +70,7 @@ void bdrv_io_limits_disable(BlockDriverState *bs) { assert(bs->throttle_state); bdrv_no_throttling_begin(bs); - throttle_group_unregister_bs(bs); + throttle_group_unregister_blk(bs->blk); bdrv_no_throttling_end(bs); } @@ -78,7 +78,7 @@ void bdrv_io_limits_disable(BlockDriverState *bs) void bdrv_io_limits_enable(BlockDriverState *bs, const char *group) { assert(!bs->throttle_state); - throttle_group_register_bs(bs, group); + throttle_group_register_blk(bs->blk, group); } void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group) diff --git a/block/throttle-groups.c b/block/throttle-groups.c index 9ac063a0cd..1e6e2e5f4f 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -23,6 +23,7 @@ */ #include "qemu/osdep.h" +#include "sysemu/block-backend.h" #include "block/throttle-groups.h" #include "qemu/queue.h" #include "qemu/thread.h" @@ -57,8 +58,8 @@ typedef struct ThrottleGroup { QemuMutex lock; /* This lock protects the following four fields */ ThrottleState ts; - QLIST_HEAD(, BlockDriverState) head; - BlockDriverState *tokens[2]; + QLIST_HEAD(, BlockBackendPublic) head; + BlockBackend *tokens[2]; bool any_timer_armed[2]; /* These two are protected by the global throttle_groups_lock */ @@ -145,81 +146,81 @@ const char *throttle_group_get_name(BlockDriverState *bs) return tg->name; } -/* Return the next BlockDriverState in the round-robin sequence, - * simulating a circular list. +/* Return the next BlockBackend in the round-robin sequence, simulating a + * circular list. * * This assumes that tg->lock is held. * - * @bs: the current BlockDriverState - * @ret: the next BlockDriverState in the sequence + * @blk: the current BlockBackend + * @ret: the next BlockBackend in the sequence */ -static BlockDriverState *throttle_group_next_bs(BlockDriverState *bs) +static BlockBackend *throttle_group_next_blk(BlockBackend *blk) { + BlockDriverState *bs = blk_bs(blk); ThrottleState *ts = bs->throttle_state; ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); - BlockDriverState *next = QLIST_NEXT(bs, round_robin); + BlockBackendPublic *next = QLIST_NEXT(blk_get_public(blk), round_robin); if (!next) { - return QLIST_FIRST(&tg->head); + next = QLIST_FIRST(&tg->head); } - return next; + return blk_by_public(next); } -/* Return the next BlockDriverState in the round-robin sequence with - * pending I/O requests. +/* Return the next BlockBackend in the round-robin sequence with pending I/O + * requests. * * This assumes that tg->lock is held. * - * @bs: the current BlockDriverState + * @blk: the current BlockBackend * @is_write: the type of operation (read/write) - * @ret: the next BlockDriverState with pending requests, or bs - * if there is none. + * @ret: the next BlockBackend with pending requests, or blk if there is + * none. */ -static BlockDriverState *next_throttle_token(BlockDriverState *bs, - bool is_write) +static BlockBackend *next_throttle_token(BlockBackend *blk, bool is_write) { - ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts); - BlockDriverState *token, *start; + ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state, + ThrottleGroup, ts); + BlockBackend *token, *start; start = token = tg->tokens[is_write]; /* get next bs round in round robin style */ - token = throttle_group_next_bs(token); - while (token != start && !token->pending_reqs[is_write]) { - token = throttle_group_next_bs(token); + token = throttle_group_next_blk(token); + while (token != start && !blk_bs(token)->pending_reqs[is_write]) { + token = throttle_group_next_blk(token); } /* If no IO are queued for scheduling on the next round robin token * then decide the token is the current bs because chances are * the current bs get the current request queued. */ - if (token == start && !token->pending_reqs[is_write]) { - token = bs; + if (token == start && !blk_bs(token)->pending_reqs[is_write]) { + token = blk; } return token; } -/* Check if the next I/O request for a BlockDriverState needs to be - * throttled or not. If there's no timer set in this group, set one - * and update the token accordingly. +/* Check if the next I/O request for a BlockBackend needs to be throttled or + * not. If there's no timer set in this group, set one and update the token + * accordingly. * * This assumes that tg->lock is held. * - * @bs: the current BlockDriverState + * @blk: the current BlockBackend * @is_write: the type of operation (read/write) * @ret: whether the I/O request needs to be throttled or not */ -static bool throttle_group_schedule_timer(BlockDriverState *bs, - bool is_write) +static bool throttle_group_schedule_timer(BlockBackend *blk, bool is_write) { - ThrottleState *ts = bs->throttle_state; - ThrottleTimers *tt = &bs->throttle_timers; + ThrottleState *ts = blk_bs(blk)->throttle_state; + ThrottleTimers *tt = &blk_bs(blk)->throttle_timers; ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); bool must_wait; - if (bs->io_limits_disabled) { + if (blk_bs(blk)->io_limits_disabled) { return false; } @@ -230,9 +231,9 @@ static bool throttle_group_schedule_timer(BlockDriverState *bs, must_wait = throttle_schedule_timer(ts, tt, is_write); - /* If a timer just got armed, set bs as the current token */ + /* If a timer just got armed, set blk as the current token */ if (must_wait) { - tg->tokens[is_write] = bs; + tg->tokens[is_write] = blk; tg->any_timer_armed[is_write] = true; } @@ -243,18 +244,19 @@ static bool throttle_group_schedule_timer(BlockDriverState *bs, * * This assumes that tg->lock is held. * - * @bs: the current BlockDriverState + * @blk: the current BlockBackend * @is_write: the type of operation (read/write) */ -static void schedule_next_request(BlockDriverState *bs, bool is_write) +static void schedule_next_request(BlockBackend *blk, bool is_write) { + BlockDriverState *bs = blk_bs(blk); ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts); bool must_wait; - BlockDriverState *token; + BlockBackend *token; /* Check if there's any pending request to schedule next */ - token = next_throttle_token(bs, is_write); - if (!token->pending_reqs[is_write]) { + token = next_throttle_token(blk, is_write); + if (!blk_bs(token)->pending_reqs[is_write]) { return; } @@ -266,9 +268,9 @@ static void schedule_next_request(BlockDriverState *bs, bool is_write) /* Give preference to requests from the current bs */ if (qemu_in_coroutine() && qemu_co_queue_next(&bs->throttled_reqs[is_write])) { - token = bs; + token = blk; } else { - ThrottleTimers *tt = &token->throttle_timers; + ThrottleTimers *tt = &blk_bs(token)->throttle_timers; int64_t now = qemu_clock_get_ns(tt->clock_type); timer_mod(tt->timers[is_write], now + 1); tg->any_timer_armed[is_write] = true; @@ -290,13 +292,13 @@ void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs, bool is_write) { bool must_wait; - BlockDriverState *token; + BlockBackend *token; ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts); qemu_mutex_lock(&tg->lock); /* First we check if this I/O has to be throttled. */ - token = next_throttle_token(bs, is_write); + token = next_throttle_token(bs->blk, is_write); must_wait = throttle_group_schedule_timer(token, is_write); /* Wait if there's a timer set or queued requests of this type */ @@ -312,7 +314,7 @@ void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs, throttle_account(bs->throttle_state, is_write, bytes); /* Schedule the next request */ - schedule_next_request(bs, is_write); + schedule_next_request(bs->blk, is_write); qemu_mutex_unlock(&tg->lock); } @@ -395,7 +397,7 @@ static void timer_cb(BlockDriverState *bs, bool is_write) * scheduling the next one */ if (empty_queue) { qemu_mutex_lock(&tg->lock); - schedule_next_request(bs, is_write); + schedule_next_request(bs->blk, is_write); qemu_mutex_unlock(&tg->lock); } } @@ -410,17 +412,17 @@ static void write_timer_cb(void *opaque) timer_cb(opaque, true); } -/* Register a BlockDriverState in the throttling group, also - * initializing its timers and updating its throttle_state pointer to - * point to it. If a throttling group with that name does not exist - * yet, it will be created. +/* Register a BlockBackend in the throttling group, also initializing its + * timers and updating its throttle_state pointer to point to it. If a + * throttling group with that name does not exist yet, it will be created. * - * @bs: the BlockDriverState to insert + * @blk: the BlockBackend to insert * @groupname: the name of the group */ -void throttle_group_register_bs(BlockDriverState *bs, const char *groupname) +void throttle_group_register_blk(BlockBackend *blk, const char *groupname) { int i; + BlockDriverState *bs = blk_bs(blk); ThrottleState *ts = throttle_group_incref(groupname); ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); int clock_type = QEMU_CLOCK_REALTIME; @@ -433,14 +435,14 @@ void throttle_group_register_bs(BlockDriverState *bs, const char *groupname) bs->throttle_state = ts; qemu_mutex_lock(&tg->lock); - /* If the ThrottleGroup is new set this BlockDriverState as the token */ + /* If the ThrottleGroup is new set this BlockBackend as the token */ for (i = 0; i < 2; i++) { if (!tg->tokens[i]) { - tg->tokens[i] = bs; + tg->tokens[i] = blk; } } - QLIST_INSERT_HEAD(&tg->head, bs, round_robin); + QLIST_INSERT_HEAD(&tg->head, blk_get_public(blk), round_robin); throttle_timers_init(&bs->throttle_timers, bdrv_get_aio_context(bs), @@ -452,19 +454,19 @@ void throttle_group_register_bs(BlockDriverState *bs, const char *groupname) qemu_mutex_unlock(&tg->lock); } -/* Unregister a BlockDriverState from its group, removing it from the - * list, destroying the timers and setting the throttle_state pointer - * to NULL. +/* Unregister a BlockBackend from its group, removing it from the list, + * destroying the timers and setting the throttle_state pointer to NULL. * - * The BlockDriverState must not have pending throttled requests, so - * the caller has to drain them first. + * The BlockBackend must not have pending throttled requests, so the caller has + * to drain them first. * * The group will be destroyed if it's empty after this operation. * - * @bs: the BlockDriverState to remove + * @blk: the BlockBackend to remove */ -void throttle_group_unregister_bs(BlockDriverState *bs) +void throttle_group_unregister_blk(BlockBackend *blk) { + BlockDriverState *bs = blk_bs(blk); ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts); int i; @@ -474,10 +476,10 @@ void throttle_group_unregister_bs(BlockDriverState *bs) qemu_mutex_lock(&tg->lock); for (i = 0; i < 2; i++) { - if (tg->tokens[i] == bs) { - BlockDriverState *token = throttle_group_next_bs(bs); + if (tg->tokens[i] == blk) { + BlockBackend *token = throttle_group_next_blk(blk); /* Take care of the case where this is the last bs in the group */ - if (token == bs) { + if (token == blk) { token = NULL; } tg->tokens[i] = token; @@ -485,7 +487,7 @@ void throttle_group_unregister_bs(BlockDriverState *bs) } /* remove the current bs from the list */ - QLIST_REMOVE(bs, round_robin); + QLIST_REMOVE(blk_get_public(blk), round_robin); throttle_timers_destroy(&bs->throttle_timers); qemu_mutex_unlock(&tg->lock); diff --git a/include/block/block_int.h b/include/block/block_int.h index a029c2003f..3f5d2b1ccc 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -435,7 +435,6 @@ struct BlockDriverState { ThrottleState *throttle_state; ThrottleTimers throttle_timers; unsigned pending_reqs[2]; - QLIST_ENTRY(BlockDriverState) round_robin; /* Offset after the highest byte written to */ uint64_t wr_highest_offset; diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h index 395f72d444..b9114eedeb 100644 --- a/include/block/throttle-groups.h +++ b/include/block/throttle-groups.h @@ -36,8 +36,8 @@ void throttle_group_unref(ThrottleState *ts); void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg); void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg); -void throttle_group_register_bs(BlockDriverState *bs, const char *groupname); -void throttle_group_unregister_bs(BlockDriverState *bs); +void throttle_group_register_blk(BlockBackend *blk, const char *groupname); +void throttle_group_unregister_blk(BlockBackend *blk); void throttle_group_restart_bs(BlockDriverState *bs); void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs, diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index a771603bd0..1dcd70e2ca 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -63,7 +63,10 @@ typedef struct BlockDevOps { * fields that must be public. This is in particular for QLIST_ENTRY() and * friends so that BlockBackends can be kept in lists outside block-backend.c */ typedef struct BlockBackendPublic { - int dummy; /* empty structs are illegal */ + /* I/O throttling */ + /* The following fields are protected by the ThrottleGroup lock. + * See the ThrottleGroup documentation for details. */ + QLIST_ENTRY(BlockBackendPublic) round_robin; } BlockBackendPublic; BlockBackend *blk_new(Error **errp); diff --git a/tests/test-throttle.c b/tests/test-throttle.c index 77b95d6380..beaa2a3518 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -20,6 +20,7 @@ #include "qemu/throttle.h" #include "qemu/error-report.h" #include "block/throttle-groups.h" +#include "sysemu/block-backend.h" static AioContext *ctx; static LeakyBucket bkt; @@ -589,9 +590,9 @@ static void test_groups(void) g_assert(bdrv2->throttle_state == NULL); g_assert(bdrv3->throttle_state == NULL); - throttle_group_register_bs(bdrv1, "bar"); - throttle_group_register_bs(bdrv2, "foo"); - throttle_group_register_bs(bdrv3, "bar"); + throttle_group_register_blk(blk1, "bar"); + throttle_group_register_blk(blk2, "foo"); + throttle_group_register_blk(blk3, "bar"); g_assert(bdrv1->throttle_state != NULL); g_assert(bdrv2->throttle_state != NULL); @@ -623,9 +624,9 @@ static void test_groups(void) throttle_group_get_config(bdrv3, &cfg2); g_assert(!memcmp(&cfg1, &cfg2, sizeof(cfg1))); - throttle_group_unregister_bs(bdrv1); - throttle_group_unregister_bs(bdrv2); - throttle_group_unregister_bs(bdrv3); + throttle_group_unregister_blk(blk1); + throttle_group_unregister_blk(blk2); + throttle_group_unregister_blk(blk3); g_assert(bdrv1->throttle_state == NULL); g_assert(bdrv2->throttle_state == NULL); -- cgit v1.2.3-55-g7522 From 49d2165d7d6b589d1ea28b15a8874c417bdc55ed Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 21 Mar 2016 11:58:21 +0100 Subject: block: Convert throttle_group_get_name() to BlockBackend Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Acked-by: Stefan Hajnoczi --- block/block-backend.c | 2 +- block/io.c | 2 +- block/qapi.c | 2 +- block/throttle-groups.c | 12 ++++++------ include/block/throttle-groups.h | 2 +- tests/test-throttle.c | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) (limited to 'include') diff --git a/block/block-backend.c b/block/block-backend.c index 2f8acbdbaf..964a205d38 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1525,7 +1525,7 @@ void blk_update_root_state(BlockBackend *blk) throttle_group_unref(blk->root_state.throttle_state); } if (blk->root->bs->throttle_state) { - const char *name = throttle_group_get_name(blk->root->bs); + const char *name = throttle_group_get_name(blk); blk->root_state.throttle_group = g_strdup(name); blk->root_state.throttle_state = throttle_group_incref(name); } else { diff --git a/block/io.c b/block/io.c index ede74c5c03..f6fb86883a 100644 --- a/block/io.c +++ b/block/io.c @@ -89,7 +89,7 @@ void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group) } /* this bs is a part of the same group than the one we want */ - if (!g_strcmp0(throttle_group_get_name(bs), group)) { + if (!g_strcmp0(throttle_group_get_name(bs->blk), group)) { return; } diff --git a/block/qapi.c b/block/qapi.c index c5f6ba643c..a3e514d89b 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -118,7 +118,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk, info->iops_size = cfg.op_size; info->has_group = true; - info->group = g_strdup(throttle_group_get_name(bs)); + info->group = g_strdup(throttle_group_get_name(bs->blk)); } info->write_threshold = bdrv_write_threshold_get(bs); diff --git a/block/throttle-groups.c b/block/throttle-groups.c index 1e6e2e5f4f..e50ccaaf7e 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -133,16 +133,16 @@ void throttle_group_unref(ThrottleState *ts) qemu_mutex_unlock(&throttle_groups_lock); } -/* Get the name from a BlockDriverState's ThrottleGroup. The name (and - * the pointer) is guaranteed to remain constant during the lifetime - * of the group. +/* Get the name from a BlockBackend's ThrottleGroup. The name (and the pointer) + * is guaranteed to remain constant during the lifetime of the group. * - * @bs: a BlockDriverState that is member of a throttling group + * @blk: a BlockBackend that is member of a throttling group * @ret: the name of the group. */ -const char *throttle_group_get_name(BlockDriverState *bs) +const char *throttle_group_get_name(BlockBackend *blk) { - ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts); + ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state, + ThrottleGroup, ts); return tg->name; } diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h index b9114eedeb..bd55a34194 100644 --- a/include/block/throttle-groups.h +++ b/include/block/throttle-groups.h @@ -28,7 +28,7 @@ #include "qemu/throttle.h" #include "block/block_int.h" -const char *throttle_group_get_name(BlockDriverState *bs); +const char *throttle_group_get_name(BlockBackend *blk); ThrottleState *throttle_group_incref(const char *name); void throttle_group_unref(ThrottleState *ts); diff --git a/tests/test-throttle.c b/tests/test-throttle.c index beaa2a3518..1a322f1897 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -598,8 +598,8 @@ static void test_groups(void) g_assert(bdrv2->throttle_state != NULL); g_assert(bdrv3->throttle_state != NULL); - g_assert(!strcmp(throttle_group_get_name(bdrv1), "bar")); - g_assert(!strcmp(throttle_group_get_name(bdrv2), "foo")); + g_assert(!strcmp(throttle_group_get_name(blk1), "bar")); + g_assert(!strcmp(throttle_group_get_name(blk2), "foo")); g_assert(bdrv1->throttle_state == bdrv3->throttle_state); /* Setting the config of a group member affects the whole group */ -- cgit v1.2.3-55-g7522 From 27ccdd52598290f0f8b58be56e235aff7aebfaf3 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 21 Mar 2016 12:56:44 +0100 Subject: block: Move throttling fields from BDS to BB This patch changes where the throttling state is stored (used to be the BlockDriverState, now it is the BlockBackend), but it doesn't actually make it a BB level feature yet. For example, throttling is still disabled when the BDS is detached from the BB. Signed-off-by: Kevin Wolf Acked-by: Stefan Hajnoczi --- block.c | 22 +++---- block/block-backend.c | 13 ++-- block/io.c | 36 +++++++---- block/qapi.c | 2 +- block/throttle-groups.c | 129 +++++++++++++++++++++------------------- blockdev.c | 4 +- include/block/block_int.h | 13 ---- include/block/throttle-groups.h | 2 +- include/sysemu/block-backend.h | 11 +++- tests/test-throttle.c | 28 +++++---- 10 files changed, 142 insertions(+), 118 deletions(-) (limited to 'include') diff --git a/block.c b/block.c index a7898449ac..f723060deb 100644 --- a/block.c +++ b/block.c @@ -237,8 +237,6 @@ BlockDriverState *bdrv_new(void) QLIST_INIT(&bs->op_blockers[i]); } notifier_with_return_list_init(&bs->before_write_notifiers); - qemu_co_queue_init(&bs->throttled_reqs[0]); - qemu_co_queue_init(&bs->throttled_reqs[1]); bs->refcnt = 1; bs->aio_context = qemu_get_aio_context(); @@ -1525,7 +1523,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, return -ENODEV; } - if (bs->throttle_state) { + if (bs->blk && blk_get_public(bs->blk)->throttle_state) { error_setg(errp, "Cannot reference an existing block device for " "which I/O throttling is enabled"); return -EINVAL; @@ -2124,7 +2122,7 @@ static void bdrv_close(BlockDriverState *bs) assert(!bs->job); /* Disable I/O limits and drain all pending throttled requests */ - if (bs->throttle_state) { + if (bs->blk && blk_get_public(bs->blk)->throttle_state) { bdrv_io_limits_disable(bs); } @@ -2257,8 +2255,8 @@ static void swap_feature_fields(BlockDriverState *bs_top, bdrv_move_feature_fields(bs_top, bs_new); bdrv_move_feature_fields(bs_new, &tmp); - assert(!bs_new->throttle_state); - if (bs_top->throttle_state) { + assert(!bs_new->blk); + if (bs_top->blk && blk_get_public(bs_top->blk)->throttle_state) { /* * FIXME Need to break I/O throttling with graph manipulations * temporarily because of conflicting invariants (3. will go away when @@ -2300,11 +2298,11 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top) assert(!bdrv_requests_pending(bs_new)); bdrv_ref(bs_top); - change_parent_backing_link(bs_top, bs_new); /* Some fields always stay on top of the backing file chain */ swap_feature_fields(bs_top, bs_new); + change_parent_backing_link(bs_top, bs_new); bdrv_set_backing_hd(bs_new, bs_top); bdrv_unref(bs_top); @@ -3676,8 +3674,9 @@ void bdrv_detach_aio_context(BlockDriverState *bs) baf->detach_aio_context(baf->opaque); } - if (bs->throttle_state) { - throttle_timers_detach_aio_context(&bs->throttle_timers); + if (bs->blk && blk_get_public(bs->blk)->throttle_state) { + throttle_timers_detach_aio_context( + &blk_get_public(bs->blk)->throttle_timers); } if (bs->drv->bdrv_detach_aio_context) { bs->drv->bdrv_detach_aio_context(bs); @@ -3712,8 +3711,9 @@ void bdrv_attach_aio_context(BlockDriverState *bs, if (bs->drv->bdrv_attach_aio_context) { bs->drv->bdrv_attach_aio_context(bs, new_context); } - if (bs->throttle_state) { - throttle_timers_attach_aio_context(&bs->throttle_timers, new_context); + if (bs->blk && blk_get_public(bs->blk)->throttle_state) { + throttle_timers_attach_aio_context( + &blk_get_public(bs->blk)->throttle_timers, new_context); } QLIST_FOREACH(ban, &bs->aio_notifiers, list) { diff --git a/block/block-backend.c b/block/block-backend.c index 964a205d38..6880659665 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -107,8 +107,12 @@ BlockBackend *blk_new(Error **errp) blk = g_new0(BlockBackend, 1); blk->refcnt = 1; + qemu_co_queue_init(&blk->public.throttled_reqs[0]); + qemu_co_queue_init(&blk->public.throttled_reqs[1]); + notifier_list_init(&blk->remove_bs_notifiers); notifier_list_init(&blk->insert_bs_notifiers); + QTAILQ_INSERT_TAIL(&block_backends, blk, link); return blk; } @@ -437,7 +441,7 @@ void blk_remove_bs(BlockBackend *blk) notifier_list_notify(&blk->remove_bs_notifiers, blk); blk_update_root_state(blk); - if (blk->root->bs->throttle_state) { + if (blk->public.throttle_state) { bdrv_io_limits_disable(blk->root->bs); } @@ -795,7 +799,6 @@ static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf, int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf, int count) { - BlockDriverState *bs = blk_bs(blk); int ret; ret = blk_check_byte_request(blk, offset, count); @@ -803,9 +806,9 @@ int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf, return ret; } - bdrv_no_throttling_begin(bs); + bdrv_no_throttling_begin(blk_bs(blk)); ret = blk_pread(blk, offset, buf, count); - bdrv_no_throttling_end(bs); + bdrv_no_throttling_end(blk_bs(blk)); return ret; } @@ -1524,7 +1527,7 @@ void blk_update_root_state(BlockBackend *blk) g_free(blk->root_state.throttle_group); throttle_group_unref(blk->root_state.throttle_state); } - if (blk->root->bs->throttle_state) { + if (blk->public.throttle_state) { const char *name = throttle_group_get_name(blk); blk->root_state.throttle_group = g_strdup(name); blk->root_state.throttle_state = throttle_group_incref(name); diff --git a/block/io.c b/block/io.c index f6fb86883a..bdbaa1c0da 100644 --- a/block/io.c +++ b/block/io.c @@ -55,20 +55,31 @@ void bdrv_set_io_limits(BlockDriverState *bs, void bdrv_no_throttling_begin(BlockDriverState *bs) { - if (bs->io_limits_disabled++ == 0) { - throttle_group_restart_bs(bs); + if (!bs->blk) { + return; + } + + if (blk_get_public(bs->blk)->io_limits_disabled++ == 0) { + throttle_group_restart_blk(bs->blk); } } void bdrv_no_throttling_end(BlockDriverState *bs) { - assert(bs->io_limits_disabled); - --bs->io_limits_disabled; + BlockBackendPublic *blkp; + + if (!bs->blk) { + return; + } + + blkp = blk_get_public(bs->blk); + assert(blkp->io_limits_disabled); + --blkp->io_limits_disabled; } void bdrv_io_limits_disable(BlockDriverState *bs) { - assert(bs->throttle_state); + assert(blk_get_public(bs->blk)->throttle_state); bdrv_no_throttling_begin(bs); throttle_group_unregister_blk(bs->blk); bdrv_no_throttling_end(bs); @@ -77,14 +88,16 @@ void bdrv_io_limits_disable(BlockDriverState *bs) /* should be called before bdrv_set_io_limits if a limit is set */ void bdrv_io_limits_enable(BlockDriverState *bs, const char *group) { - assert(!bs->throttle_state); + BlockBackendPublic *blkp = blk_get_public(bs->blk); + + assert(!blkp->throttle_state); throttle_group_register_blk(bs->blk, group); } void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group) { /* this bs is not part of any group */ - if (!bs->throttle_state) { + if (!blk_get_public(bs->blk)->throttle_state) { return; } @@ -178,14 +191,15 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs) bool bdrv_requests_pending(BlockDriverState *bs) { BdrvChild *child; + BlockBackendPublic *blkp = bs->blk ? blk_get_public(bs->blk) : NULL; if (!QLIST_EMPTY(&bs->tracked_requests)) { return true; } - if (!qemu_co_queue_empty(&bs->throttled_reqs[0])) { + if (blkp && !qemu_co_queue_empty(&blkp->throttled_reqs[0])) { return true; } - if (!qemu_co_queue_empty(&bs->throttled_reqs[1])) { + if (blkp && !qemu_co_queue_empty(&blkp->throttled_reqs[1])) { return true; } @@ -1070,7 +1084,7 @@ int coroutine_fn bdrv_co_preadv(BlockDriverState *bs, } /* throttling disk I/O */ - if (bs->throttle_state) { + if (bs->blk && blk_get_public(bs->blk)->throttle_state) { throttle_group_co_io_limits_intercept(bs, bytes, false); } @@ -1431,7 +1445,7 @@ int coroutine_fn bdrv_co_pwritev(BlockDriverState *bs, } /* throttling disk I/O */ - if (bs->throttle_state) { + if (bs->blk && blk_get_public(bs->blk)->throttle_state) { throttle_group_co_io_limits_intercept(bs, bytes, true); } diff --git a/block/qapi.c b/block/qapi.c index a3e514d89b..1e4bb8a0a5 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -67,7 +67,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk, info->backing_file_depth = bdrv_get_backing_file_depth(bs); info->detect_zeroes = bs->detect_zeroes; - if (bs->throttle_state) { + if (bs->blk && blk_get_public(bs->blk)->throttle_state) { ThrottleConfig cfg; throttle_group_get_config(bs, &cfg); diff --git a/block/throttle-groups.c b/block/throttle-groups.c index e50ccaaf7e..56dc311867 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -30,7 +30,7 @@ #include "sysemu/qtest.h" /* The ThrottleGroup structure (with its ThrottleState) is shared - * among different BlockDriverState and it's independent from + * among different BlockBackends and it's independent from * AioContext, so in order to use it from different threads it needs * its own locking. * @@ -40,18 +40,18 @@ * The whole ThrottleGroup structure is private and invisible to * outside users, that only use it through its ThrottleState. * - * In addition to the ThrottleGroup structure, BlockDriverState has + * In addition to the ThrottleGroup structure, BlockBackendPublic has * fields that need to be accessed by other members of the group and - * therefore also need to be protected by this lock. Once a BDS is - * registered in a group those fields can be accessed by other threads - * any time. + * therefore also need to be protected by this lock. Once a + * BlockBackend is registered in a group those fields can be accessed + * by other threads any time. * * Again, all this is handled internally and is mostly transparent to * the outside. The 'throttle_timers' field however has an additional * constraint because it may be temporarily invalid (see for example * bdrv_set_aio_context()). Therefore in this file a thread will - * access some other BDS's timers only after verifying that that BDS - * has throttled requests in the queue. + * access some other BlockBackend's timers only after verifying that + * that BlockBackend has throttled requests in the queue. */ typedef struct ThrottleGroup { char *name; /* This is constant during the lifetime of the group */ @@ -141,8 +141,8 @@ void throttle_group_unref(ThrottleState *ts) */ const char *throttle_group_get_name(BlockBackend *blk) { - ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state, - ThrottleGroup, ts); + BlockBackendPublic *blkp = blk_get_public(blk); + ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts); return tg->name; } @@ -156,10 +156,10 @@ const char *throttle_group_get_name(BlockBackend *blk) */ static BlockBackend *throttle_group_next_blk(BlockBackend *blk) { - BlockDriverState *bs = blk_bs(blk); - ThrottleState *ts = bs->throttle_state; + BlockBackendPublic *blkp = blk_get_public(blk); + ThrottleState *ts = blkp->throttle_state; ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); - BlockBackendPublic *next = QLIST_NEXT(blk_get_public(blk), round_robin); + BlockBackendPublic *next = QLIST_NEXT(blkp, round_robin); if (!next) { next = QLIST_FIRST(&tg->head); @@ -180,15 +180,15 @@ static BlockBackend *throttle_group_next_blk(BlockBackend *blk) */ static BlockBackend *next_throttle_token(BlockBackend *blk, bool is_write) { - ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state, - ThrottleGroup, ts); + BlockBackendPublic *blkp = blk_get_public(blk); + ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts); BlockBackend *token, *start; start = token = tg->tokens[is_write]; /* get next bs round in round robin style */ token = throttle_group_next_blk(token); - while (token != start && !blk_bs(token)->pending_reqs[is_write]) { + while (token != start && !blkp->pending_reqs[is_write]) { token = throttle_group_next_blk(token); } @@ -196,7 +196,7 @@ static BlockBackend *next_throttle_token(BlockBackend *blk, bool is_write) * then decide the token is the current bs because chances are * the current bs get the current request queued. */ - if (token == start && !blk_bs(token)->pending_reqs[is_write]) { + if (token == start && !blkp->pending_reqs[is_write]) { token = blk; } @@ -215,12 +215,13 @@ static BlockBackend *next_throttle_token(BlockBackend *blk, bool is_write) */ static bool throttle_group_schedule_timer(BlockBackend *blk, bool is_write) { - ThrottleState *ts = blk_bs(blk)->throttle_state; - ThrottleTimers *tt = &blk_bs(blk)->throttle_timers; + BlockBackendPublic *blkp = blk_get_public(blk); + ThrottleState *ts = blkp->throttle_state; + ThrottleTimers *tt = &blkp->throttle_timers; ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); bool must_wait; - if (blk_bs(blk)->io_limits_disabled) { + if (blkp->io_limits_disabled) { return false; } @@ -249,14 +250,14 @@ static bool throttle_group_schedule_timer(BlockBackend *blk, bool is_write) */ static void schedule_next_request(BlockBackend *blk, bool is_write) { - BlockDriverState *bs = blk_bs(blk); - ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts); + BlockBackendPublic *blkp = blk_get_public(blk); + ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts); bool must_wait; BlockBackend *token; /* Check if there's any pending request to schedule next */ token = next_throttle_token(blk, is_write); - if (!blk_bs(token)->pending_reqs[is_write]) { + if (!blkp->pending_reqs[is_write]) { return; } @@ -265,12 +266,12 @@ static void schedule_next_request(BlockBackend *blk, bool is_write) /* If it doesn't have to wait, queue it for immediate execution */ if (!must_wait) { - /* Give preference to requests from the current bs */ + /* Give preference to requests from the current blk */ if (qemu_in_coroutine() && - qemu_co_queue_next(&bs->throttled_reqs[is_write])) { + qemu_co_queue_next(&blkp->throttled_reqs[is_write])) { token = blk; } else { - ThrottleTimers *tt = &blk_bs(token)->throttle_timers; + ThrottleTimers *tt = &blkp->throttle_timers; int64_t now = qemu_clock_get_ns(tt->clock_type); timer_mod(tt->timers[is_write], now + 1); tg->any_timer_armed[is_write] = true; @@ -294,37 +295,40 @@ void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs, bool must_wait; BlockBackend *token; - ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts); + BlockBackend *blk = bs->blk; + BlockBackendPublic *blkp = blk_get_public(blk); + ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts); qemu_mutex_lock(&tg->lock); /* First we check if this I/O has to be throttled. */ - token = next_throttle_token(bs->blk, is_write); + token = next_throttle_token(blk, is_write); must_wait = throttle_group_schedule_timer(token, is_write); /* Wait if there's a timer set or queued requests of this type */ - if (must_wait || bs->pending_reqs[is_write]) { - bs->pending_reqs[is_write]++; + if (must_wait || blkp->pending_reqs[is_write]) { + blkp->pending_reqs[is_write]++; qemu_mutex_unlock(&tg->lock); - qemu_co_queue_wait(&bs->throttled_reqs[is_write]); + qemu_co_queue_wait(&blkp->throttled_reqs[is_write]); qemu_mutex_lock(&tg->lock); - bs->pending_reqs[is_write]--; + blkp->pending_reqs[is_write]--; } /* The I/O will be executed, so do the accounting */ - throttle_account(bs->throttle_state, is_write, bytes); + throttle_account(blkp->throttle_state, is_write, bytes); /* Schedule the next request */ - schedule_next_request(bs->blk, is_write); + schedule_next_request(blk, is_write); qemu_mutex_unlock(&tg->lock); } -void throttle_group_restart_bs(BlockDriverState *bs) +void throttle_group_restart_blk(BlockBackend *blk) { + BlockBackendPublic *blkp = blk_get_public(blk); int i; for (i = 0; i < 2; i++) { - while (qemu_co_enter_next(&bs->throttled_reqs[i])) { + while (qemu_co_enter_next(&blkp->throttled_reqs[i])) { ; } } @@ -339,8 +343,9 @@ void throttle_group_restart_bs(BlockDriverState *bs) */ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg) { - ThrottleTimers *tt = &bs->throttle_timers; - ThrottleState *ts = bs->throttle_state; + BlockBackendPublic *blkp = blk_get_public(bs->blk); + ThrottleTimers *tt = &blkp->throttle_timers; + ThrottleState *ts = blkp->throttle_state; ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); qemu_mutex_lock(&tg->lock); /* throttle_config() cancels the timers */ @@ -353,8 +358,8 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg) throttle_config(ts, tt, cfg); qemu_mutex_unlock(&tg->lock); - qemu_co_enter_next(&bs->throttled_reqs[0]); - qemu_co_enter_next(&bs->throttled_reqs[1]); + qemu_co_enter_next(&blkp->throttled_reqs[0]); + qemu_co_enter_next(&blkp->throttled_reqs[1]); } /* Get the throttle configuration from a particular group. Similar to @@ -366,7 +371,8 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg) */ void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg) { - ThrottleState *ts = bs->throttle_state; + BlockBackendPublic *blkp = blk_get_public(bs->blk); + ThrottleState *ts = blkp->throttle_state; ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); qemu_mutex_lock(&tg->lock); throttle_get_config(ts, cfg); @@ -376,12 +382,13 @@ void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg) /* ThrottleTimers callback. This wakes up a request that was waiting * because it had been throttled. * - * @bs: the BlockDriverState whose request had been throttled + * @blk: the BlockBackend whose request had been throttled * @is_write: the type of operation (read/write) */ -static void timer_cb(BlockDriverState *bs, bool is_write) +static void timer_cb(BlockBackend *blk, bool is_write) { - ThrottleState *ts = bs->throttle_state; + BlockBackendPublic *blkp = blk_get_public(blk); + ThrottleState *ts = blkp->throttle_state; ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); bool empty_queue; @@ -391,13 +398,13 @@ static void timer_cb(BlockDriverState *bs, bool is_write) qemu_mutex_unlock(&tg->lock); /* Run the request that was waiting for this timer */ - empty_queue = !qemu_co_enter_next(&bs->throttled_reqs[is_write]); + empty_queue = !qemu_co_enter_next(&blkp->throttled_reqs[is_write]); /* If the request queue was empty then we have to take care of * scheduling the next one */ if (empty_queue) { qemu_mutex_lock(&tg->lock); - schedule_next_request(bs->blk, is_write); + schedule_next_request(blk, is_write); qemu_mutex_unlock(&tg->lock); } } @@ -422,7 +429,7 @@ static void write_timer_cb(void *opaque) void throttle_group_register_blk(BlockBackend *blk, const char *groupname) { int i; - BlockDriverState *bs = blk_bs(blk); + BlockBackendPublic *blkp = blk_get_public(blk); ThrottleState *ts = throttle_group_incref(groupname); ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); int clock_type = QEMU_CLOCK_REALTIME; @@ -432,7 +439,7 @@ void throttle_group_register_blk(BlockBackend *blk, const char *groupname) clock_type = QEMU_CLOCK_VIRTUAL; } - bs->throttle_state = ts; + blkp->throttle_state = ts; qemu_mutex_lock(&tg->lock); /* If the ThrottleGroup is new set this BlockBackend as the token */ @@ -442,14 +449,14 @@ void throttle_group_register_blk(BlockBackend *blk, const char *groupname) } } - QLIST_INSERT_HEAD(&tg->head, blk_get_public(blk), round_robin); + QLIST_INSERT_HEAD(&tg->head, blkp, round_robin); - throttle_timers_init(&bs->throttle_timers, - bdrv_get_aio_context(bs), + throttle_timers_init(&blkp->throttle_timers, + blk_get_aio_context(blk), clock_type, read_timer_cb, write_timer_cb, - bs); + blk); qemu_mutex_unlock(&tg->lock); } @@ -466,19 +473,19 @@ void throttle_group_register_blk(BlockBackend *blk, const char *groupname) */ void throttle_group_unregister_blk(BlockBackend *blk) { - BlockDriverState *bs = blk_bs(blk); - ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts); + BlockBackendPublic *blkp = blk_get_public(blk); + ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts); int i; - assert(bs->pending_reqs[0] == 0 && bs->pending_reqs[1] == 0); - assert(qemu_co_queue_empty(&bs->throttled_reqs[0])); - assert(qemu_co_queue_empty(&bs->throttled_reqs[1])); + assert(blkp->pending_reqs[0] == 0 && blkp->pending_reqs[1] == 0); + assert(qemu_co_queue_empty(&blkp->throttled_reqs[0])); + assert(qemu_co_queue_empty(&blkp->throttled_reqs[1])); qemu_mutex_lock(&tg->lock); for (i = 0; i < 2; i++) { if (tg->tokens[i] == blk) { BlockBackend *token = throttle_group_next_blk(blk); - /* Take care of the case where this is the last bs in the group */ + /* Take care of the case where this is the last blk in the group */ if (token == blk) { token = NULL; } @@ -486,13 +493,13 @@ void throttle_group_unregister_blk(BlockBackend *blk) } } - /* remove the current bs from the list */ - QLIST_REMOVE(blk_get_public(blk), round_robin); - throttle_timers_destroy(&bs->throttle_timers); + /* remove the current blk from the list */ + QLIST_REMOVE(blkp, round_robin); + throttle_timers_destroy(&blkp->throttle_timers); qemu_mutex_unlock(&tg->lock); throttle_group_unref(&tg->ts); - bs->throttle_state = NULL; + blkp->throttle_state = NULL; } static void throttle_groups_init(void) diff --git a/blockdev.c b/blockdev.c index 8106ca7654..3211a40615 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2726,14 +2726,14 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd, if (throttle_enabled(&cfg)) { /* Enable I/O limits if they're not enabled yet, otherwise * just update the throttling group. */ - if (!bs->throttle_state) { + if (!blk_get_public(bs->blk)->throttle_state) { bdrv_io_limits_enable(bs, has_group ? group : device); } else if (has_group) { bdrv_io_limits_update_group(bs, group); } /* Set the new throttling configuration */ bdrv_set_io_limits(bs, &cfg); - } else if (bs->throttle_state) { + } else if (blk_get_public(bs->blk)->throttle_state) { /* If all throttling settings are set to 0, disable I/O limits */ bdrv_io_limits_disable(bs); } diff --git a/include/block/block_int.h b/include/block/block_int.h index 3f5d2b1ccc..2bbc2c0609 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -26,7 +26,6 @@ #include "block/accounting.h" #include "block/block.h" -#include "block/throttle-groups.h" #include "qemu/option.h" #include "qemu/queue.h" #include "qemu/coroutine.h" @@ -424,18 +423,6 @@ struct BlockDriverState { /* number of in-flight serialising requests */ unsigned int serialising_in_flight; - /* I/O throttling. - * throttle_state tells us if this BDS has I/O limits configured. - * io_limits_disabled tells us if they are currently being enforced */ - CoQueue throttled_reqs[2]; - unsigned int io_limits_disabled; - - /* The following fields are protected by the ThrottleGroup lock. - * See the ThrottleGroup documentation for details. */ - ThrottleState *throttle_state; - ThrottleTimers throttle_timers; - unsigned pending_reqs[2]; - /* Offset after the highest byte written to */ uint64_t wr_highest_offset; diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h index bd55a34194..840ba443ae 100644 --- a/include/block/throttle-groups.h +++ b/include/block/throttle-groups.h @@ -38,7 +38,7 @@ void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg); void throttle_group_register_blk(BlockBackend *blk, const char *groupname); void throttle_group_unregister_blk(BlockBackend *blk); -void throttle_group_restart_bs(BlockDriverState *bs); +void throttle_group_restart_blk(BlockBackend *blk); void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs, unsigned int bytes, diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 1dcd70e2ca..08d27a86be 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -14,6 +14,7 @@ #define BLOCK_BACKEND_H #include "qemu/iov.h" +#include "block/throttle-groups.h" /* * TODO Have to include block/block.h for a bunch of block layer @@ -63,9 +64,17 @@ typedef struct BlockDevOps { * fields that must be public. This is in particular for QLIST_ENTRY() and * friends so that BlockBackends can be kept in lists outside block-backend.c */ typedef struct BlockBackendPublic { - /* I/O throttling */ + /* I/O throttling. + * throttle_state tells us if this BlockBackend has I/O limits configured. + * io_limits_disabled tells us if they are currently being enforced */ + CoQueue throttled_reqs[2]; + unsigned int io_limits_disabled; + /* The following fields are protected by the ThrottleGroup lock. * See the ThrottleGroup documentation for details. */ + ThrottleState *throttle_state; + ThrottleTimers throttle_timers; + unsigned pending_reqs[2]; QLIST_ENTRY(BlockBackendPublic) round_robin; } BlockBackendPublic; diff --git a/tests/test-throttle.c b/tests/test-throttle.c index 1a322f1897..a0200686d6 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -576,31 +576,35 @@ static void test_groups(void) { ThrottleConfig cfg1, cfg2; BlockBackend *blk1, *blk2, *blk3; - BlockDriverState *bdrv1, *bdrv2, *bdrv3; + BlockBackendPublic *blkp1, *blkp2, *blkp3; + BlockDriverState *bdrv1, *bdrv3; blk1 = blk_new_with_bs(&error_abort); blk2 = blk_new_with_bs(&error_abort); blk3 = blk_new_with_bs(&error_abort); bdrv1 = blk_bs(blk1); - bdrv2 = blk_bs(blk2); bdrv3 = blk_bs(blk3); - g_assert(bdrv1->throttle_state == NULL); - g_assert(bdrv2->throttle_state == NULL); - g_assert(bdrv3->throttle_state == NULL); + blkp1 = blk_get_public(blk1); + blkp2 = blk_get_public(blk2); + blkp3 = blk_get_public(blk3); + + g_assert(blkp1->throttle_state == NULL); + g_assert(blkp2->throttle_state == NULL); + g_assert(blkp3->throttle_state == NULL); throttle_group_register_blk(blk1, "bar"); throttle_group_register_blk(blk2, "foo"); throttle_group_register_blk(blk3, "bar"); - g_assert(bdrv1->throttle_state != NULL); - g_assert(bdrv2->throttle_state != NULL); - g_assert(bdrv3->throttle_state != NULL); + g_assert(blkp1->throttle_state != NULL); + g_assert(blkp2->throttle_state != NULL); + g_assert(blkp3->throttle_state != NULL); g_assert(!strcmp(throttle_group_get_name(blk1), "bar")); g_assert(!strcmp(throttle_group_get_name(blk2), "foo")); - g_assert(bdrv1->throttle_state == bdrv3->throttle_state); + g_assert(blkp1->throttle_state == blkp3->throttle_state); /* Setting the config of a group member affects the whole group */ throttle_config_init(&cfg1); @@ -628,9 +632,9 @@ static void test_groups(void) throttle_group_unregister_blk(blk2); throttle_group_unregister_blk(blk3); - g_assert(bdrv1->throttle_state == NULL); - g_assert(bdrv2->throttle_state == NULL); - g_assert(bdrv3->throttle_state == NULL); + g_assert(blkp1->throttle_state == NULL); + g_assert(blkp2->throttle_state == NULL); + g_assert(blkp3->throttle_state == NULL); } int main(int argc, char **argv) -- cgit v1.2.3-55-g7522 From 441565b2792d4ee9ee1928a8d14538be39211292 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 21 Mar 2016 13:30:30 +0100 Subject: block: Move actual I/O throttling to BlockBackend Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Acked-by: Stefan Hajnoczi --- block/block-backend.c | 10 ++++++++++ block/io.c | 10 ---------- block/throttle-groups.c | 5 ++--- include/block/throttle-groups.h | 2 +- 4 files changed, 13 insertions(+), 14 deletions(-) (limited to 'include') diff --git a/block/block-backend.c b/block/block-backend.c index 6880659665..730b8a9949 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -716,6 +716,11 @@ static int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset, return ret; } + /* throttling disk I/O */ + if (blk->public.throttle_state) { + throttle_group_co_io_limits_intercept(blk, bytes, false); + } + return bdrv_co_preadv(blk_bs(blk), offset, bytes, qiov, flags); } @@ -730,6 +735,11 @@ static int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset, return ret; } + /* throttling disk I/O */ + if (blk->public.throttle_state) { + throttle_group_co_io_limits_intercept(blk, bytes, true); + } + if (!blk->enable_write_cache) { flags |= BDRV_REQ_FUA; } diff --git a/block/io.c b/block/io.c index bdbaa1c0da..cf2ac4cca5 100644 --- a/block/io.c +++ b/block/io.c @@ -1083,11 +1083,6 @@ int coroutine_fn bdrv_co_preadv(BlockDriverState *bs, flags |= BDRV_REQ_COPY_ON_READ; } - /* throttling disk I/O */ - if (bs->blk && blk_get_public(bs->blk)->throttle_state) { - throttle_group_co_io_limits_intercept(bs, bytes, false); - } - /* Align read if necessary by padding qiov */ if (offset & (align - 1)) { head_buf = qemu_blockalign(bs, align); @@ -1444,11 +1439,6 @@ int coroutine_fn bdrv_co_pwritev(BlockDriverState *bs, return ret; } - /* throttling disk I/O */ - if (bs->blk && blk_get_public(bs->blk)->throttle_state) { - throttle_group_co_io_limits_intercept(bs, bytes, true); - } - /* * Align write if necessary by performing a read-modify-write cycle. * Pad qiov with the read parts and be sure to have a tracked request not diff --git a/block/throttle-groups.c b/block/throttle-groups.c index 56dc311867..3db8cf74a5 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -284,18 +284,17 @@ static void schedule_next_request(BlockBackend *blk, bool is_write) * if necessary, and schedule the next request using a round robin * algorithm. * - * @bs: the current BlockDriverState + * @blk: the current BlockBackend * @bytes: the number of bytes for this I/O * @is_write: the type of operation (read/write) */ -void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs, +void coroutine_fn throttle_group_co_io_limits_intercept(BlockBackend *blk, unsigned int bytes, bool is_write) { bool must_wait; BlockBackend *token; - BlockBackend *blk = bs->blk; BlockBackendPublic *blkp = blk_get_public(blk); ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts); qemu_mutex_lock(&tg->lock); diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h index 840ba443ae..ac4224835b 100644 --- a/include/block/throttle-groups.h +++ b/include/block/throttle-groups.h @@ -40,7 +40,7 @@ void throttle_group_register_blk(BlockBackend *blk, const char *groupname); void throttle_group_unregister_blk(BlockBackend *blk); void throttle_group_restart_blk(BlockBackend *blk); -void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs, +void coroutine_fn throttle_group_co_io_limits_intercept(BlockBackend *blk, unsigned int bytes, bool is_write); -- cgit v1.2.3-55-g7522 From 97148076e8beebbcab11e5cb581d8508722143fc Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 21 Mar 2016 13:53:52 +0100 Subject: block: Move I/O throttling configuration functions to BlockBackend Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Acked-by: Stefan Hajnoczi --- block.c | 2 +- block/block-backend.c | 43 +++++++++++++++++++++++++++++++++++++++-- block/io.c | 41 --------------------------------------- block/qapi.c | 2 +- block/throttle-groups.c | 12 ++++++------ blockdev.c | 16 +++++++-------- include/block/block.h | 4 ---- include/block/block_int.h | 3 +-- include/block/throttle-groups.h | 4 ++-- include/sysemu/block-backend.h | 5 +++++ tests/test-throttle.c | 16 ++++++--------- 11 files changed, 71 insertions(+), 77 deletions(-) (limited to 'include') diff --git a/block.c b/block.c index f723060deb..b5c02c4654 100644 --- a/block.c +++ b/block.c @@ -2123,7 +2123,7 @@ static void bdrv_close(BlockDriverState *bs) /* Disable I/O limits and drain all pending throttled requests */ if (bs->blk && blk_get_public(bs->blk)->throttle_state) { - bdrv_io_limits_disable(bs); + blk_io_limits_disable(bs->blk); } bdrv_drained_begin(bs); /* complete I/O */ diff --git a/block/block-backend.c b/block/block-backend.c index 730b8a9949..b9aaa604a6 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -442,7 +442,7 @@ void blk_remove_bs(BlockBackend *blk) blk_update_root_state(blk); if (blk->public.throttle_state) { - bdrv_io_limits_disable(blk->root->bs); + blk_io_limits_disable(blk); } blk->root->bs->blk = NULL; @@ -1556,7 +1556,7 @@ void blk_apply_root_state(BlockBackend *blk, BlockDriverState *bs) { bs->detect_zeroes = blk->root_state.detect_zeroes; if (blk->root_state.throttle_group) { - bdrv_io_limits_enable(bs, blk->root_state.throttle_group); + blk_io_limits_enable(blk, blk->root_state.throttle_group); } } @@ -1620,3 +1620,42 @@ int blk_flush_all(void) return result; } + + +/* throttling disk I/O limits */ +void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg) +{ + throttle_group_config(blk, cfg); +} + +void blk_io_limits_disable(BlockBackend *blk) +{ + assert(blk->public.throttle_state); + bdrv_no_throttling_begin(blk_bs(blk)); + throttle_group_unregister_blk(blk); + bdrv_no_throttling_end(blk_bs(blk)); +} + +/* should be called before blk_set_io_limits if a limit is set */ +void blk_io_limits_enable(BlockBackend *blk, const char *group) +{ + assert(!blk->public.throttle_state); + throttle_group_register_blk(blk, group); +} + +void blk_io_limits_update_group(BlockBackend *blk, const char *group) +{ + /* this BB is not part of any group */ + if (!blk->public.throttle_state) { + return; + } + + /* this BB is a part of the same group than the one we want */ + if (!g_strcmp0(throttle_group_get_name(blk), group)) { + return; + } + + /* need to change the group this bs belong to */ + blk_io_limits_disable(blk); + blk_io_limits_enable(blk, group); +} diff --git a/block/io.c b/block/io.c index cf2ac4cca5..1699f1ef18 100644 --- a/block/io.c +++ b/block/io.c @@ -46,13 +46,6 @@ static void coroutine_fn bdrv_co_do_rw(void *opaque); static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs, int64_t sector_num, int nb_sectors, BdrvRequestFlags flags); -/* throttling disk I/O limits */ -void bdrv_set_io_limits(BlockDriverState *bs, - ThrottleConfig *cfg) -{ - throttle_group_config(bs, cfg); -} - void bdrv_no_throttling_begin(BlockDriverState *bs) { if (!bs->blk) { @@ -77,40 +70,6 @@ void bdrv_no_throttling_end(BlockDriverState *bs) --blkp->io_limits_disabled; } -void bdrv_io_limits_disable(BlockDriverState *bs) -{ - assert(blk_get_public(bs->blk)->throttle_state); - bdrv_no_throttling_begin(bs); - throttle_group_unregister_blk(bs->blk); - bdrv_no_throttling_end(bs); -} - -/* should be called before bdrv_set_io_limits if a limit is set */ -void bdrv_io_limits_enable(BlockDriverState *bs, const char *group) -{ - BlockBackendPublic *blkp = blk_get_public(bs->blk); - - assert(!blkp->throttle_state); - throttle_group_register_blk(bs->blk, group); -} - -void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group) -{ - /* this bs is not part of any group */ - if (!blk_get_public(bs->blk)->throttle_state) { - return; - } - - /* this bs is a part of the same group than the one we want */ - if (!g_strcmp0(throttle_group_get_name(bs->blk), group)) { - return; - } - - /* need to change the group this bs belong to */ - bdrv_io_limits_disable(bs); - bdrv_io_limits_enable(bs, group); -} - void bdrv_refresh_limits(BlockDriverState *bs, Error **errp) { BlockDriver *drv = bs->drv; diff --git a/block/qapi.c b/block/qapi.c index 1e4bb8a0a5..b0d8d6b71e 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -70,7 +70,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk, if (bs->blk && blk_get_public(bs->blk)->throttle_state) { ThrottleConfig cfg; - throttle_group_get_config(bs, &cfg); + throttle_group_get_config(bs->blk, &cfg); info->bps = cfg.buckets[THROTTLE_BPS_TOTAL].avg; info->bps_rd = cfg.buckets[THROTTLE_BPS_READ].avg; diff --git a/block/throttle-groups.c b/block/throttle-groups.c index 3db8cf74a5..59545e287e 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -337,12 +337,12 @@ void throttle_group_restart_blk(BlockBackend *blk) * to throttle_config(), but guarantees atomicity within the * throttling group. * - * @bs: a BlockDriverState that is member of the group + * @blk: a BlockBackend that is a member of the group * @cfg: the configuration to set */ -void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg) +void throttle_group_config(BlockBackend *blk, ThrottleConfig *cfg) { - BlockBackendPublic *blkp = blk_get_public(bs->blk); + BlockBackendPublic *blkp = blk_get_public(blk); ThrottleTimers *tt = &blkp->throttle_timers; ThrottleState *ts = blkp->throttle_state; ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); @@ -365,12 +365,12 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg) * throttle_get_config(), but guarantees atomicity within the * throttling group. * - * @bs: a BlockDriverState that is member of the group + * @blk: a BlockBackend that is a member of the group * @cfg: the configuration will be written here */ -void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg) +void throttle_group_get_config(BlockBackend *blk, ThrottleConfig *cfg) { - BlockBackendPublic *blkp = blk_get_public(bs->blk); + BlockBackendPublic *blkp = blk_get_public(blk); ThrottleState *ts = blkp->throttle_state; ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); qemu_mutex_lock(&tg->lock); diff --git a/blockdev.c b/blockdev.c index 3211a40615..89768ab759 100644 --- a/blockdev.c +++ b/blockdev.c @@ -616,8 +616,8 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, if (!throttling_group) { throttling_group = blk_name(blk); } - bdrv_io_limits_enable(bs, throttling_group); - bdrv_set_io_limits(bs, &cfg); + blk_io_limits_enable(blk, throttling_group); + blk_set_io_limits(blk, &cfg); } if (bdrv_key_required(bs)) { @@ -2726,16 +2726,16 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd, if (throttle_enabled(&cfg)) { /* Enable I/O limits if they're not enabled yet, otherwise * just update the throttling group. */ - if (!blk_get_public(bs->blk)->throttle_state) { - bdrv_io_limits_enable(bs, has_group ? group : device); + if (!blk_get_public(blk)->throttle_state) { + blk_io_limits_enable(blk, has_group ? group : device); } else if (has_group) { - bdrv_io_limits_update_group(bs, group); + blk_io_limits_update_group(blk, group); } /* Set the new throttling configuration */ - bdrv_set_io_limits(bs, &cfg); - } else if (blk_get_public(bs->blk)->throttle_state) { + blk_set_io_limits(blk, &cfg); + } else if (blk_get_public(blk)->throttle_state) { /* If all throttling settings are set to 0, disable I/O limits */ - bdrv_io_limits_disable(bs); + blk_io_limits_disable(blk); } out: diff --git a/include/block/block.h b/include/block/block.h index b210832778..2c5c280a42 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -187,10 +187,6 @@ void bdrv_stats_print(Monitor *mon, const QObject *data); void bdrv_info_stats(Monitor *mon, QObject **ret_data); /* disk I/O throttling */ -void bdrv_io_limits_enable(BlockDriverState *bs, const char *group); -void bdrv_io_limits_disable(BlockDriverState *bs); -void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group); - void bdrv_init(void); void bdrv_init_with_whitelist(void); bool bdrv_uses_whitelist(void); diff --git a/include/block/block_int.h b/include/block/block_int.h index 2bbc2c0609..1218857f82 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -525,8 +525,7 @@ int get_tmp_filename(char *filename, int size); BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size, const char *filename); -void bdrv_set_io_limits(BlockDriverState *bs, - ThrottleConfig *cfg); +bool bdrv_start_throttled_reqs(BlockDriverState *bs); /** diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h index ac4224835b..d983d34074 100644 --- a/include/block/throttle-groups.h +++ b/include/block/throttle-groups.h @@ -33,8 +33,8 @@ const char *throttle_group_get_name(BlockBackend *blk); ThrottleState *throttle_group_incref(const char *name); void throttle_group_unref(ThrottleState *ts); -void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg); -void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg); +void throttle_group_config(BlockBackend *blk, ThrottleConfig *cfg); +void throttle_group_get_config(BlockBackend *blk, ThrottleConfig *cfg); void throttle_group_register_blk(BlockBackend *blk, const char *groupname); void throttle_group_unregister_blk(BlockBackend *blk); diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 08d27a86be..dd9c8ca4e0 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -212,4 +212,9 @@ BlockAIOCB *blk_abort_aio_request(BlockBackend *blk, BlockCompletionFunc *cb, void *opaque, int ret); +void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg); +void blk_io_limits_disable(BlockBackend *blk); +void blk_io_limits_enable(BlockBackend *blk, const char *group); +void blk_io_limits_update_group(BlockBackend *blk, const char *group); + #endif diff --git a/tests/test-throttle.c b/tests/test-throttle.c index a0200686d6..5ec966c8a4 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -577,15 +577,11 @@ static void test_groups(void) ThrottleConfig cfg1, cfg2; BlockBackend *blk1, *blk2, *blk3; BlockBackendPublic *blkp1, *blkp2, *blkp3; - BlockDriverState *bdrv1, *bdrv3; blk1 = blk_new_with_bs(&error_abort); blk2 = blk_new_with_bs(&error_abort); blk3 = blk_new_with_bs(&error_abort); - bdrv1 = blk_bs(blk1); - bdrv3 = blk_bs(blk3); - blkp1 = blk_get_public(blk1); blkp2 = blk_get_public(blk2); blkp3 = blk_get_public(blk3); @@ -612,20 +608,20 @@ static void test_groups(void) cfg1.buckets[THROTTLE_BPS_WRITE].avg = 285000; cfg1.buckets[THROTTLE_OPS_READ].avg = 20000; cfg1.buckets[THROTTLE_OPS_WRITE].avg = 12000; - throttle_group_config(bdrv1, &cfg1); + throttle_group_config(blk1, &cfg1); - throttle_group_get_config(bdrv1, &cfg1); - throttle_group_get_config(bdrv3, &cfg2); + throttle_group_get_config(blk1, &cfg1); + throttle_group_get_config(blk3, &cfg2); g_assert(!memcmp(&cfg1, &cfg2, sizeof(cfg1))); cfg2.buckets[THROTTLE_BPS_READ].avg = 4547; cfg2.buckets[THROTTLE_BPS_WRITE].avg = 1349; cfg2.buckets[THROTTLE_OPS_READ].avg = 123; cfg2.buckets[THROTTLE_OPS_WRITE].avg = 86; - throttle_group_config(bdrv3, &cfg1); + throttle_group_config(blk3, &cfg1); - throttle_group_get_config(bdrv1, &cfg1); - throttle_group_get_config(bdrv3, &cfg2); + throttle_group_get_config(blk1, &cfg1); + throttle_group_get_config(blk3, &cfg2); g_assert(!memcmp(&cfg1, &cfg2, sizeof(cfg1))); throttle_group_unregister_blk(blk1); -- cgit v1.2.3-55-g7522 From 22aa8b246ada8d45add8eef64d4640a20b4f7ef3 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 24 Feb 2016 15:13:35 +0100 Subject: block: Introduce BdrvChild.opaque BlockBackends use it to get a back pointer from BdrvChild to BlockBackend in any BdrvChildRole callbacks. Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Acked-by: Stefan Hajnoczi --- block/block-backend.c | 2 ++ include/block/block_int.h | 1 + 2 files changed, 3 insertions(+) (limited to 'include') diff --git a/block/block-backend.c b/block/block-backend.c index b9aaa604a6..52b7b922c2 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -133,6 +133,7 @@ BlockBackend *blk_new_with_bs(Error **errp) bs = bdrv_new_root(); blk->root = bdrv_root_attach_child(bs, "root", &child_root); + blk->root->opaque = blk; bs->blk = blk; return blk; } @@ -458,6 +459,7 @@ void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs) assert(!blk->root && !bs->blk); bdrv_ref(bs); blk->root = bdrv_root_attach_child(bs, "root", &child_root); + blk->root->opaque = blk; bs->blk = blk; notifier_list_notify(&blk->insert_bs_notifiers, blk); diff --git a/include/block/block_int.h b/include/block/block_int.h index 1218857f82..47dfafa462 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -373,6 +373,7 @@ struct BdrvChild { BlockDriverState *bs; char *name; const BdrvChildRole *role; + void *opaque; QLIST_ENTRY(BdrvChild) next; QLIST_ENTRY(BdrvChild) next_parent; }; -- cgit v1.2.3-55-g7522 From c2066af0514d80a805dac82accaec1121babfa15 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 22 Mar 2016 12:05:35 +0100 Subject: block: Drain throttling queue with BdrvChild callback This removes the last part of I/O throttling from block/io.c and moves it to the BlockBackend. Instead of having knowledge about throttling inside io.c, we can call a BdrvChild callback .drained_begin/end, which happens to drain the throttled requests for BlockBackend parents. Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Acked-by: Stefan Hajnoczi --- block/block-backend.c | 32 +++++++++++++++++++++++++++----- block/io.c | 39 ++++++++++++++++++--------------------- include/block/block_int.h | 16 +++++++++++----- 3 files changed, 56 insertions(+), 31 deletions(-) (limited to 'include') diff --git a/block/block-backend.c b/block/block-backend.c index 52b7b922c2..74429ae36b 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -91,9 +91,14 @@ static void blk_root_inherit_options(int *child_flags, QDict *child_options, /* We're not supposed to call this function for root nodes */ abort(); } +static void blk_root_drained_begin(BdrvChild *child); +static void blk_root_drained_end(BdrvChild *child); static const BdrvChildRole child_root = { - .inherit_options = blk_root_inherit_options, + .inherit_options = blk_root_inherit_options, + + .drained_begin = blk_root_drained_begin, + .drained_end = blk_root_drained_end, }; /* @@ -818,9 +823,9 @@ int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf, return ret; } - bdrv_no_throttling_begin(blk_bs(blk)); + blk_root_drained_begin(blk->root); ret = blk_pread(blk, offset, buf, count); - bdrv_no_throttling_end(blk_bs(blk)); + blk_root_drained_end(blk->root); return ret; } @@ -1633,9 +1638,9 @@ void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg) void blk_io_limits_disable(BlockBackend *blk) { assert(blk->public.throttle_state); - bdrv_no_throttling_begin(blk_bs(blk)); + bdrv_drained_begin(blk_bs(blk)); throttle_group_unregister_blk(blk); - bdrv_no_throttling_end(blk_bs(blk)); + bdrv_drained_end(blk_bs(blk)); } /* should be called before blk_set_io_limits if a limit is set */ @@ -1661,3 +1666,20 @@ void blk_io_limits_update_group(BlockBackend *blk, const char *group) blk_io_limits_disable(blk); blk_io_limits_enable(blk, group); } + +static void blk_root_drained_begin(BdrvChild *child) +{ + BlockBackend *blk = child->opaque; + + if (blk->public.io_limits_disabled++ == 0) { + throttle_group_restart_blk(blk); + } +} + +static void blk_root_drained_end(BdrvChild *child) +{ + BlockBackend *blk = child->opaque; + + assert(blk->public.io_limits_disabled); + --blk->public.io_limits_disabled; +} diff --git a/block/io.c b/block/io.c index 1699f1ef18..7c213ec226 100644 --- a/block/io.c +++ b/block/io.c @@ -27,7 +27,6 @@ #include "sysemu/block-backend.h" #include "block/blockjob.h" #include "block/block_int.h" -#include "block/throttle-groups.h" #include "qemu/cutils.h" #include "qapi/error.h" #include "qemu/error-report.h" @@ -46,28 +45,26 @@ static void coroutine_fn bdrv_co_do_rw(void *opaque); static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs, int64_t sector_num, int nb_sectors, BdrvRequestFlags flags); -void bdrv_no_throttling_begin(BlockDriverState *bs) +static void bdrv_parent_drained_begin(BlockDriverState *bs) { - if (!bs->blk) { - return; - } + BdrvChild *c; - if (blk_get_public(bs->blk)->io_limits_disabled++ == 0) { - throttle_group_restart_blk(bs->blk); + QLIST_FOREACH(c, &bs->parents, next_parent) { + if (c->role->drained_begin) { + c->role->drained_begin(c); + } } } -void bdrv_no_throttling_end(BlockDriverState *bs) +static void bdrv_parent_drained_end(BlockDriverState *bs) { - BlockBackendPublic *blkp; + BdrvChild *c; - if (!bs->blk) { - return; + QLIST_FOREACH(c, &bs->parents, next_parent) { + if (c->role->drained_end) { + c->role->drained_end(c); + } } - - blkp = blk_get_public(bs->blk); - assert(blkp->io_limits_disabled); - --blkp->io_limits_disabled; } void bdrv_refresh_limits(BlockDriverState *bs, Error **errp) @@ -248,17 +245,17 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs) */ void coroutine_fn bdrv_co_drain(BlockDriverState *bs) { - bdrv_no_throttling_begin(bs); + bdrv_parent_drained_begin(bs); bdrv_io_unplugged_begin(bs); bdrv_drain_recurse(bs); bdrv_co_yield_to_drain(bs); bdrv_io_unplugged_end(bs); - bdrv_no_throttling_end(bs); + bdrv_parent_drained_end(bs); } void bdrv_drain(BlockDriverState *bs) { - bdrv_no_throttling_begin(bs); + bdrv_parent_drained_begin(bs); bdrv_io_unplugged_begin(bs); bdrv_drain_recurse(bs); if (qemu_in_coroutine()) { @@ -267,7 +264,7 @@ void bdrv_drain(BlockDriverState *bs) bdrv_drain_poll(bs); } bdrv_io_unplugged_end(bs); - bdrv_no_throttling_end(bs); + bdrv_parent_drained_end(bs); } /* @@ -290,7 +287,7 @@ void bdrv_drain_all(void) if (bs->job) { block_job_pause(bs->job); } - bdrv_no_throttling_begin(bs); + bdrv_parent_drained_begin(bs); bdrv_io_unplugged_begin(bs); bdrv_drain_recurse(bs); aio_context_release(aio_context); @@ -333,7 +330,7 @@ void bdrv_drain_all(void) aio_context_acquire(aio_context); bdrv_io_unplugged_end(bs); - bdrv_no_throttling_end(bs); + bdrv_parent_drained_end(bs); if (bs->job) { block_job_resume(bs->job); } diff --git a/include/block/block_int.h b/include/block/block_int.h index 47dfafa462..a1f8488bca 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -364,6 +364,17 @@ typedef struct BdrvAioNotifier { struct BdrvChildRole { void (*inherit_options)(int *child_flags, QDict *child_options, int parent_flags, QDict *parent_options); + + /* + * If this pair of functions is implemented, the parent doesn't issue new + * requests after returning from .drained_begin() until .drained_end() is + * called. + * + * Note that this can be nested. If drained_begin() was called twice, new + * I/O is allowed only after drained_end() was called twice, too. + */ + void (*drained_begin)(BdrvChild *child); + void (*drained_end)(BdrvChild *child); }; extern const BdrvChildRole child_file; @@ -526,8 +537,6 @@ int get_tmp_filename(char *filename, int size); BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size, const char *filename); -bool bdrv_start_throttled_reqs(BlockDriverState *bs); - /** * bdrv_add_before_write_notifier: @@ -710,9 +719,6 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs, const BdrvChildRole *child_role); void bdrv_root_unref_child(BdrvChild *child); -void bdrv_no_throttling_begin(BlockDriverState *bs); -void bdrv_no_throttling_end(BlockDriverState *bs); - void blk_dev_change_media_cb(BlockBackend *blk, bool load); bool blk_dev_has_removable_media(BlockBackend *blk); bool blk_dev_has_tray(BlockBackend *blk); -- cgit v1.2.3-55-g7522 From 7ca7f0f6db1fedd28d490795d778cf23979a2aa7 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 22 Mar 2016 13:00:08 +0100 Subject: block: Decouple throttling from BlockDriverState This moves the throttling related part of the BDS life cycle management to BlockBackend. The throttling group reference is now kept even when no medium is inserted. With this commit, throttling isn't disabled and then re-enabled any more during graph reconfiguration. This fixes the temporary breakage of I/O throttling when used with live snapshots or block jobs that manipulate the graph. Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Acked-by: Stefan Hajnoczi --- block.c | 34 ---------------------------------- block/block-backend.c | 37 ++++++++++++++----------------------- blockdev.c | 27 +++++++++------------------ include/block/block_int.h | 3 --- 4 files changed, 23 insertions(+), 78 deletions(-) (limited to 'include') diff --git a/block.c b/block.c index b5c02c4654..98226fe073 100644 --- a/block.c +++ b/block.c @@ -38,7 +38,6 @@ #include "qmp-commands.h" #include "qemu/timer.h" #include "qapi-event.h" -#include "block/throttle-groups.h" #include "qemu/cutils.h" #include "qemu/id.h" @@ -2121,11 +2120,6 @@ static void bdrv_close(BlockDriverState *bs) assert(!bs->job); - /* Disable I/O limits and drain all pending throttled requests */ - if (bs->blk && blk_get_public(bs->blk)->throttle_state) { - blk_io_limits_disable(bs->blk); - } - bdrv_drained_begin(bs); /* complete I/O */ bdrv_flush(bs); bdrv_drain(bs); /* in case flush left pending I/O */ @@ -2254,26 +2248,6 @@ static void swap_feature_fields(BlockDriverState *bs_top, bdrv_move_feature_fields(&tmp, bs_top); bdrv_move_feature_fields(bs_top, bs_new); bdrv_move_feature_fields(bs_new, &tmp); - - assert(!bs_new->blk); - if (bs_top->blk && blk_get_public(bs_top->blk)->throttle_state) { - /* - * FIXME Need to break I/O throttling with graph manipulations - * temporarily because of conflicting invariants (3. will go away when - * throttling is fully converted to work on BlockBackends): - * - * 1. Every BlockBackend has a single root BDS - * 2. I/O throttling functions require an attached BlockBackend - * 3. We need to first enable throttling on the new BDS and then - * disable it on the old one (because of throttle group refcounts) - */ -#if 0 - bdrv_io_limits_enable(bs_new, throttle_group_get_name(bs_top)); - bdrv_io_limits_disable(bs_top); -#else - abort(); -#endif - } } /* @@ -3674,10 +3648,6 @@ void bdrv_detach_aio_context(BlockDriverState *bs) baf->detach_aio_context(baf->opaque); } - if (bs->blk && blk_get_public(bs->blk)->throttle_state) { - throttle_timers_detach_aio_context( - &blk_get_public(bs->blk)->throttle_timers); - } if (bs->drv->bdrv_detach_aio_context) { bs->drv->bdrv_detach_aio_context(bs); } @@ -3711,10 +3681,6 @@ void bdrv_attach_aio_context(BlockDriverState *bs, if (bs->drv->bdrv_attach_aio_context) { bs->drv->bdrv_attach_aio_context(bs, new_context); } - if (bs->blk && blk_get_public(bs->blk)->throttle_state) { - throttle_timers_attach_aio_context( - &blk_get_public(bs->blk)->throttle_timers, new_context); - } QLIST_FOREACH(ban, &bs->aio_notifiers, list) { ban->attached_aio_context(new_context, ban->opaque); diff --git a/block/block-backend.c b/block/block-backend.c index 74429ae36b..a71b54d24d 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -188,10 +188,6 @@ static void blk_delete(BlockBackend *blk) } assert(QLIST_EMPTY(&blk->remove_bs_notifiers.notifiers)); assert(QLIST_EMPTY(&blk->insert_bs_notifiers.notifiers)); - if (blk->root_state.throttle_state) { - g_free(blk->root_state.throttle_group); - throttle_group_unref(blk->root_state.throttle_state); - } QTAILQ_REMOVE(&block_backends, blk, link); drive_info_del(blk->legacy_dinfo); block_acct_cleanup(&blk->stats); @@ -445,12 +441,12 @@ void blk_remove_bs(BlockBackend *blk) assert(blk->root->bs->blk == blk); notifier_list_notify(&blk->remove_bs_notifiers, blk); - - blk_update_root_state(blk); if (blk->public.throttle_state) { - blk_io_limits_disable(blk); + throttle_timers_detach_aio_context(&blk->public.throttle_timers); } + blk_update_root_state(blk); + blk->root->bs->blk = NULL; bdrv_root_unref_child(blk->root); blk->root = NULL; @@ -468,6 +464,10 @@ void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs) bs->blk = blk; notifier_list_notify(&blk->insert_bs_notifiers, blk); + if (blk->public.throttle_state) { + throttle_timers_attach_aio_context( + &blk->public.throttle_timers, bdrv_get_aio_context(bs)); + } } /* @@ -1374,7 +1374,14 @@ void blk_set_aio_context(BlockBackend *blk, AioContext *new_context) BlockDriverState *bs = blk_bs(blk); if (bs) { + if (blk->public.throttle_state) { + throttle_timers_detach_aio_context(&blk->public.throttle_timers); + } bdrv_set_aio_context(bs, new_context); + if (blk->public.throttle_state) { + throttle_timers_attach_aio_context(&blk->public.throttle_timers, + new_context); + } } } @@ -1539,19 +1546,6 @@ void blk_update_root_state(BlockBackend *blk) blk->root_state.open_flags = blk->root->bs->open_flags; blk->root_state.read_only = blk->root->bs->read_only; blk->root_state.detect_zeroes = blk->root->bs->detect_zeroes; - - if (blk->root_state.throttle_group) { - g_free(blk->root_state.throttle_group); - throttle_group_unref(blk->root_state.throttle_state); - } - if (blk->public.throttle_state) { - const char *name = throttle_group_get_name(blk); - blk->root_state.throttle_group = g_strdup(name); - blk->root_state.throttle_state = throttle_group_incref(name); - } else { - blk->root_state.throttle_group = NULL; - blk->root_state.throttle_state = NULL; - } } /* @@ -1562,9 +1556,6 @@ void blk_update_root_state(BlockBackend *blk) void blk_apply_root_state(BlockBackend *blk, BlockDriverState *bs) { bs->detect_zeroes = blk->root_state.detect_zeroes; - if (blk->root_state.throttle_group) { - blk_io_limits_enable(blk, blk->root_state.throttle_group); - } } /* diff --git a/blockdev.c b/blockdev.c index 89768ab759..93ddc27307 100644 --- a/blockdev.c +++ b/blockdev.c @@ -577,15 +577,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, blk_rs->read_only = !(bdrv_flags & BDRV_O_RDWR); blk_rs->detect_zeroes = detect_zeroes; - if (throttle_enabled(&cfg)) { - if (!throttling_group) { - throttling_group = blk_name(blk); - } - blk_rs->throttle_group = g_strdup(throttling_group); - blk_rs->throttle_state = throttle_group_incref(throttling_group); - blk_rs->throttle_state->cfg = cfg; - } - QDECREF(bs_opts); } else { if (file && !*file) { @@ -611,15 +602,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, bs->detect_zeroes = detect_zeroes; - /* disk I/O throttling */ - if (throttle_enabled(&cfg)) { - if (!throttling_group) { - throttling_group = blk_name(blk); - } - blk_io_limits_enable(blk, throttling_group); - blk_set_io_limits(blk, &cfg); - } - if (bdrv_key_required(bs)) { autostart = 0; } @@ -633,6 +615,15 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, } } + /* disk I/O throttling */ + if (throttle_enabled(&cfg)) { + if (!throttling_group) { + throttling_group = blk_name(blk); + } + blk_io_limits_enable(blk, throttling_group); + blk_set_io_limits(blk, &cfg); + } + blk_set_enable_write_cache(blk, !writethrough); blk_set_on_error(blk, on_read_error, on_write_error); diff --git a/include/block/block_int.h b/include/block/block_int.h index a1f8488bca..db2b4bced3 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -500,9 +500,6 @@ struct BlockBackendRootState { int open_flags; bool read_only; BlockdevDetectZeroesOptions detect_zeroes; - - char *throttle_group; - ThrottleState *throttle_state; }; static inline BlockDriverState *backing_bs(BlockDriverState *bs) -- cgit v1.2.3-55-g7522 From 5c8cab48087d3544cb788309ac729bca08244020 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 24 Feb 2016 15:13:35 +0100 Subject: block: Use BdrvChild callbacks for change_media/resize We want to get rid of BlockDriverState.blk in order to allow multiple BlockBackends per BDS. Converting the device callbacks in block.c (which assume a single BlockBackend) to per-child callbacks gets us rid of the first few instances. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- block.c | 39 ++++++++++++++++++++++++++------------- block/block-backend.c | 15 ++++++++++++++- include/block/block_int.h | 4 +++- 3 files changed, 43 insertions(+), 15 deletions(-) (limited to 'include') diff --git a/block.c b/block.c index cedbd7061b..ab592acaf7 100644 --- a/block.c +++ b/block.c @@ -1214,6 +1214,27 @@ void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child) bdrv_root_unref_child(child); } + +static void bdrv_parent_cb_change_media(BlockDriverState *bs, bool load) +{ + BdrvChild *c; + QLIST_FOREACH(c, &bs->parents, next_parent) { + if (c->role->change_media) { + c->role->change_media(c, load); + } + } +} + +static void bdrv_parent_cb_resize(BlockDriverState *bs) +{ + BdrvChild *c; + QLIST_FOREACH(c, &bs->parents, next_parent) { + if (c->role->resize) { + c->role->resize(c); + } + } +} + /* * Sets the backing file link of a BDS. A new reference is created; callers * which don't need their own reference any more must call bdrv_unref(). @@ -1673,9 +1694,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, } if (!bdrv_key_required(bs)) { - if (bs->blk) { - blk_dev_change_media_cb(bs->blk, true); - } + bdrv_parent_cb_change_media(bs, true); } else if (!runstate_check(RUN_STATE_PRELAUNCH) && !runstate_check(RUN_STATE_INMIGRATE) && !runstate_check(RUN_STATE_PAUSED)) { /* HACK */ @@ -2121,9 +2140,7 @@ static void bdrv_close(BlockDriverState *bs) bdrv_release_named_dirty_bitmaps(bs); assert(QLIST_EMPTY(&bs->dirty_bitmaps)); - if (bs->blk) { - blk_dev_change_media_cb(bs->blk, false); - } + bdrv_parent_cb_change_media(bs, false); if (bs->drv) { BdrvChild *child, *next; @@ -2574,9 +2591,7 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset) if (ret == 0) { ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); bdrv_dirty_bitmap_truncate(bs); - if (bs->blk) { - blk_dev_resize_cb(bs->blk); - } + bdrv_parent_cb_resize(bs); } return ret; } @@ -2686,11 +2701,9 @@ int bdrv_set_key(BlockDriverState *bs, const char *key) if (ret < 0) { bs->valid_key = 0; } else if (!bs->valid_key) { + /* call the change callback now, we skipped it on open */ bs->valid_key = 1; - if (bs->blk) { - /* call the change callback now, we skipped it on open */ - blk_dev_change_media_cb(bs->blk, true); - } + bdrv_parent_cb_change_media(bs, true); } return ret; } diff --git a/block/block-backend.c b/block/block-backend.c index a71b54d24d..a085455850 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -94,9 +94,15 @@ static void blk_root_inherit_options(int *child_flags, QDict *child_options, static void blk_root_drained_begin(BdrvChild *child); static void blk_root_drained_end(BdrvChild *child); +static void blk_root_change_media(BdrvChild *child, bool load); +static void blk_root_resize(BdrvChild *child); + static const BdrvChildRole child_root = { .inherit_options = blk_root_inherit_options, + .change_media = blk_root_change_media, + .resize = blk_root_resize, + .drained_begin = blk_root_drained_begin, .drained_end = blk_root_drained_end, }; @@ -556,6 +562,11 @@ void blk_dev_change_media_cb(BlockBackend *blk, bool load) } } +static void blk_root_change_media(BdrvChild *child, bool load) +{ + blk_dev_change_media_cb(child->opaque, load); +} + /* * Does @blk's attached device model have removable media? * %true if no device model is attached. @@ -610,8 +621,10 @@ bool blk_dev_is_medium_locked(BlockBackend *blk) /* * Notify @blk's attached device model of a backend size change. */ -void blk_dev_resize_cb(BlockBackend *blk) +static void blk_root_resize(BdrvChild *child) { + BlockBackend *blk = child->opaque; + if (blk->dev_ops && blk->dev_ops->resize_cb) { blk->dev_ops->resize_cb(blk->dev_opaque); } diff --git a/include/block/block_int.h b/include/block/block_int.h index db2b4bced3..9555429e40 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -365,6 +365,9 @@ struct BdrvChildRole { void (*inherit_options)(int *child_flags, QDict *child_options, int parent_flags, QDict *parent_options); + void (*change_media)(BdrvChild *child, bool load); + void (*resize)(BdrvChild *child); + /* * If this pair of functions is implemented, the parent doesn't issue new * requests after returning from .drained_begin() until .drained_end() is @@ -722,7 +725,6 @@ bool blk_dev_has_tray(BlockBackend *blk); void blk_dev_eject_request(BlockBackend *blk, bool force); bool blk_dev_is_tray_open(BlockBackend *blk); bool blk_dev_is_medium_locked(BlockBackend *blk); -void blk_dev_resize_cb(BlockBackend *blk); void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors); bool bdrv_requests_pending(BlockDriverState *bs); -- cgit v1.2.3-55-g7522 From 4c265bf9f434d4b47f42e3c079adc205b7625ad6 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 26 Feb 2016 10:22:16 +0100 Subject: block: User BdrvChild callback for device name In order to get rid of bs->blk for bdrv_get_device_name() and bdrv_get_device_or_node_name(), ask all parents for their name and simply pick the first one. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- block.c | 22 ++++++++++++++++++++-- block/block-backend.c | 6 ++++++ include/block/block_int.h | 5 +++++ 3 files changed, 31 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/block.c b/block.c index ab592acaf7..fd4cf81096 100644 --- a/block.c +++ b/block.c @@ -2894,10 +2894,28 @@ const char *bdrv_get_node_name(const BlockDriverState *bs) return bs->node_name; } +static const char *bdrv_get_parent_name(const BlockDriverState *bs) +{ + BdrvChild *c; + const char *name; + + /* If multiple parents have a name, just pick the first one. */ + QLIST_FOREACH(c, &bs->parents, next_parent) { + if (c->role->get_name) { + name = c->role->get_name(c); + if (name && *name) { + return name; + } + } + } + + return NULL; +} + /* TODO check what callers really want: bs->node_name or blk_name() */ const char *bdrv_get_device_name(const BlockDriverState *bs) { - return bs->blk ? blk_name(bs->blk) : ""; + return bdrv_get_parent_name(bs) ?: ""; } /* This can be used to identify nodes that might not have a device @@ -2906,7 +2924,7 @@ const char *bdrv_get_device_name(const BlockDriverState *bs) * absent, then this returns an empty (non-null) string. */ const char *bdrv_get_device_or_node_name(const BlockDriverState *bs) { - return bs->blk ? blk_name(bs->blk) : bs->node_name; + return bdrv_get_parent_name(bs) ?: bs->node_name; } int bdrv_get_flags(BlockDriverState *bs) diff --git a/block/block-backend.c b/block/block-backend.c index a085455850..a31fc204cd 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -97,11 +97,17 @@ static void blk_root_drained_end(BdrvChild *child); static void blk_root_change_media(BdrvChild *child, bool load); static void blk_root_resize(BdrvChild *child); +static const char *blk_root_get_name(BdrvChild *child) +{ + return blk_name(child->opaque); +} + static const BdrvChildRole child_root = { .inherit_options = blk_root_inherit_options, .change_media = blk_root_change_media, .resize = blk_root_resize, + .get_name = blk_root_get_name, .drained_begin = blk_root_drained_begin, .drained_end = blk_root_drained_end, diff --git a/include/block/block_int.h b/include/block/block_int.h index 9555429e40..80e2da519a 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -368,6 +368,11 @@ struct BdrvChildRole { void (*change_media)(BdrvChild *child, bool load); void (*resize)(BdrvChild *child); + /* Returns a name that is supposedly more useful for human users than the + * node name for identifying the node in question (in particular, a BB + * name), or NULL if the parent can't provide a better name. */ + const char* (*get_name)(BdrvChild *child); + /* * If this pair of functions is implemented, the parent doesn't issue new * requests after returning from .drained_begin() until .drained_end() is -- cgit v1.2.3-55-g7522 From 81e254dc8349795d17c585a997f9076b77a863fc Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 18 Apr 2016 11:36:38 +0200 Subject: blockjob: Don't set iostatus of target When block job errors were introduced, we assigned the iostatus of the target BDS "just in case". The field has never been accessible for the user because the target isn't listed in query-block. Before we can allow the user to have a second BlockBackend on the target, we need to clean this up. If anything, we would want to set the iostatus for the internal BB of the job (which we can always do later), but certainly not for a separate BB which the job doesn't even use. As a nice side effect, this gets us rid of another bs->blk use. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- block/backup.c | 8 ++++---- block/mirror.c | 8 ++++---- block/stream.c | 3 +-- blockjob.c | 6 +----- include/block/blockjob.h | 4 +--- 5 files changed, 11 insertions(+), 18 deletions(-) (limited to 'include') diff --git a/block/backup.c b/block/backup.c index 491fd14068..ef7609d744 100644 --- a/block/backup.c +++ b/block/backup.c @@ -272,11 +272,11 @@ static BlockErrorAction backup_error_action(BackupBlockJob *job, bool read, int error) { if (read) { - return block_job_error_action(&job->common, job->common.bs, - job->on_source_error, true, error); + return block_job_error_action(&job->common, job->on_source_error, + true, error); } else { - return block_job_error_action(&job->common, job->target, - job->on_target_error, false, error); + return block_job_error_action(&job->common, job->on_target_error, + false, error); } } diff --git a/block/mirror.c b/block/mirror.c index 039f48125e..c7f51ad094 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -80,11 +80,11 @@ static BlockErrorAction mirror_error_action(MirrorBlockJob *s, bool read, { s->synced = false; if (read) { - return block_job_error_action(&s->common, s->common.bs, - s->on_source_error, true, error); + return block_job_error_action(&s->common, s->on_source_error, + true, error); } else { - return block_job_error_action(&s->common, s->target, - s->on_target_error, false, error); + return block_job_error_action(&s->common, s->on_target_error, + false, error); } } diff --git a/block/stream.c b/block/stream.c index 332b9a183e..16c3e90021 100644 --- a/block/stream.c +++ b/block/stream.c @@ -163,8 +163,7 @@ wait: } if (ret < 0) { BlockErrorAction action = - block_job_error_action(&s->common, s->common.bs, s->on_error, - true, -ret); + block_job_error_action(&s->common, s->on_error, true, -ret); if (action == BLOCK_ERROR_ACTION_STOP) { n = 0; continue; diff --git a/blockjob.c b/blockjob.c index 9fc37ca965..5b840a7df6 100644 --- a/blockjob.c +++ b/blockjob.c @@ -411,8 +411,7 @@ void block_job_event_ready(BlockJob *job) job->speed, &error_abort); } -BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs, - BlockdevOnError on_err, +BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err, int is_read, int error) { BlockErrorAction action; @@ -443,9 +442,6 @@ BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs, job->user_paused = true; block_job_pause(job); block_job_iostatus_set_err(job, error); - if (bs->blk && bs != job->bs) { - blk_iostatus_set_err(bs->blk, error); - } } return action; } diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 8bedc4936c..073a433cf8 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -383,7 +383,6 @@ void block_job_iostatus_reset(BlockJob *job); /** * block_job_error_action: * @job: The job to signal an error for. - * @bs: The block device on which to set an I/O error. * @on_err: The error action setting. * @is_read: Whether the operation was a read. * @error: The error that was reported. @@ -391,8 +390,7 @@ void block_job_iostatus_reset(BlockJob *job); * Report an I/O error for a block job and possibly stop the VM. Return the * action that was selected based on @on_err and @error. */ -BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs, - BlockdevOnError on_err, +BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err, int is_read, int error); typedef void BlockJobDeferToMainLoopFn(BlockJob *job, void *opaque); -- cgit v1.2.3-55-g7522 From 91c6e4b7bba906cfb8d84481da6340957f876c90 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 26 Feb 2016 13:50:43 +0100 Subject: block: Remove bdrv_aio_multiwrite() Since virtio-blk implements request merging itself these days, the only remaining users are test cases for the function. That doesn't make the function exactly useful any more. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz Reviewed-by: Eric Blake --- block/block-backend.c | 14 --- block/io.c | 194 --------------------------------------- include/block/block.h | 7 +- include/sysemu/block-backend.h | 1 - qemu-io-cmds.c | 203 ----------------------------------------- tests/qemu-iotests/100 | 152 ------------------------------ tests/qemu-iotests/100.out | 103 --------------------- tests/qemu-iotests/136 | 20 +--- tests/qemu-iotests/136.out | 4 +- tests/qemu-iotests/group | 2 +- trace-events | 2 - 11 files changed, 9 insertions(+), 693 deletions(-) delete mode 100755 tests/qemu-iotests/100 delete mode 100644 tests/qemu-iotests/100.out (limited to 'include') diff --git a/block/block-backend.c b/block/block-backend.c index a31fc204cd..8d6fc77b26 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1067,20 +1067,6 @@ void blk_aio_cancel_async(BlockAIOCB *acb) bdrv_aio_cancel_async(acb); } -int blk_aio_multiwrite(BlockBackend *blk, BlockRequest *reqs, int num_reqs) -{ - int i, ret; - - for (i = 0; i < num_reqs; i++) { - ret = blk_check_request(blk, reqs[i].sector, reqs[i].nb_sectors); - if (ret < 0) { - return ret; - } - } - - return bdrv_aio_multiwrite(blk_bs(blk), reqs, num_reqs); -} - int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf) { if (!blk_is_available(blk)) { diff --git a/block/io.c b/block/io.c index 6e90805019..8a6f470478 100644 --- a/block/io.c +++ b/block/io.c @@ -1878,200 +1878,6 @@ BlockAIOCB *bdrv_aio_write_zeroes(BlockDriverState *bs, cb, opaque, true); } - -typedef struct MultiwriteCB { - int error; - int num_requests; - int num_callbacks; - struct { - BlockCompletionFunc *cb; - void *opaque; - QEMUIOVector *free_qiov; - } callbacks[]; -} MultiwriteCB; - -static void multiwrite_user_cb(MultiwriteCB *mcb) -{ - int i; - - for (i = 0; i < mcb->num_callbacks; i++) { - mcb->callbacks[i].cb(mcb->callbacks[i].opaque, mcb->error); - if (mcb->callbacks[i].free_qiov) { - qemu_iovec_destroy(mcb->callbacks[i].free_qiov); - } - g_free(mcb->callbacks[i].free_qiov); - } -} - -static void multiwrite_cb(void *opaque, int ret) -{ - MultiwriteCB *mcb = opaque; - - trace_multiwrite_cb(mcb, ret); - - if (ret < 0 && !mcb->error) { - mcb->error = ret; - } - - mcb->num_requests--; - if (mcb->num_requests == 0) { - multiwrite_user_cb(mcb); - g_free(mcb); - } -} - -static int multiwrite_req_compare(const void *a, const void *b) -{ - const BlockRequest *req1 = a, *req2 = b; - - /* - * Note that we can't simply subtract req2->sector from req1->sector - * here as that could overflow the return value. - */ - if (req1->sector > req2->sector) { - return 1; - } else if (req1->sector < req2->sector) { - return -1; - } else { - return 0; - } -} - -/* - * Takes a bunch of requests and tries to merge them. Returns the number of - * requests that remain after merging. - */ -static int multiwrite_merge(BlockDriverState *bs, BlockRequest *reqs, - int num_reqs, MultiwriteCB *mcb) -{ - int i, outidx; - - // Sort requests by start sector - qsort(reqs, num_reqs, sizeof(*reqs), &multiwrite_req_compare); - - // Check if adjacent requests touch the same clusters. If so, combine them, - // filling up gaps with zero sectors. - outidx = 0; - for (i = 1; i < num_reqs; i++) { - int merge = 0; - int64_t oldreq_last = reqs[outidx].sector + reqs[outidx].nb_sectors; - - // Handle exactly sequential writes and overlapping writes. - if (reqs[i].sector <= oldreq_last) { - merge = 1; - } - - if (reqs[outidx].qiov->niov + reqs[i].qiov->niov + 1 > - bs->bl.max_iov) { - merge = 0; - } - - if (bs->bl.max_transfer_length && reqs[outidx].nb_sectors + - reqs[i].nb_sectors > bs->bl.max_transfer_length) { - merge = 0; - } - - if (merge) { - size_t size; - QEMUIOVector *qiov = g_malloc0(sizeof(*qiov)); - qemu_iovec_init(qiov, - reqs[outidx].qiov->niov + reqs[i].qiov->niov + 1); - - // Add the first request to the merged one. If the requests are - // overlapping, drop the last sectors of the first request. - size = (reqs[i].sector - reqs[outidx].sector) << 9; - qemu_iovec_concat(qiov, reqs[outidx].qiov, 0, size); - - // We should need to add any zeros between the two requests - assert (reqs[i].sector <= oldreq_last); - - // Add the second request - qemu_iovec_concat(qiov, reqs[i].qiov, 0, reqs[i].qiov->size); - - // Add tail of first request, if necessary - if (qiov->size < reqs[outidx].qiov->size) { - qemu_iovec_concat(qiov, reqs[outidx].qiov, qiov->size, - reqs[outidx].qiov->size - qiov->size); - } - - reqs[outidx].nb_sectors = qiov->size >> 9; - reqs[outidx].qiov = qiov; - - mcb->callbacks[i].free_qiov = reqs[outidx].qiov; - } else { - outidx++; - reqs[outidx].sector = reqs[i].sector; - reqs[outidx].nb_sectors = reqs[i].nb_sectors; - reqs[outidx].qiov = reqs[i].qiov; - } - } - - if (bs->blk) { - block_acct_merge_done(blk_get_stats(bs->blk), BLOCK_ACCT_WRITE, - num_reqs - outidx - 1); - } - - return outidx + 1; -} - -/* - * Submit multiple AIO write requests at once. - * - * On success, the function returns 0 and all requests in the reqs array have - * been submitted. In error case this function returns -1, and any of the - * requests may or may not be submitted yet. In particular, this means that the - * callback will be called for some of the requests, for others it won't. The - * caller must check the error field of the BlockRequest to wait for the right - * callbacks (if error != 0, no callback will be called). - * - * The implementation may modify the contents of the reqs array, e.g. to merge - * requests. However, the fields opaque and error are left unmodified as they - * are used to signal failure for a single request to the caller. - */ -int bdrv_aio_multiwrite(BlockDriverState *bs, BlockRequest *reqs, int num_reqs) -{ - MultiwriteCB *mcb; - int i; - - /* don't submit writes if we don't have a medium */ - if (bs->drv == NULL) { - for (i = 0; i < num_reqs; i++) { - reqs[i].error = -ENOMEDIUM; - } - return -1; - } - - if (num_reqs == 0) { - return 0; - } - - // Create MultiwriteCB structure - mcb = g_malloc0(sizeof(*mcb) + num_reqs * sizeof(*mcb->callbacks)); - mcb->num_requests = 0; - mcb->num_callbacks = num_reqs; - - for (i = 0; i < num_reqs; i++) { - mcb->callbacks[i].cb = reqs[i].cb; - mcb->callbacks[i].opaque = reqs[i].opaque; - } - - // Check for mergable requests - num_reqs = multiwrite_merge(bs, reqs, num_reqs, mcb); - - trace_bdrv_aio_multiwrite(mcb, mcb->num_callbacks, num_reqs); - - /* Run the aio requests. */ - mcb->num_requests = num_reqs; - for (i = 0; i < num_reqs; i++) { - bdrv_co_aio_rw_vector(bs, reqs[i].sector, reqs[i].qiov, - reqs[i].nb_sectors, reqs[i].flags, - multiwrite_cb, mcb, - true); - } - - return 0; -} - void bdrv_aio_cancel(BlockAIOCB *acb) { qemu_aio_ref(acb); diff --git a/include/block/block.h b/include/block/block.h index 2c5c280a42..d1f938064a 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -329,7 +329,7 @@ void bdrv_aio_cancel(BlockAIOCB *acb); void bdrv_aio_cancel_async(BlockAIOCB *acb); typedef struct BlockRequest { - /* Fields to be filled by multiwrite caller */ + /* Fields to be filled by caller */ union { struct { int64_t sector; @@ -345,13 +345,10 @@ typedef struct BlockRequest { BlockCompletionFunc *cb; void *opaque; - /* Filled by multiwrite implementation */ + /* Filled by block layer */ int error; } BlockRequest; -int bdrv_aio_multiwrite(BlockDriverState *bs, BlockRequest *reqs, - int num_reqs); - /* sg packet commands */ int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf); BlockAIOCB *bdrv_aio_ioctl(BlockDriverState *bs, diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index dd9c8ca4e0..79f39b889e 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -138,7 +138,6 @@ BlockAIOCB *blk_aio_discard(BlockBackend *blk, BlockCompletionFunc *cb, void *opaque); void blk_aio_cancel(BlockAIOCB *acb); void blk_aio_cancel_async(BlockAIOCB *acb); -int blk_aio_multiwrite(BlockBackend *blk, BlockRequest *reqs, int num_reqs); int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf); BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf, BlockCompletionFunc *cb, void *opaque); diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 4a00bc604d..22f2ecf6e5 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -574,49 +574,6 @@ static int do_aio_writev(BlockBackend *blk, QEMUIOVector *qiov, return async_ret < 0 ? async_ret : 1; } -struct multiwrite_async_ret { - int num_done; - int error; -}; - -static void multiwrite_cb(void *opaque, int ret) -{ - struct multiwrite_async_ret *async_ret = opaque; - - async_ret->num_done++; - if (ret < 0) { - async_ret->error = ret; - } -} - -static int do_aio_multiwrite(BlockBackend *blk, BlockRequest* reqs, - int num_reqs, int *total) -{ - int i, ret; - struct multiwrite_async_ret async_ret = { - .num_done = 0, - .error = 0, - }; - - *total = 0; - for (i = 0; i < num_reqs; i++) { - reqs[i].cb = multiwrite_cb; - reqs[i].opaque = &async_ret; - *total += reqs[i].qiov->size; - } - - ret = blk_aio_multiwrite(blk, reqs, num_reqs); - if (ret < 0) { - return ret; - } - - while (async_ret.num_done < num_reqs) { - main_loop_wait(false); - } - - return async_ret.error < 0 ? async_ret.error : 1; -} - static void read_help(void) { printf( @@ -1211,165 +1168,6 @@ out: return 0; } -static void multiwrite_help(void) -{ - printf( -"\n" -" writes a range of bytes from the given offset source from multiple buffers,\n" -" in a batch of requests that may be merged by qemu\n" -"\n" -" Example:\n" -" 'multiwrite 512 1k 1k ; 4k 1k'\n" -" writes 2 kB at 512 bytes and 1 kB at 4 kB into the open file\n" -"\n" -" Writes into a segment of the currently open file, using a buffer\n" -" filled with a set pattern (0xcdcdcdcd). The pattern byte is increased\n" -" by one for each request contained in the multiwrite command.\n" -" -P, -- use different pattern to fill file\n" -" -C, -- report statistics in a machine parsable format\n" -" -q, -- quiet mode, do not show I/O statistics\n" -"\n"); -} - -static int multiwrite_f(BlockBackend *blk, int argc, char **argv); - -static const cmdinfo_t multiwrite_cmd = { - .name = "multiwrite", - .cfunc = multiwrite_f, - .argmin = 2, - .argmax = -1, - .args = "[-Cq] [-P pattern ] off len [len..] [; off len [len..]..]", - .oneline = "issues multiple write requests at once", - .help = multiwrite_help, -}; - -static int multiwrite_f(BlockBackend *blk, int argc, char **argv) -{ - struct timeval t1, t2; - bool Cflag = false, qflag = false; - int c, cnt; - char **buf; - int64_t offset, first_offset = 0; - /* Some compilers get confused and warn if this is not initialized. */ - int total = 0; - int nr_iov; - int nr_reqs; - int pattern = 0xcd; - QEMUIOVector *qiovs; - int i; - BlockRequest *reqs; - - while ((c = getopt(argc, argv, "CqP:")) != -1) { - switch (c) { - case 'C': - Cflag = true; - break; - case 'q': - qflag = true; - break; - case 'P': - pattern = parse_pattern(optarg); - if (pattern < 0) { - return 0; - } - break; - default: - return qemuio_command_usage(&writev_cmd); - } - } - - if (optind > argc - 2) { - return qemuio_command_usage(&writev_cmd); - } - - nr_reqs = 1; - for (i = optind; i < argc; i++) { - if (!strcmp(argv[i], ";")) { - nr_reqs++; - } - } - - reqs = g_new0(BlockRequest, nr_reqs); - buf = g_new0(char *, nr_reqs); - qiovs = g_new(QEMUIOVector, nr_reqs); - - for (i = 0; i < nr_reqs && optind < argc; i++) { - int j; - - /* Read the offset of the request */ - offset = cvtnum(argv[optind]); - if (offset < 0) { - print_cvtnum_err(offset, argv[optind]); - goto out; - } - optind++; - - if (offset & 0x1ff) { - printf("offset %lld is not sector aligned\n", - (long long)offset); - goto out; - } - - if (i == 0) { - first_offset = offset; - } - - /* Read lengths for qiov entries */ - for (j = optind; j < argc; j++) { - if (!strcmp(argv[j], ";")) { - break; - } - } - - nr_iov = j - optind; - - /* Build request */ - buf[i] = create_iovec(blk, &qiovs[i], &argv[optind], nr_iov, pattern); - if (buf[i] == NULL) { - goto out; - } - - reqs[i].qiov = &qiovs[i]; - reqs[i].sector = offset >> 9; - reqs[i].nb_sectors = reqs[i].qiov->size >> 9; - - optind = j + 1; - - pattern++; - } - - /* If there were empty requests at the end, ignore them */ - nr_reqs = i; - - gettimeofday(&t1, NULL); - cnt = do_aio_multiwrite(blk, reqs, nr_reqs, &total); - gettimeofday(&t2, NULL); - - if (cnt < 0) { - printf("aio_multiwrite failed: %s\n", strerror(-cnt)); - goto out; - } - - if (qflag) { - goto out; - } - - /* Finally, report back -- -C gives a parsable format */ - t2 = tsub(t2, t1); - print_report("wrote", &t2, first_offset, total, total, cnt, Cflag); -out: - for (i = 0; i < nr_reqs; i++) { - qemu_io_free(buf[i]); - if (reqs[i].qiov != NULL) { - qemu_iovec_destroy(&qiovs[i]); - } - } - g_free(buf); - g_free(reqs); - g_free(qiovs); - return 0; -} - struct aio_ctx { BlockBackend *blk; QEMUIOVector qiov; @@ -2436,7 +2234,6 @@ static void __attribute((constructor)) init_qemuio_commands(void) qemuio_add_command(&readv_cmd); qemuio_add_command(&write_cmd); qemuio_add_command(&writev_cmd); - qemuio_add_command(&multiwrite_cmd); qemuio_add_command(&aio_read_cmd); qemuio_add_command(&aio_write_cmd); qemuio_add_command(&aio_flush_cmd); diff --git a/tests/qemu-iotests/100 b/tests/qemu-iotests/100 deleted file mode 100755 index e66db07982..0000000000 --- a/tests/qemu-iotests/100 +++ /dev/null @@ -1,152 +0,0 @@ -#!/bin/bash -# -# Test simple read/write using plain bdrv_read/bdrv_write -# -# Copyright (C) 2014 Red Hat, Inc. -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . -# - -# creator -owner=stefanha@redhat.com - -seq=`basename $0` -echo "QA output created by $seq" - -here=`pwd` -status=1 # failure is the default! - -_cleanup() -{ - _cleanup_test_img -} -trap "_cleanup; exit \$status" 0 1 2 3 15 - -# get standard environment, filters and checks -. ./common.rc -. ./common.filter - -_supported_fmt generic -_supported_proto generic -_supported_os Linux - - -size=128M - -echo -echo "== Single request ==" -_make_test_img $size -$QEMU_IO -c "write -z 0 8k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "multiwrite 0 4k" "$TEST_IMG" | _filter_qemu_io - -echo -echo "== verify pattern ==" -$QEMU_IO -c "read -P 0xcd 0 4k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io - -_cleanup_test_img - -echo -echo "== Sequential requests ==" -_make_test_img $size -$QEMU_IO -c "write -z 0 12k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "multiwrite 0 4k ; 4k 4k" "$TEST_IMG" | _filter_qemu_io - -echo -echo "== verify pattern ==" -$QEMU_IO -c "read -P 0xcd 0 4k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "read -P 0xce 4k 4k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "read -P 0 8k 4k" "$TEST_IMG" | _filter_qemu_io - -_cleanup_test_img - -echo -echo "== Superset overlapping requests ==" -_make_test_img $size -$QEMU_IO -c "write -z 0 8k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "multiwrite 0 4k ; 1k 2k" "$TEST_IMG" | _filter_qemu_io - -echo -echo "== verify pattern ==" -# Order of overlapping in-flight requests is not guaranteed so we cannot verify -# [1k, 3k) since it could have either pattern 0xcd or 0xce. -$QEMU_IO -c "read -P 0xcd 0 1k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "read -P 0xcd 3k 1k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io - -_cleanup_test_img - -echo -echo "== Subset overlapping requests ==" -_make_test_img $size -$QEMU_IO -c "write -z 0 8k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "multiwrite 1k 2k ; 0k 4k" "$TEST_IMG" | _filter_qemu_io - -echo -echo "== verify pattern ==" -# Order of overlapping in-flight requests is not guaranteed so we cannot verify -# [1k, 3k) since it could have either pattern 0xcd or 0xce. -$QEMU_IO -c "read -P 0xce 0 1k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "read -P 0xce 3k 1k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io - -_cleanup_test_img - -echo -echo "== Head overlapping requests ==" -_make_test_img $size -$QEMU_IO -c "write -z 0 8k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "multiwrite 0k 2k ; 0k 4k" "$TEST_IMG" | _filter_qemu_io - -echo -echo "== verify pattern ==" -# Order of overlapping in-flight requests is not guaranteed so we cannot verify -# [0k, 2k) since it could have either pattern 0xcd or 0xce. -$QEMU_IO -c "read -P 0xce 2k 2k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io - -_cleanup_test_img - -echo -echo "== Tail overlapping requests ==" -_make_test_img $size -$QEMU_IO -c "write -z 0 8k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "multiwrite 2k 2k ; 0k 4k" "$TEST_IMG" | _filter_qemu_io - -echo -echo "== verify pattern ==" -# Order of overlapping in-flight requests is not guaranteed so we cannot verify -# [2k, 4k) since it could have either pattern 0xcd or 0xce. -$QEMU_IO -c "read -P 0xce 0k 2k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io - -_cleanup_test_img - -echo -echo "== Disjoint requests ==" -_make_test_img $size -$QEMU_IO -c "write -z 0 72k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "multiwrite 0 4k ; 64k 4k" "$TEST_IMG" | _filter_qemu_io - -echo -echo "== verify pattern ==" -$QEMU_IO -c "read -P 0xcd 0 4k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "read -P 0 4k 60k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "read -P 0xce 64k 4k" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "read -P 0 68k 4k" "$TEST_IMG" | _filter_qemu_io - -# success, all done -echo "*** done" -rm -f $seq.full -status=0 diff --git a/tests/qemu-iotests/100.out b/tests/qemu-iotests/100.out deleted file mode 100644 index a44cae40db..0000000000 --- a/tests/qemu-iotests/100.out +++ /dev/null @@ -1,103 +0,0 @@ -QA output created by 100 - -== Single request == -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 -wrote 8192/8192 bytes at offset 0 -8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -wrote 4096/4096 bytes at offset 0 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== verify pattern == -read 4096/4096 bytes at offset 0 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 4096/4096 bytes at offset 4096 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== Sequential requests == -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 -wrote 12288/12288 bytes at offset 0 -12 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -wrote 8192/8192 bytes at offset 0 -8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== verify pattern == -read 4096/4096 bytes at offset 0 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 4096/4096 bytes at offset 4096 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 4096/4096 bytes at offset 8192 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== Superset overlapping requests == -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 -wrote 8192/8192 bytes at offset 0 -8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -wrote 6144/6144 bytes at offset 0 -6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== verify pattern == -read 1024/1024 bytes at offset 0 -1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 1024/1024 bytes at offset 3072 -1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 4096/4096 bytes at offset 4096 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== Subset overlapping requests == -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 -wrote 8192/8192 bytes at offset 0 -8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -wrote 6144/6144 bytes at offset 1024 -6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== verify pattern == -read 1024/1024 bytes at offset 0 -1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 1024/1024 bytes at offset 3072 -1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 4096/4096 bytes at offset 4096 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== Head overlapping requests == -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 -wrote 8192/8192 bytes at offset 0 -8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -wrote 6144/6144 bytes at offset 0 -6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== verify pattern == -read 2048/2048 bytes at offset 2048 -2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 4096/4096 bytes at offset 4096 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== Tail overlapping requests == -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 -wrote 8192/8192 bytes at offset 0 -8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -wrote 6144/6144 bytes at offset 2048 -6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== verify pattern == -read 2048/2048 bytes at offset 0 -2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 4096/4096 bytes at offset 4096 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== Disjoint requests == -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 -wrote 73728/73728 bytes at offset 0 -72 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -wrote 8192/8192 bytes at offset 0 -8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== verify pattern == -read 4096/4096 bytes at offset 0 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 61440/61440 bytes at offset 4096 -60 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 4096/4096 bytes at offset 65536 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 4096/4096 bytes at offset 69632 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -*** done diff --git a/tests/qemu-iotests/136 b/tests/qemu-iotests/136 index e8c6937fc9..996265e626 100644 --- a/tests/qemu-iotests/136 +++ b/tests/qemu-iotests/136 @@ -248,14 +248,6 @@ sector = "%d" if failed_wr_ops > 0: highest_offset = max(highest_offset, bad_offset + 512) - for i in range(wr_merged): - first = i * wr_size * 2 - second = first + wr_size - ops.append("multiwrite %d %d ; %d %d" % - (first, wr_size, second, wr_size)) - - highest_offset = max(highest_offset, wr_merged * wr_size * 2) - # Now perform all operations for op in ops: self.vm.hmp_qemu_io("drive0", op) @@ -309,19 +301,15 @@ sector = "%d" def test_flush(self): self.do_test_stats(flush_ops = 8) - def test_merged(self): - for i in range(5): - self.do_test_stats(wr_merged = i * 3) - def test_all(self): # rd_size, rd_ops, wr_size, wr_ops, flush_ops # invalid_rd_ops, invalid_wr_ops, # failed_rd_ops, failed_wr_ops # wr_merged - test_values = [[512, 1, 512, 1, 1, 4, 7, 5, 2, 1], - [65536, 1, 2048, 12, 7, 7, 5, 2, 5, 5], - [32768, 9, 8192, 1, 4, 3, 2, 4, 6, 4], - [16384, 11, 3584, 16, 9, 8, 6, 7, 3, 4]] + test_values = [[512, 1, 512, 1, 1, 4, 7, 5, 2, 0], + [65536, 1, 2048, 12, 7, 7, 5, 2, 5, 0], + [32768, 9, 8192, 1, 4, 3, 2, 4, 6, 0], + [16384, 11, 3584, 16, 9, 8, 6, 7, 3, 0]] for i in test_values: self.do_test_stats(*i) diff --git a/tests/qemu-iotests/136.out b/tests/qemu-iotests/136.out index 0a5e9583a4..cfa5c0d0e6 100644 --- a/tests/qemu-iotests/136.out +++ b/tests/qemu-iotests/136.out @@ -1,5 +1,5 @@ -........................................ +................................... ---------------------------------------------------------------------- -Ran 40 tests +Ran 35 tests OK diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 822953b6fa..60676738d0 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -106,7 +106,7 @@ 097 rw auto backing 098 rw auto backing quick 099 rw auto quick -100 rw auto quick +# 100 was removed, do not reuse 101 rw auto quick 102 rw auto quick 103 rw auto quick diff --git a/trace-events b/trace-events index e35b80e980..b53c3541a3 100644 --- a/trace-events +++ b/trace-events @@ -62,8 +62,6 @@ bdrv_open_common(void *bs, const char *filename, int flags, const char *format_n bdrv_lock_medium(void *bs, bool locked) "bs %p locked %d" # block/io.c -multiwrite_cb(void *mcb, int ret) "mcb %p ret %d" -bdrv_aio_multiwrite(void *mcb, int num_callbacks, int num_reqs) "mcb %p num_callbacks %d num_reqs %d" bdrv_aio_discard(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p" bdrv_aio_flush(void *bs, void *opaque) "bs %p opaque %p" bdrv_aio_readv(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p" -- cgit v1.2.3-55-g7522 From dde33812a83b9c55b180a85411bfc4d6c39e8b11 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 29 Feb 2016 10:50:38 +0100 Subject: block: Add bdrv_has_blk() In many cases we just want to know whether a BDS has at least one BB attached, without needing to know the exact BB that is attached. In contrast to bs->blk, this is still a valid question when more than one BB can be attached, so just answer it by checking the parents list. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- block/block-backend.c | 17 +++++++++++++++++ include/sysemu/block-backend.h | 1 + 2 files changed, 18 insertions(+) (limited to 'include') diff --git a/block/block-backend.c b/block/block-backend.c index 8d6fc77b26..9dcac9792f 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -394,6 +394,23 @@ BlockDriverState *blk_bs(BlockBackend *blk) return blk->root ? blk->root->bs : NULL; } +/* + * Returns true if @bs has an associated BlockBackend. + */ +bool bdrv_has_blk(BlockDriverState *bs) +{ + BdrvChild *child; + QLIST_FOREACH(child, &bs->parents, next_parent) { + if (child->role == &child_root) { + assert(bs->blk); + return true; + } + } + + assert(!bs->blk); + return false; +} + /* * Return @blk's DriveInfo if any, else null. */ diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 79f39b889e..44a222d884 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -99,6 +99,7 @@ BlockBackend *blk_by_public(BlockBackendPublic *public); BlockDriverState *blk_bs(BlockBackend *blk); void blk_remove_bs(BlockBackend *blk); void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs); +bool bdrv_has_blk(BlockDriverState *bs); void blk_set_allow_write_beyond_eof(BlockBackend *blk, bool allow); void blk_iostatus_enable(BlockBackend *blk); -- cgit v1.2.3-55-g7522 From 7c8eece45b10fc9b716850345118ed6fa8d17887 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 22 Mar 2016 18:58:50 +0100 Subject: block: Avoid bs->blk in bdrv_next() We need to introduce a separate BdrvNextIterator struct that can keep more state than just the current BDS in order to avoid using the bs->blk pointer. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- block.c | 40 +++++++---------------- block/block-backend.c | 72 +++++++++++++++++++++++++++++------------- block/io.c | 13 ++++---- block/snapshot.c | 30 +++++++++++------- blockdev.c | 3 +- include/block/block.h | 3 +- include/sysemu/block-backend.h | 1 - migration/block.c | 4 ++- monitor.c | 6 ++-- qmp.c | 5 ++- 10 files changed, 102 insertions(+), 75 deletions(-) (limited to 'include') diff --git a/block.c b/block.c index fd4cf81096..91bf43188f 100644 --- a/block.c +++ b/block.c @@ -2870,25 +2870,6 @@ BlockDriverState *bdrv_next_node(BlockDriverState *bs) return QTAILQ_NEXT(bs, node_list); } -/* Iterates over all top-level BlockDriverStates, i.e. BDSs that are owned by - * the monitor or attached to a BlockBackend */ -BlockDriverState *bdrv_next(BlockDriverState *bs) -{ - if (!bs || bs->blk) { - bs = blk_next_root_bs(bs); - if (bs) { - return bs; - } - } - - /* Ignore all BDSs that are attached to a BlockBackend here; they have been - * handled by the above block already */ - do { - bs = bdrv_next_monitor_owned(bs); - } while (bs && bs->blk); - return bs; -} - const char *bdrv_get_node_name(const BlockDriverState *bs) { return bs->node_name; @@ -3220,10 +3201,11 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp) void bdrv_invalidate_cache_all(Error **errp) { - BlockDriverState *bs = NULL; + BlockDriverState *bs; Error *local_err = NULL; + BdrvNextIterator *it = NULL; - while ((bs = bdrv_next(bs)) != NULL) { + while ((it = bdrv_next(it, &bs)) != NULL) { AioContext *aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); @@ -3265,10 +3247,11 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs, int bdrv_inactivate_all(void) { BlockDriverState *bs = NULL; + BdrvNextIterator *it = NULL; int ret = 0; int pass; - while ((bs = bdrv_next(bs)) != NULL) { + while ((it = bdrv_next(it, &bs)) != NULL) { aio_context_acquire(bdrv_get_aio_context(bs)); } @@ -3277,8 +3260,8 @@ int bdrv_inactivate_all(void) * the second pass sets the BDRV_O_INACTIVE flag so that no further write * is allowed. */ for (pass = 0; pass < 2; pass++) { - bs = NULL; - while ((bs = bdrv_next(bs)) != NULL) { + it = NULL; + while ((it = bdrv_next(it, &bs)) != NULL) { ret = bdrv_inactivate_recurse(bs, pass); if (ret < 0) { goto out; @@ -3287,8 +3270,8 @@ int bdrv_inactivate_all(void) } out: - bs = NULL; - while ((bs = bdrv_next(bs)) != NULL) { + it = NULL; + while ((it = bdrv_next(it, &bs)) != NULL) { aio_context_release(bdrv_get_aio_context(bs)); } @@ -3781,10 +3764,11 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs, */ bool bdrv_is_first_non_filter(BlockDriverState *candidate) { - BlockDriverState *bs = NULL; + BlockDriverState *bs; + BdrvNextIterator *it = NULL; /* walk down the bs forest recursively */ - while ((bs = bdrv_next(bs)) != NULL) { + while ((it = bdrv_next(it, &bs)) != NULL) { bool perm; /* try to recurse in this top level bs */ diff --git a/block/block-backend.c b/block/block-backend.c index 9dcac9792f..7f2eeb0559 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -75,6 +75,7 @@ static const AIOCBInfo block_backend_aiocb_info = { }; static void drive_info_del(DriveInfo *dinfo); +static BlockBackend *bdrv_first_blk(BlockDriverState *bs); /* All BlockBackends */ static QTAILQ_HEAD(, BlockBackend) block_backends = @@ -286,28 +287,50 @@ BlockBackend *blk_next(BlockBackend *blk) : QTAILQ_FIRST(&monitor_block_backends); } -/* - * Iterates over all BlockDriverStates which are attached to a BlockBackend. - * This function is for use by bdrv_next(). - * - * @bs must be NULL or a BDS that is attached to a BB. - */ -BlockDriverState *blk_next_root_bs(BlockDriverState *bs) -{ +struct BdrvNextIterator { + enum { + BDRV_NEXT_BACKEND_ROOTS, + BDRV_NEXT_MONITOR_OWNED, + } phase; BlockBackend *blk; + BlockDriverState *bs; +}; - if (bs) { - assert(bs->blk); - blk = bs->blk; - } else { - blk = NULL; +/* Iterates over all top-level BlockDriverStates, i.e. BDSs that are owned by + * the monitor or attached to a BlockBackend */ +BdrvNextIterator *bdrv_next(BdrvNextIterator *it, BlockDriverState **bs) +{ + if (!it) { + it = g_new(BdrvNextIterator, 1); + *it = (BdrvNextIterator) { + .phase = BDRV_NEXT_BACKEND_ROOTS, + }; + } + + /* First, return all root nodes of BlockBackends. In order to avoid + * returning a BDS twice when multiple BBs refer to it, we only return it + * if the BB is the first one in the parent list of the BDS. */ + if (it->phase == BDRV_NEXT_BACKEND_ROOTS) { + do { + it->blk = blk_all_next(it->blk); + *bs = it->blk ? blk_bs(it->blk) : NULL; + } while (it->blk && (*bs == NULL || bdrv_first_blk(*bs) != it->blk)); + + if (*bs) { + return it; + } + it->phase = BDRV_NEXT_MONITOR_OWNED; } + /* Then return the monitor-owned BDSes without a BB attached. Ignore all + * BDSes that are attached to a BlockBackend here; they have been handled + * by the above block already */ do { - blk = blk_all_next(blk); - } while (blk && !blk->root); + it->bs = bdrv_next_monitor_owned(it->bs); + *bs = it->bs; + } while (*bs && bdrv_has_blk(*bs)); - return blk ? blk->root->bs : NULL; + return *bs ? it : NULL; } /* @@ -394,21 +417,26 @@ BlockDriverState *blk_bs(BlockBackend *blk) return blk->root ? blk->root->bs : NULL; } -/* - * Returns true if @bs has an associated BlockBackend. - */ -bool bdrv_has_blk(BlockDriverState *bs) +static BlockBackend *bdrv_first_blk(BlockDriverState *bs) { BdrvChild *child; QLIST_FOREACH(child, &bs->parents, next_parent) { if (child->role == &child_root) { assert(bs->blk); - return true; + return child->opaque; } } assert(!bs->blk); - return false; + return NULL; +} + +/* + * Returns true if @bs has an associated BlockBackend. + */ +bool bdrv_has_blk(BlockDriverState *bs) +{ + return bdrv_first_blk(bs) != NULL; } /* diff --git a/block/io.c b/block/io.c index 8a6f470478..60a6bd8bdb 100644 --- a/block/io.c +++ b/block/io.c @@ -270,10 +270,11 @@ void bdrv_drain_all(void) { /* Always run first iteration so any pending completion BHs run */ bool busy = true; - BlockDriverState *bs = NULL; + BlockDriverState *bs; + BdrvNextIterator *it = NULL; GSList *aio_ctxs = NULL, *ctx; - while ((bs = bdrv_next(bs))) { + while ((it = bdrv_next(it, &bs))) { AioContext *aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); @@ -301,10 +302,10 @@ void bdrv_drain_all(void) for (ctx = aio_ctxs; ctx != NULL; ctx = ctx->next) { AioContext *aio_context = ctx->data; - bs = NULL; + it = NULL; aio_context_acquire(aio_context); - while ((bs = bdrv_next(bs))) { + while ((it = bdrv_next(it, &bs))) { if (aio_context == bdrv_get_aio_context(bs)) { if (bdrv_requests_pending(bs)) { busy = true; @@ -317,8 +318,8 @@ void bdrv_drain_all(void) } } - bs = NULL; - while ((bs = bdrv_next(bs))) { + it = NULL; + while ((it = bdrv_next(it, &bs))) { AioContext *aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); diff --git a/block/snapshot.c b/block/snapshot.c index e9d721df68..3917ec5c91 100644 --- a/block/snapshot.c +++ b/block/snapshot.c @@ -373,9 +373,10 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs, bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs) { bool ok = true; - BlockDriverState *bs = NULL; + BlockDriverState *bs; + BdrvNextIterator *it = NULL; - while (ok && (bs = bdrv_next(bs))) { + while (ok && (it = bdrv_next(it, &bs))) { AioContext *ctx = bdrv_get_aio_context(bs); aio_context_acquire(ctx); @@ -393,10 +394,11 @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs, Error **err) { int ret = 0; - BlockDriverState *bs = NULL; + BlockDriverState *bs; + BdrvNextIterator *it = NULL; QEMUSnapshotInfo sn1, *snapshot = &sn1; - while (ret == 0 && (bs = bdrv_next(bs))) { + while (ret == 0 && (it = bdrv_next(it, &bs))) { AioContext *ctx = bdrv_get_aio_context(bs); aio_context_acquire(ctx); @@ -415,9 +417,10 @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs, int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs) { int err = 0; - BlockDriverState *bs = NULL; + BlockDriverState *bs; + BdrvNextIterator *it = NULL; - while (err == 0 && (bs = bdrv_next(bs))) { + while (err == 0 && (it = bdrv_next(it, &bs))) { AioContext *ctx = bdrv_get_aio_context(bs); aio_context_acquire(ctx); @@ -435,9 +438,10 @@ int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs) { QEMUSnapshotInfo sn; int err = 0; - BlockDriverState *bs = NULL; + BlockDriverState *bs; + BdrvNextIterator *it = NULL; - while (err == 0 && (bs = bdrv_next(bs))) { + while (err == 0 && (it = bdrv_next(it, &bs))) { AioContext *ctx = bdrv_get_aio_context(bs); aio_context_acquire(ctx); @@ -457,9 +461,10 @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, BlockDriverState **first_bad_bs) { int err = 0; - BlockDriverState *bs = NULL; + BlockDriverState *bs; + BdrvNextIterator *it = NULL; - while (err == 0 && (bs = bdrv_next(bs))) { + while (err == 0 && (it = bdrv_next(it, &bs))) { AioContext *ctx = bdrv_get_aio_context(bs); aio_context_acquire(ctx); @@ -480,9 +485,10 @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, BlockDriverState *bdrv_all_find_vmstate_bs(void) { bool not_found = true; - BlockDriverState *bs = NULL; + BlockDriverState *bs; + BdrvNextIterator *it = NULL; - while (not_found && (bs = bdrv_next(bs))) { + while (not_found && (it = bdrv_next(it, &bs))) { AioContext *ctx = bdrv_get_aio_context(bs); aio_context_acquire(ctx); diff --git a/blockdev.c b/blockdev.c index 03ddd3ac6e..442ca8d19d 100644 --- a/blockdev.c +++ b/blockdev.c @@ -4135,8 +4135,9 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp) { BlockJobInfoList *head = NULL, **p_next = &head; BlockDriverState *bs; + BdrvNextIterator *it = NULL; - for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) { + while ((it = bdrv_next(it, &bs))) { AioContext *aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); diff --git a/include/block/block.h b/include/block/block.h index d1f938064a..a8c15e36e7 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -17,6 +17,7 @@ typedef struct BlockJob BlockJob; typedef struct BdrvChild BdrvChild; typedef struct BdrvChildRole BdrvChildRole; typedef struct BlockJobTxn BlockJobTxn; +typedef struct BdrvNextIterator BdrvNextIterator; typedef struct BlockDriverInfo { /* in bytes, 0 if irrelevant */ @@ -401,7 +402,7 @@ BlockDriverState *bdrv_lookup_bs(const char *device, Error **errp); bool bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base); BlockDriverState *bdrv_next_node(BlockDriverState *bs); -BlockDriverState *bdrv_next(BlockDriverState *bs); +BdrvNextIterator *bdrv_next(BdrvNextIterator *it, BlockDriverState **bs); BlockDriverState *bdrv_next_monitor_owned(BlockDriverState *bs); int bdrv_is_encrypted(BlockDriverState *bs); int bdrv_key_required(BlockDriverState *bs); diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 44a222d884..68d92b556e 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -89,7 +89,6 @@ void blk_remove_all_bs(void); const char *blk_name(BlockBackend *blk); BlockBackend *blk_by_name(const char *name); BlockBackend *blk_next(BlockBackend *blk); -BlockDriverState *blk_next_root_bs(BlockDriverState *bs); bool monitor_add_blk(BlockBackend *blk, const char *name, Error **errp); void monitor_remove_blk(BlockBackend *blk); diff --git a/migration/block.c b/migration/block.c index 1743317288..a7a76a0fb9 100644 --- a/migration/block.c +++ b/migration/block.c @@ -383,6 +383,7 @@ static void init_blk_migration(QEMUFile *f) BlockDriverState *bs; BlkMigDevState *bmds; int64_t sectors; + BdrvNextIterator *it = NULL; block_mig_state.submitted = 0; block_mig_state.read_done = 0; @@ -392,7 +393,8 @@ static void init_blk_migration(QEMUFile *f) block_mig_state.bulk_completed = 0; block_mig_state.zero_blocks = migrate_zero_blocks(); - for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) { + + while ((it = bdrv_next(it, &bs))) { if (bdrv_is_read_only(bs)) { continue; } diff --git a/monitor.c b/monitor.c index d1c193013e..2c87244073 100644 --- a/monitor.c +++ b/monitor.c @@ -3427,11 +3427,13 @@ void host_net_remove_completion(ReadLineState *rs, int nb_args, const char *str) static void vm_completion(ReadLineState *rs, const char *str) { size_t len; - BlockDriverState *bs = NULL; + BlockDriverState *bs; + BdrvNextIterator *it = NULL; len = strlen(str); readline_set_completion_index(rs, len); - while ((bs = bdrv_next(bs))) { + + while ((it = bdrv_next(it, &bs))) { SnapshotInfoList *snapshots, *snapshot; AioContext *ctx = bdrv_get_aio_context(bs); bool ok = false; diff --git a/qmp.c b/qmp.c index e784a67631..8f8ae3a79d 100644 --- a/qmp.c +++ b/qmp.c @@ -181,6 +181,7 @@ void qmp_cont(Error **errp) Error *local_err = NULL; BlockBackend *blk; BlockDriverState *bs; + BdrvNextIterator *it; /* if there is a dump in background, we should wait until the dump * finished */ @@ -199,7 +200,9 @@ void qmp_cont(Error **errp) for (blk = blk_next(NULL); blk; blk = blk_next(blk)) { blk_iostatus_reset(blk); } - for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) { + + it = NULL; + while ((it = bdrv_next(it, &bs))) { bdrv_add_key(bs, NULL, &local_err); if (local_err) { error_propagate(errp, local_err); -- cgit v1.2.3-55-g7522 From 1f0c461b82d5ec2664ca0cfc9548f80da87a8f8a Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 22 Mar 2016 18:38:44 +0100 Subject: block: Remove BlockDriverState.blk This patch removes the remaining users of bs->blk, which will allow us to have multiple BBs on top of a single BDS. In the meantime, all checks that are currently in place to prevent the user from creating such setups can be switched to bdrv_has_blk() instead of accessing BDS.blk. Future patches can allow them and e.g. enable users to mirror to a block device that already has a BlockBackend on it. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- block.c | 10 +--------- block/block-backend.c | 8 -------- block/mirror.c | 4 ++-- blockdev.c | 16 ++++++++-------- include/block/block_int.h | 3 +-- 5 files changed, 12 insertions(+), 29 deletions(-) (limited to 'include') diff --git a/block.c b/block.c index 91bf43188f..cbc10f7480 100644 --- a/block.c +++ b/block.c @@ -2226,14 +2226,6 @@ static void change_parent_backing_link(BlockDriverState *from, { BdrvChild *c, *next; - if (from->blk) { - /* FIXME We bypass blk_set_bs(), so we need to make these updates - * manually. The root problem is not in this change function, but the - * existence of BlockDriverState.blk. */ - to->blk = from->blk; - from->blk = NULL; - } - QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) { assert(c->role != &child_backing); c->bs = to; @@ -2875,7 +2867,7 @@ const char *bdrv_get_node_name(const BlockDriverState *bs) return bs->node_name; } -static const char *bdrv_get_parent_name(const BlockDriverState *bs) +const char *bdrv_get_parent_name(const BlockDriverState *bs) { BdrvChild *c; const char *name; diff --git a/block/block-backend.c b/block/block-backend.c index 7f2eeb0559..6928d61de4 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -152,7 +152,6 @@ BlockBackend *blk_new_with_bs(Error **errp) bs = bdrv_new_root(); blk->root = bdrv_root_attach_child(bs, "root", &child_root); blk->root->opaque = blk; - bs->blk = blk; return blk; } @@ -422,12 +421,10 @@ static BlockBackend *bdrv_first_blk(BlockDriverState *bs) BdrvChild *child; QLIST_FOREACH(child, &bs->parents, next_parent) { if (child->role == &child_root) { - assert(bs->blk); return child->opaque; } } - assert(!bs->blk); return NULL; } @@ -495,8 +492,6 @@ BlockBackend *blk_by_public(BlockBackendPublic *public) */ void blk_remove_bs(BlockBackend *blk) { - assert(blk->root->bs->blk == blk); - notifier_list_notify(&blk->remove_bs_notifiers, blk); if (blk->public.throttle_state) { throttle_timers_detach_aio_context(&blk->public.throttle_timers); @@ -504,7 +499,6 @@ void blk_remove_bs(BlockBackend *blk) blk_update_root_state(blk); - blk->root->bs->blk = NULL; bdrv_root_unref_child(blk->root); blk->root = NULL; } @@ -514,11 +508,9 @@ void blk_remove_bs(BlockBackend *blk) */ void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs) { - assert(!blk->root && !bs->blk); bdrv_ref(bs); blk->root = bdrv_root_attach_child(bs, "root", &child_root); blk->root->opaque = blk; - bs->blk = blk; notifier_list_notify(&blk->insert_bs_notifiers, blk); if (blk->public.throttle_state) { diff --git a/block/mirror.c b/block/mirror.c index 71d7a4e357..b9986d8218 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -468,7 +468,7 @@ static void mirror_exit(BlockJob *job, void *opaque) /* This was checked in mirror_start_job(), but meanwhile one of the * nodes could have been newly attached to a BlockBackend. */ - if (to_replace->blk && s->target->blk) { + if (bdrv_has_blk(to_replace) && bdrv_has_blk(s->target)) { error_report("block job: Can't create node with two BlockBackends"); data->ret = -EINVAL; goto out; @@ -831,7 +831,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target, } else { replaced_bs = bs; } - if (replaced_bs->blk && target->blk) { + if (bdrv_has_blk(replaced_bs) && bdrv_has_blk(target)) { error_setg(errp, "Can't create node with two BlockBackends"); return; } diff --git a/blockdev.c b/blockdev.c index 442ca8d19d..b6fa210249 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1776,9 +1776,9 @@ static void external_snapshot_prepare(BlkActionState *common, return; } - if (state->new_bs->blk != NULL) { + if (bdrv_has_blk(state->new_bs)) { error_setg(errp, "The snapshot is already in use by %s", - blk_name(state->new_bs->blk)); + bdrv_get_parent_name(state->new_bs)); return; } @@ -2494,9 +2494,9 @@ void qmp_x_blockdev_insert_medium(const char *device, const char *node_name, return; } - if (bs->blk) { + if (bdrv_has_blk(bs)) { error_setg(errp, "Node '%s' is already in use by '%s'", node_name, - blk_name(bs->blk)); + bdrv_get_parent_name(bs)); return; } @@ -3441,7 +3441,7 @@ static void blockdev_mirror_common(BlockDriverState *bs, if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_MIRROR_TARGET, errp)) { return; } - if (target->blk) { + if (bdrv_has_blk(target)) { error_setg(errp, "Cannot mirror to an attached block device"); return; } @@ -4030,15 +4030,15 @@ void qmp_x_blockdev_del(bool has_id, const char *id, bs = blk_bs(blk); aio_context = blk_get_aio_context(blk); } else { + blk = NULL; bs = bdrv_find_node(node_name); if (!bs) { error_setg(errp, "Cannot find node %s", node_name); return; } - blk = bs->blk; - if (blk) { + if (bdrv_has_blk(bs)) { error_setg(errp, "Node %s is in use by %s", - node_name, blk_name(blk)); + node_name, bdrv_get_parent_name(bs)); return; } aio_context = bdrv_get_aio_context(bs); diff --git a/include/block/block_int.h b/include/block/block_int.h index 80e2da519a..b6f4755725 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -418,8 +418,6 @@ struct BlockDriverState { BlockDriver *drv; /* NULL means no media */ void *opaque; - BlockBackend *blk; /* owning backend, if any */ - AioContext *aio_context; /* event loop used for fd handlers, timers, etc */ /* long-running tasks intended to always use the same AioContext as this * BDS may register themselves in this list to be notified of changes @@ -724,6 +722,7 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs, const BdrvChildRole *child_role); void bdrv_root_unref_child(BdrvChild *child); +const char *bdrv_get_parent_name(const BlockDriverState *bs); void blk_dev_change_media_cb(BlockBackend *blk, bool load); bool blk_dev_has_removable_media(BlockBackend *blk); bool blk_dev_has_tray(BlockBackend *blk); -- cgit v1.2.3-55-g7522