throttle: acquire the ThrottleGroup lock in bdrv_swap()

bdrv_swap() touches the fields of a BlockDriverState that are
protected by the ThrottleGroup lock. Although those fields end up in
their original place, they are temporarily swapped in the process,
so there's a chance that an operation on a member of the same group
happening on a different thread can try to use them.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: d92dc40d7c4f1fc5cda5cbbf4ffb7a4670b79d17.1433779731.git.berto@igalia.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
This commit is contained in:
Alberto Garcia 2015-06-08 18:17:45 +02:00 committed by Stefan Hajnoczi
parent 76f4afb40f
commit db6283385c
3 changed files with 49 additions and 1 deletions

16
block.c
View file

@ -36,6 +36,7 @@
#include "qmp-commands.h"
#include "qemu/timer.h"
#include "qapi-event.h"
#include "block/throttle-groups.h"
#ifdef CONFIG_BSD
#include <sys/types.h>
@ -1887,11 +1888,20 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)
QTAILQ_REMOVE(&graph_bdrv_states, bs_old, node_list);
}
/* If the BlockDriverState is part of a throttling group acquire
* its lock since we're going to mess with the protected fields.
* Otherwise there's no need to worry since no one else can touch
* them. */
if (bs_old->throttle_state) {
throttle_group_lock(bs_old);
}
/* bs_new must be unattached and shouldn't have anything fancy enabled */
assert(!bs_new->blk);
assert(QLIST_EMPTY(&bs_new->dirty_bitmaps));
assert(bs_new->job == NULL);
assert(bs_new->io_limits_enabled == false);
assert(bs_new->throttle_state == NULL);
assert(!throttle_timers_are_initialized(&bs_new->throttle_timers));
tmp = *bs_new;
@ -1909,8 +1919,14 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)
/* Check a few fields that should remain attached to the device */
assert(bs_new->job == NULL);
assert(bs_new->io_limits_enabled == false);
assert(bs_new->throttle_state == NULL);
assert(!throttle_timers_are_initialized(&bs_new->throttle_timers));
/* Release the ThrottleGroup lock */
if (bs_old->throttle_state) {
throttle_group_unlock(bs_old);
}
/* insert the nodes back into the graph node list if needed */
if (bs_new->node_name[0] != '\0') {
QTAILQ_INSERT_TAIL(&graph_bdrv_states, bs_new, node_list);