On 01.04.22 18:08, Vladimir Sementsov-Ogievskiy wrote:
01.04.2022 16:16, Hanna Reitz wrote:
On 01.04.22 11:19, Vladimir Sementsov-Ogievskiy wrote:
Add possibility to limit block_copy() call in time. To be used in the
next commit.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@openvz.org>
---
block/block-copy.c | 26 +++++++++++++++++++-------
block/copy-before-write.c | 2 +-
include/block/block-copy.h | 2 +-
3 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index ec46775ea5..b47cb188dd 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
[...]
@@ -894,12 +902,16 @@ int coroutine_fn block_copy(BlockCopyState *s,
int64_t start, int64_t bytes,
.max_workers = BLOCK_COPY_MAX_WORKERS,
};
- return block_copy_common(&call_state);
-}
+ ret = qemu_co_timeout(block_copy_async_co_entry, call_state,
timeout_ns,
+ g_free);
A direct path for timeout_ns == 0 might still be nice to have.
+ if (ret < 0) {
+ /* Timeout. call_state will be freed by running coroutine. */
Maybe assert(ret == -ETIMEDOUT);?
OK
+ return ret;
If I’m right in understanding how qemu_co_timeout() works,
block_copy_common() will continue to run here. Shouldn’t we at least
cancel it by setting call_state->cancelled to true?
Agree
(Besides this, I think that letting block_copy_common() running in
the background should be OK. I’m not sure what the implications are
if we do cancel the call here, while on-cbw-error is
break-guest-write, though. Should be fine, I guess, because
block_copy_common() will still correctly keep track of what it has
successfully copied and what it hasn’t?)
Hmm. I now think, that we should at least wait for such cancelled
background requests before block_copy_state_free in cbw_close(). But
in "[PATCH v5 00/45] Transactional block-graph modifying API" I want
to detach children from CBW filter before calling .close().. So,
possible solution is to wait for all cancelled requests on
.bdrv_co_drain_begin().
Or alternatively, may be just increase bs->in_flight for CBW filter
for each background cancelled request? And decrease when it finish.
For this we should add a kind of callback to be called when timed-out
coroutine entry finish.
in_flight sounds good to me. That would automatically work for
draining, right?
