[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 08/20] block/block-copy: add block_copy_cancel
From: |
Max Reitz |
Subject: |
Re: [PATCH v2 08/20] block/block-copy: add block_copy_cancel |
Date: |
Wed, 4 Nov 2020 18:26:04 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 |
On 22.10.20 22: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 <vsementsov@virtuozzo.com>
>>> ---
>>> 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
OK.
Max
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH v2 08/20] block/block-copy: add block_copy_cancel,
Max Reitz <=