diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 12e2ecf517..3503ce3b69 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -1527,6 +1527,122 @@ static void test_set_aio_context(void) iothread_join(b); } + +typedef struct TestDropBackingBlockJob { + BlockJob common; + bool should_complete; + bool *did_complete; +} TestDropBackingBlockJob; + +static int coroutine_fn test_drop_backing_job_run(Job *job, Error **errp) +{ + TestDropBackingBlockJob *s = + container_of(job, TestDropBackingBlockJob, common.job); + + while (!s->should_complete) { + job_sleep_ns(job, 0); + } + + return 0; +} + +static void test_drop_backing_job_commit(Job *job) +{ + TestDropBackingBlockJob *s = + container_of(job, TestDropBackingBlockJob, common.job); + + bdrv_set_backing_hd(blk_bs(s->common.blk), NULL, &error_abort); + + *s->did_complete = true; +} + +static const BlockJobDriver test_drop_backing_job_driver = { + .job_driver = { + .instance_size = sizeof(TestDropBackingBlockJob), + .free = block_job_free, + .user_resume = block_job_user_resume, + .drain = block_job_drain, + .run = test_drop_backing_job_run, + .commit = test_drop_backing_job_commit, + } +}; + +/** + * Creates a child node with three parent nodes on it, and then runs a + * block job on the final one, parent-node-2. + * + * (TODO: parent-node-0 currently serves no purpose, but will as of a + * follow-up patch.) + * + * The job is then asked to complete before a section where the child + * is drained. + * + * Ending this section will undrain the child's parents, first + * parent-node-2, then parent-node-1, then parent-node-0 -- the parent + * list is in reverse order of how they were added. Ending the drain + * on parent-node-2 will resume the job, thus completing it and + * scheduling job_exit(). + * + * Ending the drain on parent-node-1 will poll the AioContext, which + * lets job_exit() and thus test_drop_backing_job_commit() run. That + * function removes the child as parent-node-2's backing file. + * + * In old (and buggy) implementations, there are two problems with + * that: + * (A) bdrv_drain_invoke() polls for every node that leaves the + * drained section. This means that job_exit() is scheduled + * before the child has left the drained section. Its + * quiesce_counter is therefore still 1 when it is removed from + * parent-node-2. + * + * (B) bdrv_replace_child_noperm() calls drained_end() on the old + * child's parents as many times as the child is quiesced. This + * means it will call drained_end() on parent-node-2 once. + * Because parent-node-2 is no longer quiesced at this point, this + * will fail. + * + * bdrv_replace_child_noperm() therefore must call drained_end() on + * the parent only if it really is still drained because the child is + * drained. + */ +static void test_blockjob_commit_by_drained_end(void) +{ + BlockDriverState *bs_child, *bs_parents[3]; + TestDropBackingBlockJob *job; + bool job_has_completed = false; + int i; + + bs_child = bdrv_new_open_driver(&bdrv_test, "child-node", BDRV_O_RDWR, + &error_abort); + + for (i = 0; i < 3; i++) { + char name[32]; + snprintf(name, sizeof(name), "parent-node-%i", i); + bs_parents[i] = bdrv_new_open_driver(&bdrv_test, name, BDRV_O_RDWR, + &error_abort); + bdrv_set_backing_hd(bs_parents[i], bs_child, &error_abort); + } + + job = block_job_create("job", &test_drop_backing_job_driver, NULL, + bs_parents[2], 0, BLK_PERM_ALL, 0, 0, NULL, NULL, + &error_abort); + + job->did_complete = &job_has_completed; + + job_start(&job->common.job); + + job->should_complete = true; + bdrv_drained_begin(bs_child); + g_assert(!job_has_completed); + bdrv_drained_end(bs_child); + g_assert(job_has_completed); + + bdrv_unref(bs_parents[0]); + bdrv_unref(bs_parents[1]); + bdrv_unref(bs_parents[2]); + bdrv_unref(bs_child); +} + int main(int argc, char **argv) { int ret; @@ -1610,6 +1726,9 @@ int main(int argc, char **argv) g_test_add_func("/bdrv-drain/set_aio_context", test_set_aio_context); + g_test_add_func("/bdrv-drain/blockjob/commit_by_drained_end", + test_blockjob_commit_by_drained_end); + ret = g_test_run(); qemu_event_destroy(&done_event); return ret;