diff --git a/block.c b/block.c index 29e931e217..9c94f7f28a 100644 --- a/block.c +++ b/block.c @@ -911,10 +911,11 @@ static bool bdrv_child_cb_drained_poll(BdrvChild *child) return bdrv_drain_poll(bs, false, NULL, false); } -static void bdrv_child_cb_drained_end(BdrvChild *child) +static void bdrv_child_cb_drained_end(BdrvChild *child, + int *drained_end_counter) { BlockDriverState *bs = child->opaque; - bdrv_drained_end(bs); + bdrv_drained_end_no_poll(bs, drained_end_counter); } static void bdrv_child_cb_attach(BdrvChild *child) @@ -2251,24 +2252,19 @@ static void bdrv_replace_child_noperm(BdrvChild *child, if (child->role->detach) { child->role->detach(child); } - if (old_bs->quiesce_counter && child->role->drained_end) { - int num = old_bs->quiesce_counter; - if (child->role->parent_is_bds) { - num -= bdrv_drain_all_count; - } - assert(num >= 0); - for (i = 0; i < num; i++) { - child->role->drained_end(child); - } + while (child->parent_quiesce_counter) { + bdrv_parent_drained_end_single(child); } QLIST_REMOVE(child, next_parent); + } else { + assert(child->parent_quiesce_counter == 0); } child->bs = new_bs; if (new_bs) { QLIST_INSERT_HEAD(&new_bs->parents, child, next_parent); - if (new_bs->quiesce_counter && child->role->drained_begin) { + if (new_bs->quiesce_counter) { int num = new_bs->quiesce_counter; if (child->role->parent_is_bds) { num -= bdrv_drain_all_count; @@ -5928,9 +5924,11 @@ static void bdrv_attach_aio_context(BlockDriverState *bs, void bdrv_set_aio_context_ignore(BlockDriverState *bs, AioContext *new_context, GSList **ignore) { + AioContext *old_context = bdrv_get_aio_context(bs); + AioContext *current_context = qemu_get_current_aio_context(); BdrvChild *child; - if (bdrv_get_aio_context(bs) == new_context) { + if (old_context == new_context) { return; } @@ -5954,13 +5952,31 @@ void bdrv_set_aio_context_ignore(BlockDriverState *bs, bdrv_detach_aio_context(bs); - /* This function executes in the old AioContext so acquire the new one in - * case it runs in a different thread. - */ - aio_context_acquire(new_context); + /* Acquire the new context, if necessary */ + if (current_context != new_context) { + aio_context_acquire(new_context); + } + bdrv_attach_aio_context(bs, new_context); + + /* + * If this function was recursively called from + * bdrv_set_aio_context_ignore(), there may be nodes in the + * subtree that have not yet been moved to the new AioContext. + * Release the old one so bdrv_drained_end() can poll them. + */ + if (current_context != old_context) { + aio_context_release(old_context); + } + bdrv_drained_end(bs); - aio_context_release(new_context); + + if (current_context != old_context) { + aio_context_acquire(old_context); + } + if (current_context != new_context) { + aio_context_release(new_context); + } } static bool bdrv_parent_can_set_aio_context(BdrvChild *c, AioContext *ctx, diff --git a/block/block-backend.c b/block/block-backend.c index a8d160fd5d..0056b526b8 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -121,7 +121,7 @@ static void blk_root_inherit_options(int *child_flags, QDict *child_options, } static void blk_root_drained_begin(BdrvChild *child); static bool blk_root_drained_poll(BdrvChild *child); -static void blk_root_drained_end(BdrvChild *child); +static void blk_root_drained_end(BdrvChild *child, int *drained_end_counter); static void blk_root_change_media(BdrvChild *child, bool load); static void blk_root_resize(BdrvChild *child); @@ -1249,7 +1249,7 @@ int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf, blk_root_drained_begin(blk->root); ret = blk_pread(blk, offset, buf, count); - blk_root_drained_end(blk->root); + blk_root_drained_end(blk->root, NULL); return ret; } @@ -2236,7 +2236,7 @@ static bool blk_root_drained_poll(BdrvChild *child) return !!blk->in_flight; } -static void blk_root_drained_end(BdrvChild *child) +static void blk_root_drained_end(BdrvChild *child, int *drained_end_counter) { BlockBackend *blk = child->opaque; assert(blk->quiesce_counter); diff --git a/block/io.c b/block/io.c index 24a18759fd..b89e155d21 100644 --- a/block/io.c +++ b/block/io.c @@ -42,8 +42,8 @@ static void bdrv_parent_cb_resize(BlockDriverState *bs); static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int bytes, BdrvRequestFlags flags); -void bdrv_parent_drained_begin(BlockDriverState *bs, BdrvChild *ignore, - bool ignore_bds_parents) +static void bdrv_parent_drained_begin(BlockDriverState *bs, BdrvChild *ignore, + bool ignore_bds_parents) { BdrvChild *c, *next; @@ -55,18 +55,34 @@ void bdrv_parent_drained_begin(BlockDriverState *bs, BdrvChild *ignore, } } -void bdrv_parent_drained_end(BlockDriverState *bs, BdrvChild *ignore, - bool ignore_bds_parents) +static void bdrv_parent_drained_end_single_no_poll(BdrvChild *c, + int *drained_end_counter) { - BdrvChild *c, *next; + assert(c->parent_quiesce_counter > 0); + c->parent_quiesce_counter--; + if (c->role->drained_end) { + c->role->drained_end(c, drained_end_counter); + } +} - QLIST_FOREACH_SAFE(c, &bs->parents, next_parent, next) { +void bdrv_parent_drained_end_single(BdrvChild *c) +{ + int drained_end_counter = 0; + bdrv_parent_drained_end_single_no_poll(c, &drained_end_counter); + BDRV_POLL_WHILE(c->bs, atomic_read(&drained_end_counter) > 0); +} + +static void bdrv_parent_drained_end(BlockDriverState *bs, BdrvChild *ignore, + bool ignore_bds_parents, + int *drained_end_counter) +{ + BdrvChild *c; + + QLIST_FOREACH(c, &bs->parents, next_parent) { if (c == ignore || (ignore_bds_parents && c->role->parent_is_bds)) { continue; } - if (c->role->drained_end) { - c->role->drained_end(c); - } + bdrv_parent_drained_end_single_no_poll(c, drained_end_counter); } } @@ -96,6 +112,7 @@ static bool bdrv_parent_drained_poll(BlockDriverState *bs, BdrvChild *ignore, void bdrv_parent_drained_begin_single(BdrvChild *c, bool poll) { + c->parent_quiesce_counter++; if (c->role->drained_begin) { c->role->drained_begin(c); } @@ -186,6 +203,7 @@ typedef struct { bool poll; BdrvChild *parent; bool ignore_bds_parents; + int *drained_end_counter; } BdrvCoDrainData; static void coroutine_fn bdrv_drain_invoke_entry(void *opaque) @@ -203,13 +221,16 @@ static void coroutine_fn bdrv_drain_invoke_entry(void *opaque) atomic_mb_set(&data->done, true); bdrv_dec_in_flight(bs); - if (data->begin) { - g_free(data); + if (!data->begin) { + atomic_dec(data->drained_end_counter); } + + g_free(data); } /* Recursively call BlockDriver.bdrv_co_drain_begin/end callbacks */ -static void bdrv_drain_invoke(BlockDriverState *bs, bool begin) +static void bdrv_drain_invoke(BlockDriverState *bs, bool begin, + int *drained_end_counter) { BdrvCoDrainData *data; @@ -222,19 +243,19 @@ static void bdrv_drain_invoke(BlockDriverState *bs, bool begin) *data = (BdrvCoDrainData) { .bs = bs, .done = false, - .begin = begin + .begin = begin, + .drained_end_counter = drained_end_counter, }; + if (!begin) { + atomic_inc(drained_end_counter); + } + /* Make sure the driver callback completes during the polling phase for * drain_begin. */ bdrv_inc_in_flight(bs); data->co = qemu_coroutine_create(bdrv_drain_invoke_entry, data); aio_co_schedule(bdrv_get_aio_context(bs), data->co); - - if (!begin) { - BDRV_POLL_WHILE(bs, !data->done); - g_free(data); - } } /* Returns true if BDRV_POLL_WHILE() should go into a blocking aio_poll() */ @@ -273,7 +294,8 @@ static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive, BdrvChild *parent, bool ignore_bds_parents, bool poll); static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive, - BdrvChild *parent, bool ignore_bds_parents); + BdrvChild *parent, bool ignore_bds_parents, + int *drained_end_counter); static void bdrv_co_drain_bh_cb(void *opaque) { @@ -296,11 +318,14 @@ static void bdrv_co_drain_bh_cb(void *opaque) } bdrv_dec_in_flight(bs); if (data->begin) { + assert(!data->drained_end_counter); bdrv_do_drained_begin(bs, data->recursive, data->parent, data->ignore_bds_parents, data->poll); } else { + assert(!data->poll); bdrv_do_drained_end(bs, data->recursive, data->parent, - data->ignore_bds_parents); + data->ignore_bds_parents, + data->drained_end_counter); } if (ctx == co_ctx) { aio_context_release(ctx); @@ -318,7 +343,8 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs, bool begin, bool recursive, BdrvChild *parent, bool ignore_bds_parents, - bool poll) + bool poll, + int *drained_end_counter) { BdrvCoDrainData data; @@ -335,7 +361,9 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs, .parent = parent, .ignore_bds_parents = ignore_bds_parents, .poll = poll, + .drained_end_counter = drained_end_counter, }; + if (bs) { bdrv_inc_in_flight(bs); } @@ -359,7 +387,7 @@ void bdrv_do_drained_begin_quiesce(BlockDriverState *bs, } bdrv_parent_drained_begin(bs, parent, ignore_bds_parents); - bdrv_drain_invoke(bs, true); + bdrv_drain_invoke(bs, true, NULL); } static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive, @@ -370,7 +398,7 @@ static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive, if (qemu_in_coroutine()) { bdrv_co_yield_to_drain(bs, true, recursive, parent, ignore_bds_parents, - poll); + poll, NULL); return; } @@ -410,22 +438,40 @@ void bdrv_subtree_drained_begin(BlockDriverState *bs) bdrv_do_drained_begin(bs, true, NULL, false, true); } +/** + * This function does not poll, nor must any of its recursively called + * functions. The *drained_end_counter pointee will be incremented + * once for every background operation scheduled, and decremented once + * the operation settles. Therefore, the pointer must remain valid + * until the pointee reaches 0. That implies that whoever sets up the + * pointee has to poll until it is 0. + * + * We use atomic operations to access *drained_end_counter, because + * (1) when called from bdrv_set_aio_context_ignore(), the subgraph of + * @bs may contain nodes in different AioContexts, + * (2) bdrv_drain_all_end() uses the same counter for all nodes, + * regardless of which AioContext they are in. + */ static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive, - BdrvChild *parent, bool ignore_bds_parents) + BdrvChild *parent, bool ignore_bds_parents, + int *drained_end_counter) { - BdrvChild *child, *next; + BdrvChild *child; int old_quiesce_counter; + assert(drained_end_counter != NULL); + if (qemu_in_coroutine()) { bdrv_co_yield_to_drain(bs, false, recursive, parent, ignore_bds_parents, - false); + false, drained_end_counter); return; } assert(bs->quiesce_counter > 0); /* Re-enable things in child-to-parent order */ - bdrv_drain_invoke(bs, false); - bdrv_parent_drained_end(bs, parent, ignore_bds_parents); + bdrv_drain_invoke(bs, false, drained_end_counter); + bdrv_parent_drained_end(bs, parent, ignore_bds_parents, + drained_end_counter); old_quiesce_counter = atomic_fetch_dec(&bs->quiesce_counter); if (old_quiesce_counter == 1) { @@ -435,20 +481,30 @@ static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive, if (recursive) { assert(!ignore_bds_parents); bs->recursive_quiesce_counter--; - QLIST_FOREACH_SAFE(child, &bs->children, next, next) { - bdrv_do_drained_end(child->bs, true, child, ignore_bds_parents); + QLIST_FOREACH(child, &bs->children, next) { + bdrv_do_drained_end(child->bs, true, child, ignore_bds_parents, + drained_end_counter); } } } void bdrv_drained_end(BlockDriverState *bs) { - bdrv_do_drained_end(bs, false, NULL, false); + int drained_end_counter = 0; + bdrv_do_drained_end(bs, false, NULL, false, &drained_end_counter); + BDRV_POLL_WHILE(bs, atomic_read(&drained_end_counter) > 0); +} + +void bdrv_drained_end_no_poll(BlockDriverState *bs, int *drained_end_counter) +{ + bdrv_do_drained_end(bs, false, NULL, false, drained_end_counter); } void bdrv_subtree_drained_end(BlockDriverState *bs) { - bdrv_do_drained_end(bs, true, NULL, false); + int drained_end_counter = 0; + bdrv_do_drained_end(bs, true, NULL, false, &drained_end_counter); + BDRV_POLL_WHILE(bs, atomic_read(&drained_end_counter) > 0); } void bdrv_apply_subtree_drain(BdrvChild *child, BlockDriverState *new_parent) @@ -462,11 +518,15 @@ void bdrv_apply_subtree_drain(BdrvChild *child, BlockDriverState *new_parent) void bdrv_unapply_subtree_drain(BdrvChild *child, BlockDriverState *old_parent) { + int drained_end_counter = 0; int i; for (i = 0; i < old_parent->recursive_quiesce_counter; i++) { - bdrv_do_drained_end(child->bs, true, child, false); + bdrv_do_drained_end(child->bs, true, child, false, + &drained_end_counter); } + + BDRV_POLL_WHILE(child->bs, atomic_read(&drained_end_counter) > 0); } /* @@ -535,7 +595,7 @@ void bdrv_drain_all_begin(void) BlockDriverState *bs = NULL; if (qemu_in_coroutine()) { - bdrv_co_yield_to_drain(NULL, true, false, NULL, true, true); + bdrv_co_yield_to_drain(NULL, true, false, NULL, true, true, NULL); return; } @@ -566,15 +626,19 @@ void bdrv_drain_all_begin(void) void bdrv_drain_all_end(void) { BlockDriverState *bs = NULL; + int drained_end_counter = 0; while ((bs = bdrv_next_all_states(bs))) { AioContext *aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); - bdrv_do_drained_end(bs, false, NULL, true); + bdrv_do_drained_end(bs, false, NULL, true, &drained_end_counter); aio_context_release(aio_context); } + assert(qemu_get_current_aio_context() == qemu_get_aio_context()); + AIO_WAIT_WHILE(NULL, atomic_read(&drained_end_counter) > 0); + assert(bdrv_drain_all_count > 0); bdrv_drain_all_count--; } diff --git a/blockjob.c b/blockjob.c index 458ae76f51..20b7f557da 100644 --- a/blockjob.c +++ b/blockjob.c @@ -135,7 +135,7 @@ static bool child_job_drained_poll(BdrvChild *c) } } -static void child_job_drained_end(BdrvChild *c) +static void child_job_drained_end(BdrvChild *c, int *drained_end_counter) { BlockJob *job = c->opaque; job_resume(&job->job); diff --git a/include/block/block.h b/include/block/block.h index 734c9d2f76..60f00479e0 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -600,15 +600,6 @@ int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo); void bdrv_io_plug(BlockDriverState *bs); void bdrv_io_unplug(BlockDriverState *bs); -/** - * bdrv_parent_drained_begin: - * - * Begin a quiesced section of all users of @bs. This is part of - * bdrv_drained_begin. - */ -void bdrv_parent_drained_begin(BlockDriverState *bs, BdrvChild *ignore, - bool ignore_bds_parents); - /** * bdrv_parent_drained_begin_single: * @@ -618,13 +609,14 @@ void bdrv_parent_drained_begin(BlockDriverState *bs, BdrvChild *ignore, void bdrv_parent_drained_begin_single(BdrvChild *c, bool poll); /** - * bdrv_parent_drained_end: + * bdrv_parent_drained_end_single: * - * End a quiesced section of all users of @bs. This is part of - * bdrv_drained_end. + * End a quiesced section for the parent of @c. + * + * This polls @bs's AioContext until all scheduled sub-drained_ends + * have settled, which may result in graph changes. */ -void bdrv_parent_drained_end(BlockDriverState *bs, BdrvChild *ignore, - bool ignore_bds_parents); +void bdrv_parent_drained_end_single(BdrvChild *c); /** * bdrv_drain_poll: @@ -672,9 +664,31 @@ void bdrv_subtree_drained_begin(BlockDriverState *bs); * bdrv_drained_end: * * End a quiescent section started by bdrv_drained_begin(). + * + * This polls @bs's AioContext until all scheduled sub-drained_ends + * have settled. On one hand, that may result in graph changes. On + * the other, this requires that all involved nodes (@bs and all of + * its parents) are in the same AioContext, and that the caller has + * acquired it. + * If there are any nodes that are in different contexts from @bs, + * these contexts must not be acquired. */ void bdrv_drained_end(BlockDriverState *bs); +/** + * bdrv_drained_end_no_poll: + * + * Same as bdrv_drained_end(), but do not poll for the subgraph to + * actually become unquiesced. Therefore, no graph changes will occur + * with this function. + * + * *drained_end_counter is incremented for every background operation + * that is scheduled, and will be decremented for every operation once + * it settles. The caller must poll until it reaches 0. The counter + * should be accessed using atomic operations only. + */ +void bdrv_drained_end_no_poll(BlockDriverState *bs, int *drained_end_counter); + /** * End a quiescent section started by bdrv_subtree_drained_begin(). */ diff --git a/include/block/block_int.h b/include/block/block_int.h index 50902531b7..3aa1e832a8 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -664,11 +664,15 @@ struct BdrvChildRole { * These functions must not change the graph (and therefore also must not * call aio_poll(), which could change the graph indirectly). * + * If drained_end() schedules background operations, it must atomically + * increment *drained_end_counter for each such operation and atomically + * decrement it once the operation has settled. + * * Note that this can be nested. If drained_begin() was called twice, new * I/O is allowed only after drained_end() was called twice, too. */ void (*drained_begin)(BdrvChild *child); - void (*drained_end)(BdrvChild *child); + void (*drained_end)(BdrvChild *child, int *drained_end_counter); /* * Returns whether the parent has pending requests for the child. This @@ -729,6 +733,15 @@ struct BdrvChild { */ bool frozen; + /* + * How many times the parent of this child has been drained + * (through role->drained_*). + * Usually, this is equal to bs->quiesce_counter (potentially + * reduced by bdrv_drain_all_count). It may differ while the + * child is entering or leaving a drained section. + */ + int parent_quiesce_counter; + QLIST_ENTRY(BdrvChild) next; QLIST_ENTRY(BdrvChild) next_parent; }; diff --git a/python/qemu/machine.py b/python/qemu/machine.py index 49445e675b..128a3d1dc2 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -329,13 +329,14 @@ class QEMUMachine(object): self._load_io_log() self._post_shutdown() - def shutdown(self): + def shutdown(self, has_quit=False): """ Terminate the VM and clean up """ if self.is_running(): try: - self._qmp.cmd('quit') + if not has_quit: + self._qmp.cmd('quit') self._qmp.close() except: self._popen.kill() diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index b81133a474..aa0b1847e3 100755 --- a/tests/qemu-iotests/040 +++ b/tests/qemu-iotests/040 @@ -92,9 +92,10 @@ class TestSingleDrive(ImageCommitTestCase): self.vm.add_device("scsi-hd,id=scsi0,drive=drive0") self.vm.launch() + self.has_quit = False def tearDown(self): - self.vm.shutdown() + self.vm.shutdown(has_quit=self.has_quit) os.remove(test_img) os.remove(mid_img) os.remove(backing_img) @@ -109,6 +110,43 @@ class TestSingleDrive(ImageCommitTestCase): self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img).find("verification failed")) self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed")) + def test_commit_with_filter_and_quit(self): + result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg') + self.assert_qmp(result, 'return', {}) + + # Add a filter outside of the backing chain + result = self.vm.qmp('blockdev-add', driver='throttle', node_name='filter', throttle_group='tg', file='mid') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('block-commit', device='drive0') + self.assert_qmp(result, 'return', {}) + + # Quit immediately, thus forcing a simultaneous cancel of the + # block job and a bdrv_drain_all() + result = self.vm.qmp('quit') + self.assert_qmp(result, 'return', {}) + + self.has_quit = True + + # Same as above, but this time we add the filter after starting the job + def test_commit_plus_filter_and_quit(self): + result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('block-commit', device='drive0') + self.assert_qmp(result, 'return', {}) + + # Add a filter outside of the backing chain + result = self.vm.qmp('blockdev-add', driver='throttle', node_name='filter', throttle_group='tg', file='mid') + self.assert_qmp(result, 'return', {}) + + # Quit immediately, thus forcing a simultaneous cancel of the + # block job and a bdrv_drain_all() + result = self.vm.qmp('quit') + self.assert_qmp(result, 'return', {}) + + self.has_quit = True + def test_device_not_found(self): result = self.vm.qmp('block-commit', device='nonexistent', top='%s' % mid_img) self.assert_qmp(result, 'error/class', 'DeviceNotFound') diff --git a/tests/qemu-iotests/040.out b/tests/qemu-iotests/040.out index 802ffaa0c0..220a5fa82c 100644 --- a/tests/qemu-iotests/040.out +++ b/tests/qemu-iotests/040.out @@ -1,5 +1,5 @@ -........................................... +............................................... ---------------------------------------------------------------------- -Ran 43 tests +Ran 47 tests OK diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051 index 200660f977..ce942a5444 100755 --- a/tests/qemu-iotests/051 +++ b/tests/qemu-iotests/051 @@ -251,11 +251,11 @@ echo # Cannot use the test image because cache=none might not work on the host FS # Use cdrom so that we won't get errors about missing media -run_qemu -drive driver=null-co,cache=none -run_qemu -drive driver=null-co,cache=directsync -run_qemu -drive driver=null-co,cache=writeback -run_qemu -drive driver=null-co,cache=writethrough -run_qemu -drive driver=null-co,cache=unsafe +run_qemu -drive driver=null-co,read-zeroes=on,cache=none +run_qemu -drive driver=null-co,read-zeroes=on,cache=directsync +run_qemu -drive driver=null-co,read-zeroes=on,cache=writeback +run_qemu -drive driver=null-co,read-zeroes=on,cache=writethrough +run_qemu -drive driver=null-co,read-zeroes=on,cache=unsafe run_qemu -drive driver=null-co,cache=invalid_value # Can't test direct=on here because O_DIRECT might not be supported on this FS diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out index 2d811c166c..000557c7c8 100644 --- a/tests/qemu-iotests/051.pc.out +++ b/tests/qemu-iotests/051.pc.out @@ -245,23 +245,23 @@ QEMU X.Y.Z monitor - type 'help' for more information === Cache modes === -Testing: -drive driver=null-co,cache=none +Testing: -drive driver=null-co,read-zeroes=on,cache=none QEMU X.Y.Z monitor - type 'help' for more information (qemu) quit -Testing: -drive driver=null-co,cache=directsync +Testing: -drive driver=null-co,read-zeroes=on,cache=directsync QEMU X.Y.Z monitor - type 'help' for more information (qemu) quit -Testing: -drive driver=null-co,cache=writeback +Testing: -drive driver=null-co,read-zeroes=on,cache=writeback QEMU X.Y.Z monitor - type 'help' for more information (qemu) quit -Testing: -drive driver=null-co,cache=writethrough +Testing: -drive driver=null-co,read-zeroes=on,cache=writethrough QEMU X.Y.Z monitor - type 'help' for more information (qemu) quit -Testing: -drive driver=null-co,cache=unsafe +Testing: -drive driver=null-co,read-zeroes=on,cache=unsafe QEMU X.Y.Z monitor - type 'help' for more information (qemu) quit diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093 index d88fbc182e..4b2cac1d0c 100755 --- a/tests/qemu-iotests/093 +++ b/tests/qemu-iotests/093 @@ -38,7 +38,7 @@ class ThrottleTestCase(iotests.QMPTestCase): def setUp(self): self.vm = iotests.VM() for i in range(0, self.max_drives): - self.vm.add_drive(self.test_img) + self.vm.add_drive(self.test_img, "file.read-zeroes=on") self.vm.launch() def tearDown(self): @@ -273,7 +273,8 @@ class ThrottleTestGroupNames(iotests.QMPTestCase): def setUp(self): self.vm = iotests.VM() for i in range(0, self.max_drives): - self.vm.add_drive(self.test_img, "throttling.iops-total=100") + self.vm.add_drive(self.test_img, + "throttling.iops-total=100,file.read-zeroes=on") self.vm.launch() def tearDown(self): @@ -378,10 +379,10 @@ class ThrottleTestRemovableMedia(iotests.QMPTestCase): 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") + read_zeroes=True, node_name="cd0") self.assert_qmp(result, 'return', {}) result = self.vm.qmp("blockdev-add", driver="null-aio", - node_name="cd1") + read_zeroes=True, node_name="cd1") self.assert_qmp(result, 'return', {}) # Attach a CD drive with cd0 inserted diff --git a/tests/qemu-iotests/136 b/tests/qemu-iotests/136 index af7ffa4540..a46a7b7630 100755 --- a/tests/qemu-iotests/136 +++ b/tests/qemu-iotests/136 @@ -74,6 +74,7 @@ sector = "%d" (self.account_invalid and "on" or "off")) drive_args.append("stats-account-failed=%s" % (self.account_failed and "on" or "off")) + drive_args.append("file.image.read-zeroes=on") self.create_blkdebug_file() self.vm = iotests.VM().add_drive('blkdebug:%s:%s' % (blkdebug_file, self.test_img), diff --git a/tests/qemu-iotests/186 b/tests/qemu-iotests/186 index 7e7d45babc..5f6b18c150 100755 --- a/tests/qemu-iotests/186 +++ b/tests/qemu-iotests/186 @@ -86,8 +86,8 @@ echo "=== -blockdev/-device= ===" echo for dev in $fixed $removable; do - check_info_block -blockdev driver=null-co,node-name=null -device $dev,drive=null - check_info_block -blockdev driver=null-co,node-name=null -device $dev,drive=null,id=qdev_id + check_info_block -blockdev driver=null-co,read-zeroes=on,node-name=null -device $dev,drive=null + check_info_block -blockdev driver=null-co,read-zeroes=on,node-name=null -device $dev,drive=null,id=qdev_id done echo @@ -97,7 +97,7 @@ echo # This creates two BlockBackends that will show up in 'info block'! # A monitor-owned one from -drive, and anonymous one from -device for dev in $fixed $removable; do - check_info_block -drive if=none,driver=null-co,node-name=null -device $dev,drive=null,id=qdev_id + check_info_block -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device $dev,drive=null,id=qdev_id done echo @@ -105,8 +105,8 @@ echo "=== -drive if=none/-device= (with medium) ===" echo for dev in $fixed $removable; do - check_info_block -drive if=none,driver=null-co,node-name=null -device $dev,drive=none0 - check_info_block -drive if=none,driver=null-co,node-name=null -device $dev,drive=none0,id=qdev_id + check_info_block -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device $dev,drive=none0 + check_info_block -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device $dev,drive=none0,id=qdev_id done echo @@ -125,15 +125,15 @@ echo "=== -drive if=... ===" echo check_info_block -drive if=floppy -check_info_block -drive if=floppy,driver=null-co +check_info_block -drive if=floppy,driver=null-co,read-zeroes=on -check_info_block -drive if=ide,driver=null-co +check_info_block -drive if=ide,driver=null-co,read-zeroes=on check_info_block -drive if=ide,media=cdrom -check_info_block -drive if=ide,driver=null-co,media=cdrom +check_info_block -drive if=ide,driver=null-co,read-zeroes=on,media=cdrom -check_info_block -drive if=virtio,driver=null-co +check_info_block -drive if=virtio,driver=null-co,read-zeroes=on -check_info_block -drive if=pflash,driver=null-co,size=1M +check_info_block -drive if=pflash,driver=null-co,read-zeroes=on,size=1M # success, all done echo "*** done" diff --git a/tests/qemu-iotests/186.out b/tests/qemu-iotests/186.out index 716b01ac3d..5b3504042a 100644 --- a/tests/qemu-iotests/186.out +++ b/tests/qemu-iotests/186.out @@ -54,103 +54,103 @@ qdev_id: [not inserted] === -blockdev/-device= === -Testing: -blockdev driver=null-co,node-name=null -device ide-hd,drive=null +Testing: -blockdev driver=null-co,read-zeroes=on,node-name=null -device ide-hd,drive=null QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -null: null-co:// (null-co) +null: json:{"read-zeroes": true, "driver": "null-co"} (null-co) Attached to: PATH Cache mode: writeback (qemu) quit -Testing: -blockdev driver=null-co,node-name=null -device ide-hd,drive=null,id=qdev_id +Testing: -blockdev driver=null-co,read-zeroes=on,node-name=null -device ide-hd,drive=null,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -null: null-co:// (null-co) +null: json:{"read-zeroes": true, "driver": "null-co"} (null-co) Attached to: qdev_id Cache mode: writeback (qemu) quit -Testing: -blockdev driver=null-co,node-name=null -device scsi-hd,drive=null +Testing: -blockdev driver=null-co,read-zeroes=on,node-name=null -device scsi-hd,drive=null QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -null: null-co:// (null-co) +null: json:{"read-zeroes": true, "driver": "null-co"} (null-co) Attached to: PATH Cache mode: writeback (qemu) quit -Testing: -blockdev driver=null-co,node-name=null -device scsi-hd,drive=null,id=qdev_id +Testing: -blockdev driver=null-co,read-zeroes=on,node-name=null -device scsi-hd,drive=null,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -null: null-co:// (null-co) +null: json:{"read-zeroes": true, "driver": "null-co"} (null-co) Attached to: qdev_id Cache mode: writeback (qemu) quit -Testing: -blockdev driver=null-co,node-name=null -device virtio-blk-pci,drive=null +Testing: -blockdev driver=null-co,read-zeroes=on,node-name=null -device virtio-blk-pci,drive=null QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -null: null-co:// (null-co) +null: json:{"read-zeroes": true, "driver": "null-co"} (null-co) Attached to: PATH Cache mode: writeback (qemu) quit -Testing: -blockdev driver=null-co,node-name=null -device virtio-blk-pci,drive=null,id=qdev_id +Testing: -blockdev driver=null-co,read-zeroes=on,node-name=null -device virtio-blk-pci,drive=null,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -null: null-co:// (null-co) +null: json:{"read-zeroes": true, "driver": "null-co"} (null-co) Attached to: PATH Cache mode: writeback (qemu) quit -Testing: -blockdev driver=null-co,node-name=null -device floppy,drive=null +Testing: -blockdev driver=null-co,read-zeroes=on,node-name=null -device floppy,drive=null QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -null: null-co:// (null-co) +null: json:{"read-zeroes": true, "driver": "null-co"} (null-co) Attached to: PATH Removable device: not locked, tray closed Cache mode: writeback (qemu) quit -Testing: -blockdev driver=null-co,node-name=null -device floppy,drive=null,id=qdev_id +Testing: -blockdev driver=null-co,read-zeroes=on,node-name=null -device floppy,drive=null,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -null: null-co:// (null-co) +null: json:{"read-zeroes": true, "driver": "null-co"} (null-co) Attached to: qdev_id Removable device: not locked, tray closed Cache mode: writeback (qemu) quit -Testing: -blockdev driver=null-co,node-name=null -device ide-cd,drive=null +Testing: -blockdev driver=null-co,read-zeroes=on,node-name=null -device ide-cd,drive=null QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -null: null-co:// (null-co) +null: json:{"read-zeroes": true, "driver": "null-co"} (null-co) Attached to: PATH Removable device: not locked, tray closed Cache mode: writeback (qemu) quit -Testing: -blockdev driver=null-co,node-name=null -device ide-cd,drive=null,id=qdev_id +Testing: -blockdev driver=null-co,read-zeroes=on,node-name=null -device ide-cd,drive=null,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -null: null-co:// (null-co) +null: json:{"read-zeroes": true, "driver": "null-co"} (null-co) Attached to: qdev_id Removable device: not locked, tray closed Cache mode: writeback (qemu) quit -Testing: -blockdev driver=null-co,node-name=null -device scsi-cd,drive=null +Testing: -blockdev driver=null-co,read-zeroes=on,node-name=null -device scsi-cd,drive=null QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -null: null-co:// (null-co) +null: json:{"read-zeroes": true, "driver": "null-co"} (null-co) Attached to: PATH Removable device: not locked, tray closed Cache mode: writeback (qemu) quit -Testing: -blockdev driver=null-co,node-name=null -device scsi-cd,drive=null,id=qdev_id +Testing: -blockdev driver=null-co,read-zeroes=on,node-name=null -device scsi-cd,drive=null,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -null: null-co:// (null-co) +null: json:{"read-zeroes": true, "driver": "null-co"} (null-co) Attached to: qdev_id Removable device: not locked, tray closed Cache mode: writeback @@ -159,76 +159,76 @@ null: null-co:// (null-co) === -drive if=none/-device= === -Testing: -drive if=none,driver=null-co,node-name=null -device ide-hd,drive=null,id=qdev_id +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device ide-hd,drive=null,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Removable device: not locked, tray closed Cache mode: writeback -null: null-co:// (null-co) +null: json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: qdev_id Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device scsi-hd,drive=null,id=qdev_id +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device scsi-hd,drive=null,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Removable device: not locked, tray closed Cache mode: writeback -null: null-co:// (null-co) +null: json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: qdev_id Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device virtio-blk-pci,drive=null,id=qdev_id +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device virtio-blk-pci,drive=null,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Removable device: not locked, tray closed Cache mode: writeback -null: null-co:// (null-co) +null: json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: PATH Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device floppy,drive=null,id=qdev_id +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device floppy,drive=null,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Removable device: not locked, tray closed Cache mode: writeback -null: null-co:// (null-co) +null: json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: qdev_id Removable device: not locked, tray closed Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device ide-cd,drive=null,id=qdev_id +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device ide-cd,drive=null,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Removable device: not locked, tray closed Cache mode: writeback -null: null-co:// (null-co) +null: json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: qdev_id Removable device: not locked, tray closed Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device scsi-cd,drive=null,id=qdev_id +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device scsi-cd,drive=null,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Removable device: not locked, tray closed Cache mode: writeback -null: null-co:// (null-co) +null: json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: qdev_id Removable device: not locked, tray closed Cache mode: writeback @@ -237,103 +237,103 @@ null: null-co:// (null-co) === -drive if=none/-device= (with medium) === -Testing: -drive if=none,driver=null-co,node-name=null -device ide-hd,drive=none0 +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device ide-hd,drive=none0 QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: PATH Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device ide-hd,drive=none0,id=qdev_id +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device ide-hd,drive=none0,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: qdev_id Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device scsi-hd,drive=none0 +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device scsi-hd,drive=none0 QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: PATH Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device scsi-hd,drive=none0,id=qdev_id +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device scsi-hd,drive=none0,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: qdev_id Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device virtio-blk-pci,drive=none0 +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device virtio-blk-pci,drive=none0 QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: PATH Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device virtio-blk-pci,drive=none0,id=qdev_id +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device virtio-blk-pci,drive=none0,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: PATH Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device floppy,drive=none0 +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device floppy,drive=none0 QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: PATH Removable device: not locked, tray closed Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device floppy,drive=none0,id=qdev_id +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device floppy,drive=none0,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: qdev_id Removable device: not locked, tray closed Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device ide-cd,drive=none0 +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device ide-cd,drive=none0 QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: PATH Removable device: not locked, tray closed Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device ide-cd,drive=none0,id=qdev_id +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device ide-cd,drive=none0,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: qdev_id Removable device: not locked, tray closed Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device scsi-cd,drive=none0 +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device scsi-cd,drive=none0 QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: PATH Removable device: not locked, tray closed Cache mode: writeback (qemu) quit -Testing: -drive if=none,driver=null-co,node-name=null -device scsi-cd,drive=none0,id=qdev_id +Testing: -drive if=none,driver=null-co,read-zeroes=on,node-name=null -device scsi-cd,drive=none0,id=qdev_id QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -none0 (null): null-co:// (null-co) +none0 (null): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: qdev_id Removable device: not locked, tray closed Cache mode: writeback @@ -408,19 +408,19 @@ floppy0: [not inserted] Removable device: not locked, tray closed (qemu) quit -Testing: -drive if=floppy,driver=null-co +Testing: -drive if=floppy,driver=null-co,read-zeroes=on QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -floppy0 (NODE_NAME): null-co:// (null-co) +floppy0 (NODE_NAME): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: PATH Removable device: not locked, tray closed Cache mode: writeback (qemu) quit -Testing: -drive if=ide,driver=null-co +Testing: -drive if=ide,driver=null-co,read-zeroes=on QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -ide0-hd0 (NODE_NAME): null-co:// (null-co) +ide0-hd0 (NODE_NAME): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: PATH Cache mode: writeback (qemu) quit @@ -433,27 +433,27 @@ ide0-cd0: [not inserted] Removable device: not locked, tray closed (qemu) quit -Testing: -drive if=ide,driver=null-co,media=cdrom +Testing: -drive if=ide,driver=null-co,read-zeroes=on,media=cdrom QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -ide0-cd0 (NODE_NAME): null-co:// (null-co, read-only) +ide0-cd0 (NODE_NAME): json:{"read-zeroes": "on", "driver": "null-co"} (null-co, read-only) Attached to: PATH Removable device: not locked, tray closed Cache mode: writeback (qemu) quit -Testing: -drive if=virtio,driver=null-co +Testing: -drive if=virtio,driver=null-co,read-zeroes=on QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -virtio0 (NODE_NAME): null-co:// (null-co) +virtio0 (NODE_NAME): json:{"read-zeroes": "on", "driver": "null-co"} (null-co) Attached to: PATH Cache mode: writeback (qemu) quit -Testing: -drive if=pflash,driver=null-co,size=1M +Testing: -drive if=pflash,driver=null-co,read-zeroes=on,size=1M QEMU X.Y.Z monitor - type 'help' for more information (qemu) info block -pflash0 (NODE_NAME): json:{"driver": "null-co", "size": "1M"} (null-co) +pflash0 (NODE_NAME): json:{"read-zeroes": "on", "driver": "null-co", "size": "1M"} (null-co) Attached to: PATH Cache mode: writeback (qemu) quit diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218 index 92c331b6fb..2554d84581 100755 --- a/tests/qemu-iotests/218 +++ b/tests/qemu-iotests/218 @@ -27,9 +27,9 @@ # Creator/Owner: Max Reitz import iotests -from iotests import log +from iotests import log, qemu_img, qemu_io_silent -iotests.verify_platform(['linux']) +iotests.verify_image_format(supported_fmts=['qcow2', 'raw']) # Launches the VM, adds two null-co nodes (source and target), and @@ -136,3 +136,54 @@ with iotests.VM() as vm: log(vm.event_wait('BLOCK_JOB_CANCELLED'), filters=[iotests.filter_qmp_event]) + +log('') +log('=== Cancel mirror job from throttled node by quitting ===') +log('') + +with iotests.VM() as vm, \ + iotests.FilePath('src.img') as src_img_path: + + assert qemu_img('create', '-f', iotests.imgfmt, src_img_path, '64M') == 0 + assert qemu_io_silent('-f', iotests.imgfmt, src_img_path, + '-c', 'write -P 42 0M 64M') == 0 + + vm.launch() + + ret = vm.qmp('object-add', qom_type='throttle-group', id='tg', + props={'x-bps-read': 4096}) + assert ret['return'] == {} + + ret = vm.qmp('blockdev-add', + node_name='source', + driver=iotests.imgfmt, + file={ + 'driver': 'file', + 'filename': src_img_path + }) + assert ret['return'] == {} + + ret = vm.qmp('blockdev-add', + node_name='throttled-source', + driver='throttle', + throttle_group='tg', + file='source') + assert ret['return'] == {} + + ret = vm.qmp('blockdev-add', + node_name='target', + driver='null-co', + size=(64 * 1048576)) + assert ret['return'] == {} + + ret = vm.qmp('blockdev-mirror', + job_id='mirror', + device='throttled-source', + target='target', + sync='full') + assert ret['return'] == {} + + log(vm.qmp('quit')) + + with iotests.Timeout(5, 'Timeout waiting for VM to quit'): + vm.shutdown(has_quit=True) diff --git a/tests/qemu-iotests/218.out b/tests/qemu-iotests/218.out index 825a657081..5a86a97550 100644 --- a/tests/qemu-iotests/218.out +++ b/tests/qemu-iotests/218.out @@ -28,3 +28,7 @@ Cancelling job Cancelling job {"return": {}} {"data": {"device": "mirror", "len": 1048576, "offset": 1048576, "speed": 0, "type": "mirror"}, "event": "BLOCK_JOB_CANCELLED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} + +=== Cancel mirror job from throttled node by quitting === + +{"return": {}} diff --git a/tests/qemu-iotests/227 b/tests/qemu-iotests/227 index bdd727a721..637d7c3726 100755 --- a/tests/qemu-iotests/227 +++ b/tests/qemu-iotests/227 @@ -57,7 +57,7 @@ echo echo '=== blockstats with -drive if=virtio ===' echo -run_qemu -drive driver=null-co,if=virtio <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); + bdrv_set_backing_hd(s->detach_also, 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. + * + * 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 first 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. + * + * If removing child from parent-node-2 was successful (as it should + * be), test_drop_backing_job_commit() will then also remove the child + * from parent-node-0. + * + * With an old version of our drain infrastructure ((A) above), that + * resulted in the following flow: + * + * 1. child attempts to leave its drained section. The call recurses + * to its parents. + * + * 2. parent-node-2 leaves the drained section. Polling in + * bdrv_drain_invoke() will schedule job_exit(). + * + * 3. parent-node-1 leaves the drained section. Polling in + * bdrv_drain_invoke() will run job_exit(), thus disconnecting + * parent-node-0 from the child node. + * + * 4. bdrv_parent_drained_end() uses a QLIST_FOREACH_SAFE() loop to + * iterate over the parents. Thus, it now accesses the BdrvChild + * object that used to connect parent-node-0 and the child node. + * However, that object no longer exists, so it accesses a dangling + * pointer. + * + * The solution is to only poll once when running a bdrv_drained_end() + * operation, specifically at the end when all drained_end() + * operations for all involved nodes have been scheduled. + * Note that this also solves (A) above, thus hiding (B). + */ +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->detach_also = bs_parents[0]; + 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 +1754,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; diff --git a/tests/test-block-iothread.c b/tests/test-block-iothread.c index 79d9cf8a57..1949d5e61a 100644 --- a/tests/test-block-iothread.c +++ b/tests/test-block-iothread.c @@ -348,8 +348,8 @@ static void test_sync_op(const void *opaque) if (t->blkfn) { t->blkfn(blk); } - aio_context_release(ctx); blk_set_aio_context(blk, qemu_get_aio_context(), &error_abort); + aio_context_release(ctx); bdrv_unref(bs); blk_unref(blk); @@ -476,6 +476,7 @@ static void test_propagate_basic(void) { IOThread *iothread = iothread_new(); AioContext *ctx = iothread_get_aio_context(iothread); + AioContext *main_ctx; BlockBackend *blk; BlockDriverState *bs_a, *bs_b, *bs_verify; QDict *options; @@ -504,12 +505,14 @@ static void test_propagate_basic(void) g_assert(bdrv_get_aio_context(bs_b) == ctx); /* Switch the AioContext back */ - ctx = qemu_get_aio_context(); - blk_set_aio_context(blk, ctx, &error_abort); - g_assert(blk_get_aio_context(blk) == ctx); - g_assert(bdrv_get_aio_context(bs_a) == ctx); - g_assert(bdrv_get_aio_context(bs_verify) == ctx); - g_assert(bdrv_get_aio_context(bs_b) == ctx); + main_ctx = qemu_get_aio_context(); + aio_context_acquire(ctx); + blk_set_aio_context(blk, main_ctx, &error_abort); + aio_context_release(ctx); + g_assert(blk_get_aio_context(blk) == main_ctx); + g_assert(bdrv_get_aio_context(bs_a) == main_ctx); + g_assert(bdrv_get_aio_context(bs_verify) == main_ctx); + g_assert(bdrv_get_aio_context(bs_b) == main_ctx); bdrv_unref(bs_verify); bdrv_unref(bs_b); @@ -534,6 +537,7 @@ static void test_propagate_diamond(void) { IOThread *iothread = iothread_new(); AioContext *ctx = iothread_get_aio_context(iothread); + AioContext *main_ctx; BlockBackend *blk; BlockDriverState *bs_a, *bs_b, *bs_c, *bs_verify; QDict *options; @@ -573,13 +577,15 @@ static void test_propagate_diamond(void) g_assert(bdrv_get_aio_context(bs_c) == ctx); /* Switch the AioContext back */ - ctx = qemu_get_aio_context(); - blk_set_aio_context(blk, ctx, &error_abort); - g_assert(blk_get_aio_context(blk) == ctx); - g_assert(bdrv_get_aio_context(bs_verify) == ctx); - g_assert(bdrv_get_aio_context(bs_a) == ctx); - g_assert(bdrv_get_aio_context(bs_b) == ctx); - g_assert(bdrv_get_aio_context(bs_c) == ctx); + main_ctx = qemu_get_aio_context(); + aio_context_acquire(ctx); + blk_set_aio_context(blk, main_ctx, &error_abort); + aio_context_release(ctx); + g_assert(blk_get_aio_context(blk) == main_ctx); + g_assert(bdrv_get_aio_context(bs_verify) == main_ctx); + g_assert(bdrv_get_aio_context(bs_a) == main_ctx); + g_assert(bdrv_get_aio_context(bs_b) == main_ctx); + g_assert(bdrv_get_aio_context(bs_c) == main_ctx); blk_unref(blk); bdrv_unref(bs_verify); @@ -685,7 +691,9 @@ static void test_attach_second_node(void) g_assert(bdrv_get_aio_context(bs) == ctx); g_assert(bdrv_get_aio_context(filter) == ctx); + aio_context_acquire(ctx); blk_set_aio_context(blk, main_ctx, &error_abort); + aio_context_release(ctx); g_assert(blk_get_aio_context(blk) == main_ctx); g_assert(bdrv_get_aio_context(bs) == main_ctx); g_assert(bdrv_get_aio_context(filter) == main_ctx); @@ -712,7 +720,9 @@ static void test_attach_preserve_blk_ctx(void) g_assert(bdrv_get_aio_context(bs) == ctx); /* Remove the node again */ + aio_context_acquire(ctx); blk_remove_bs(blk); + aio_context_release(ctx); g_assert(blk_get_aio_context(blk) == ctx); g_assert(bdrv_get_aio_context(bs) == qemu_get_aio_context()); @@ -721,7 +731,9 @@ static void test_attach_preserve_blk_ctx(void) g_assert(blk_get_aio_context(blk) == ctx); g_assert(bdrv_get_aio_context(bs) == ctx); + aio_context_acquire(ctx); blk_set_aio_context(blk, qemu_get_aio_context(), &error_abort); + aio_context_release(ctx); bdrv_unref(bs); blk_unref(blk); } diff --git a/vl.c b/vl.c index a5808f9a02..53335a5470 100644 --- a/vl.c +++ b/vl.c @@ -4480,6 +4480,17 @@ int main(int argc, char **argv, char **envp) */ migration_shutdown(); + /* + * We must cancel all block jobs while the block layer is drained, + * or cancelling will be affected by throttling and thus may block + * for an extended period of time. + * vm_shutdown() will bdrv_drain_all(), so we may as well include + * it in the drained section. + * We do not need to end this section, because we do not want any + * requests happening from here on anyway. + */ + bdrv_drain_all_begin(); + /* No more vcpu or device emulation activity beyond this point */ vm_shutdown();