From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41223) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b3PmT-00057P-Jj for qemu-devel@nongnu.org; Thu, 19 May 2016 11:22:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b3PmN-0008TG-F0 for qemu-devel@nongnu.org; Thu, 19 May 2016 11:22:32 -0400 From: Kevin Wolf Date: Thu, 19 May 2016 17:21:39 +0200 Message-Id: <1463671329-22655-2-git-send-email-kwolf@redhat.com> In-Reply-To: <1463671329-22655-1-git-send-email-kwolf@redhat.com> References: <1463671329-22655-1-git-send-email-kwolf@redhat.com> Subject: [Qemu-devel] [PULL 01/31] block: Make sure throttled BDSes always have a BB List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-block@nongnu.org Cc: kwolf@redhat.com, qemu-devel@nongnu.org It was already true in principle that a throttled BDS always has a BB attached, except that the order of operations while attaching or detaching a BDS to/from a BB wasn't careful enough. This commit breaks graph manipulations while I/O throttling is enabled. It would have been possible to keep things working with some temporary hacks, but quite cumbersome, so it's not worth the hassle. We'll fix things again in a minute. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Alberto Garcia Acked-by: Stefan Hajnoczi --- block.c | 14 ++++++++++++++ block/block-backend.c | 3 +++ blockdev.c | 4 ++-- tests/test-throttle.c | 11 ++++++++--- 4 files changed, 27 insertions(+), 5 deletions(-) diff --git a/block.c b/block.c index 18a497f..a789844 100644 --- a/block.c +++ b/block.c @@ -2259,8 +2259,22 @@ static void swap_feature_fields(BlockDriverState *bs_top, assert(!bs_new->throttle_state); if (bs_top->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 } } diff --git a/block/block-backend.c b/block/block-backend.c index a1e2c7f..53fd9d2 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -420,6 +420,9 @@ 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) { + bdrv_io_limits_disable(blk->root->bs); + } blk->root->bs->blk = NULL; bdrv_root_unref_child(blk->root); diff --git a/blockdev.c b/blockdev.c index 1892b8e..8106ca7 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2570,8 +2570,6 @@ void qmp_blockdev_change_medium(const char *device, const char *filename, goto fail; } - blk_apply_root_state(blk, medium_bs); - bdrv_add_key(medium_bs, NULL, &err); if (err) { error_propagate(errp, err); @@ -2596,6 +2594,8 @@ void qmp_blockdev_change_medium(const char *device, const char *filename, goto fail; } + blk_apply_root_state(blk, medium_bs); + qmp_blockdev_close_tray(device, errp); fail: diff --git a/tests/test-throttle.c b/tests/test-throttle.c index 744a524..77b95d6 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -574,11 +574,16 @@ static void test_accounting(void) static void test_groups(void) { ThrottleConfig cfg1, cfg2; + BlockBackend *blk1, *blk2, *blk3; BlockDriverState *bdrv1, *bdrv2, *bdrv3; - bdrv1 = bdrv_new(); - bdrv2 = bdrv_new(); - bdrv3 = bdrv_new(); + 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); -- 1.8.3.1