Skip to content

Commit

Permalink
test-bdrv-drain: Fix incorrrect drain assumptions
Browse files Browse the repository at this point in the history
The test case assumes that a drain only happens in one specific place
where it drains explicitly. This assumption happened to hold true until
now, but block layer functions may drain interally (any graph
modifications are going to do that through bdrv_graph_wrlock()), so this
is incorrect. Make sure that the test code in .drained_begin only runs
where we actually want it to run.

When scheduling a BH from .drained_begin, we also need to increase the
in_flight counter to make sure that the operation is actually completed
in time before the node that it works on goes away.

Signed-off-by: Kevin Wolf <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Emanuele Giuseppe Esposito <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
  • Loading branch information
kevmw committed Dec 15, 2022
1 parent d9bfb9d commit 617f3a9
Showing 1 changed file with 18 additions and 0 deletions.
18 changes: 18 additions & 0 deletions tests/unit/test-bdrv-drain.c
Original file line number Diff line number Diff line change
Expand Up @@ -1107,13 +1107,15 @@ struct detach_by_parent_data {
BlockDriverState *c;
BdrvChild *child_c;
bool by_parent_cb;
bool detach_on_drain;
};
static struct detach_by_parent_data detach_by_parent_data;

static void detach_indirect_bh(void *opaque)
{
struct detach_by_parent_data *data = opaque;

bdrv_dec_in_flight(data->child_b->bs);
bdrv_unref_child(data->parent_b, data->child_b);

bdrv_ref(data->c);
Expand All @@ -1128,12 +1130,21 @@ static void detach_by_parent_aio_cb(void *opaque, int ret)

g_assert_cmpint(ret, ==, 0);
if (data->by_parent_cb) {
bdrv_inc_in_flight(data->child_b->bs);
detach_indirect_bh(data);
}
}

static void detach_by_driver_cb_drained_begin(BdrvChild *child)
{
struct detach_by_parent_data *data = &detach_by_parent_data;

if (!data->detach_on_drain) {
return;
}
data->detach_on_drain = false;

bdrv_inc_in_flight(data->child_b->bs);
aio_bh_schedule_oneshot(qemu_get_current_aio_context(),
detach_indirect_bh, &detach_by_parent_data);
child_of_bds.drained_begin(child);
Expand Down Expand Up @@ -1174,8 +1185,14 @@ static void test_detach_indirect(bool by_parent_cb)
detach_by_driver_cb_class = child_of_bds;
detach_by_driver_cb_class.drained_begin =
detach_by_driver_cb_drained_begin;
detach_by_driver_cb_class.drained_end = NULL;
detach_by_driver_cb_class.drained_poll = NULL;
}

detach_by_parent_data = (struct detach_by_parent_data) {
.detach_on_drain = false,
};

/* Create all involved nodes */
parent_a = bdrv_new_open_driver(&bdrv_test, "parent-a", BDRV_O_RDWR,
&error_abort);
Expand Down Expand Up @@ -1227,6 +1244,7 @@ static void test_detach_indirect(bool by_parent_cb)
.child_b = child_b,
.c = c,
.by_parent_cb = by_parent_cb,
.detach_on_drain = true,
};
acb = blk_aio_preadv(blk, 0, &qiov, 0, detach_by_parent_aio_cb, NULL);
g_assert(acb != NULL);
Expand Down

0 comments on commit 617f3a9

Please sign in to comment.