@@ -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;