Commit a5614993 authored by Kevin Wolf's avatar Kevin Wolf
Browse files

block: Make sure throttled BDSes always have a BB 



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: default avatarKevin Wolf <kwolf@redhat.com>
Reviewed-by: default avatarEric Blake <eblake@redhat.com>
Reviewed-by: default avatarAlberto Garcia <berto@igalia.com>
Acked-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
parent 8ec4fe0a

block.c

+14 −0
Original line number Diff line number Diff line
@@ -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

    }

}

block/block-backend.c

+3 −0
Original line number Diff line number Diff line
@@ -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);

blockdev.c

+2 −2
Original line number Diff line number Diff line
@@ -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:

tests/test-throttle.c

+8 −3
Original line number Diff line number Diff line
@@ -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);