Skip to content

Commit

Permalink
block: Decouple throttling from BlockDriverState
Browse files Browse the repository at this point in the history 
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 <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
  • Loading branch information
@kevmw
kevmw committed May 19, 2016
1 parent bb9aaec commit 7ca7f0f
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 78 deletions.
34 changes: 0 additions & 34 deletions block.c
View file
Expand Up @@ -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"

Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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
}
}

/*
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down
37 changes: 14 additions & 23 deletions block/block-backend.c
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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));
}
}

/*
Expand Down Expand Up @@ -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);
}
}
}

Expand Down Expand Up @@ -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;
}
}

/*
Expand All @@ -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);
}
}

/*
Expand Down
27 changes: 9 additions & 18 deletions blockdev.c
View file
Expand Up @@ -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) {
Expand All @@ -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;
}
Expand All @@ -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);

Expand Down
3 changes: 0 additions & 3 deletions include/block/block_int.h
View file
Expand Up @@ -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)
Expand Down

0 comments on commit 7ca7f0f

Please sign in to comment.