22.10.2020 23:50, Vladimir Sementsov-Ogievskiy wrote:
22.07.2020 14:28, Max Reitz wrote:
On 01.06.20 20:11, Vladimir Sementsov-Ogievskiy wrote:
Add function to cancel running async block-copy call. It will be used
in backup.
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
---
include/block/block-copy.h | 7 +++++++
block/block-copy.c | 22 +++++++++++++++++++---
2 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index d40e691123..370a194d3c 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -67,6 +67,13 @@ BlockCopyCallState *block_copy_async(BlockCopyState *s,
void block_copy_set_speed(BlockCopyState *s, BlockCopyCallState *call_state,
uint64_t speed);
+/*
+ * Cancel running block-copy call.
+ * Cancel leaves block-copy state valid: dirty bits are correct and you may use
+ * cancel + <run block_copy with same parameters> to emulate pause/resume.
+ */
+void block_copy_cancel(BlockCopyCallState *call_state);
+
BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s);
void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip);
diff --git a/block/block-copy.c b/block/block-copy.c
index 851d9c8aaf..b551feb6c2 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -44,6 +44,8 @@ typedef struct BlockCopyCallState {
bool failed;
bool finished;
QemuCoSleepState *sleep_state;
+ bool cancelled;
+ Coroutine *canceller;
/* OUT parameters */
bool error_is_read;
@@ -582,7 +584,7 @@ block_copy_dirty_clusters(BlockCopyCallState *call_state)
assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
assert(QEMU_IS_ALIGNED(bytes, s->cluster_size));
- while (bytes && aio_task_pool_status(aio) == 0) {
+ while (bytes && aio_task_pool_status(aio) == 0 && !call_state->cancelled) {
BlockCopyTask *task;
int64_t status_bytes;
@@ -693,7 +695,7 @@ static int coroutine_fn
block_copy_common(BlockCopyCallState *call_state)
do {
ret = block_copy_dirty_clusters(call_state);
- if (ret == 0) {
+ if (ret == 0 && !call_state->cancelled) {
ret = block_copy_wait_one(call_state->s, call_state->offset,
call_state->bytes);
}
@@ -707,13 +709,18 @@ static int coroutine_fn
block_copy_common(BlockCopyCallState *call_state)
* 2. We have waited for some intersecting block-copy request
* It may have failed and produced new dirty bits.
*/
- } while (ret > 0);
+ } while (ret > 0 && !call_state->cancelled);
Would it be cleaner if block_copy_dirty_cluster() just returned
-ECANCELED? Or would that pose a problem for its callers or the async
callback?
I'd prefer not to merge io ret with block-copy logic: who knows what underlying
operations may return.. Can't it be _another_ ECANCELED?
And it would be just a sugar for block_copy_dirty_clusters() call, I'll have to
check ->cancelled after block_copy_wait_one() anyway.
Also, for the next version I try to make it more obvious that finished
block-copy call is in one of thee states:
- success
- failed
- cancelled
Hmm. Also, cancelled should be OK for copy-on-write operations in filter, it
just mean that we don't need to care anymore.
This is unrelated: actually only async block-copy call may be cancelled.
if (call_state->cb) {
call_state->cb(ret, call_state->error_is_read,
call_state->s->progress_opaque);
}
+ if (call_state->canceller) {
+ aio_co_wake(call_state->canceller);
+ call_state->canceller = NULL;
+ }
+
call_state->finished = true;
return ret;
--
Best regards,
Vladimir
