Commit 191b5fbf authored by Peter Maydell's avatar Peter Maydell
Browse files

Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging 



Pull request

The following disk I/O throttling fixes solve recent bugs.

# gpg: Signature made Tue 14 Nov 2017 10:37:12 GMT
# gpg:                using RSA key 0x9CA4ABB381AB73C8
# gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>"
# gpg:                 aka "Stefan Hajnoczi <stefanha@gmail.com>"
# Primary key fingerprint: 8695 A8BF D3F9 7CDA AC35  775A 9CA4 ABB3 81AB 73C8

* remotes/stefanha/tags/block-pull-request:
  qemu-iotests: Test I/O limits with removable media
  block: Leave valid throttle timers when removing a BDS from a backend
  block: Check for inserted BlockDriverState in blk_io_limits_disable()
  throttle-groups: drain before detaching ThrottleState
  block: all I/O should be completed before removing throttle timers.

Signed-off-by: default avatarPeter Maydell <peter.maydell@linaro.org>
parents 0dc8874a 07615626

block/block-backend.c

+24 −12
Original line number Diff line number Diff line
@@ -655,12 +655,16 @@ BlockBackend *blk_by_public(BlockBackendPublic *public) 
 */

void blk_remove_bs(BlockBackend *blk)

{

    ThrottleTimers *tt;

    ThrottleGroupMember *tgm = &blk->public.throttle_group_member;

    BlockDriverState *bs;



    notifier_list_notify(&blk->remove_bs_notifiers, blk);

    if (blk->public.throttle_group_member.throttle_state) {

        tt = &blk->public.throttle_group_member.throttle_timers;

        throttle_timers_detach_aio_context(tt);

    if (tgm->throttle_state) {

        bs = blk_bs(blk);

        bdrv_drained_begin(bs);

        throttle_group_detach_aio_context(tgm);

        throttle_group_attach_aio_context(tgm, qemu_get_aio_context());

        bdrv_drained_end(bs);

    }



    blk_update_root_state(blk);
@@ -674,6 +678,7 @@ void blk_remove_bs(BlockBackend *blk) 
 */

int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)

{

    ThrottleGroupMember *tgm = &blk->public.throttle_group_member;

    blk->root = bdrv_root_attach_child(bs, "root", &child_root,

                                       blk->perm, blk->shared_perm, blk, errp);

    if (blk->root == NULL) {
@@ -682,10 +687,9 @@ int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp) 
    bdrv_ref(bs);



    notifier_list_notify(&blk->insert_bs_notifiers, blk);

    if (blk->public.throttle_group_member.throttle_state) {

        throttle_timers_attach_aio_context(

            &blk->public.throttle_group_member.throttle_timers,

            bdrv_get_aio_context(bs));

    if (tgm->throttle_state) {

        throttle_group_detach_aio_context(tgm);

        throttle_group_attach_aio_context(tgm, bdrv_get_aio_context(bs));

    }



    return 0;
@@ -1748,8 +1752,10 @@ void blk_set_aio_context(BlockBackend *blk, AioContext *new_context) 


    if (bs) {

        if (tgm->throttle_state) {

            bdrv_drained_begin(bs);

            throttle_group_detach_aio_context(tgm);

            throttle_group_attach_aio_context(tgm, new_context);

            bdrv_drained_end(bs);

        }

        bdrv_set_aio_context(bs, new_context);

    }
@@ -1974,10 +1980,16 @@ void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg) 


void blk_io_limits_disable(BlockBackend *blk)

{

    assert(blk->public.throttle_group_member.throttle_state);

    bdrv_drained_begin(blk_bs(blk));

    throttle_group_unregister_tgm(&blk->public.throttle_group_member);

    bdrv_drained_end(blk_bs(blk));

    BlockDriverState *bs = blk_bs(blk);

    ThrottleGroupMember *tgm = &blk->public.throttle_group_member;

    assert(tgm->throttle_state);

    if (bs) {

        bdrv_drained_begin(bs);

    }

    throttle_group_unregister_tgm(tgm);

    if (bs) {

        bdrv_drained_end(bs);

    }

}



/* should be called before blk_set_io_limits if a limit is set */

block/throttle-groups.c

+6 −0
Original line number Diff line number Diff line
@@ -594,6 +594,12 @@ void throttle_group_attach_aio_context(ThrottleGroupMember *tgm, 
void throttle_group_detach_aio_context(ThrottleGroupMember *tgm)

{

    ThrottleTimers *tt = &tgm->throttle_timers;



    /* Requests must have been drained */

    assert(tgm->pending_reqs[0] == 0 && tgm->pending_reqs[1] == 0);

    assert(qemu_co_queue_empty(&tgm->throttled_reqs[0]));

    assert(qemu_co_queue_empty(&tgm->throttled_reqs[1]));



    throttle_timers_detach_aio_context(tt);

    tgm->aio_context = NULL;

}

tests/qemu-iotests/093

+62 −0
Original line number Diff line number Diff line
@@ -308,6 +308,68 @@ class ThrottleTestGroupNames(iotests.QMPTestCase): 
            groupname = "group%d" % i

            self.verify_name(devname, groupname)



class ThrottleTestRemovableMedia(iotests.QMPTestCase):

    def setUp(self):

        self.vm = iotests.VM()

        if iotests.qemu_default_machine == 's390-ccw-virtio':

            self.vm.add_device("virtio-scsi-ccw,id=virtio-scsi")

        else:

            self.vm.add_device("virtio-scsi-pci,id=virtio-scsi")

        self.vm.launch()



    def tearDown(self):

        self.vm.shutdown()



    def test_removable_media(self):

        # Add a couple of dummy nodes named cd0 and cd1

        result = self.vm.qmp("blockdev-add", driver="null-aio",

                             node_name="cd0")

        self.assert_qmp(result, 'return', {})

        result = self.vm.qmp("blockdev-add", driver="null-aio",

                             node_name="cd1")

        self.assert_qmp(result, 'return', {})



        # Attach a CD drive with cd0 inserted

        result = self.vm.qmp("device_add", driver="scsi-cd",

                             id="dev0", drive="cd0")

        self.assert_qmp(result, 'return', {})



        # Set I/O limits

        args = { "id": "dev0", "iops": 100, "iops_rd": 0, "iops_wr": 0,

                                "bps":  50,  "bps_rd": 0,  "bps_wr": 0 }

        result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **args)

        self.assert_qmp(result, 'return', {})



        # Check that the I/O limits have been set

        result = self.vm.qmp("query-block")

        self.assert_qmp(result, 'return[0]/inserted/iops', 100)

        self.assert_qmp(result, 'return[0]/inserted/bps',   50)



        # Now eject cd0 and insert cd1

        result = self.vm.qmp("blockdev-open-tray", id='dev0')

        self.assert_qmp(result, 'return', {})

        result = self.vm.qmp("x-blockdev-remove-medium", id='dev0')

        self.assert_qmp(result, 'return', {})

        result = self.vm.qmp("x-blockdev-insert-medium", id='dev0', node_name='cd1')

        self.assert_qmp(result, 'return', {})



        # Check that the I/O limits are still the same

        result = self.vm.qmp("query-block")

        self.assert_qmp(result, 'return[0]/inserted/iops', 100)

        self.assert_qmp(result, 'return[0]/inserted/bps',   50)



        # Eject cd1

        result = self.vm.qmp("x-blockdev-remove-medium", id='dev0')

        self.assert_qmp(result, 'return', {})



        # Check that we can't set limits if the device has no medium

        result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **args)

        self.assert_qmp(result, 'error/class', 'GenericError')



        # Remove the CD drive

        result = self.vm.qmp("device_del", id='dev0')

        self.assert_qmp(result, 'return', {})





if __name__ == '__main__':

    iotests.main(supported_fmts=["raw"])

tests/qemu-iotests/093.out

+2 −2
Original line number Diff line number Diff line 
.......

........

----------------------------------------------------------------------

Ran 7 tests

Ran 8 tests



OK