Re: [PATCH v2 04/20] block/block-copy: More explicit call_state

[Max Reitz]

On 18.09.20 22:11, Vladimir Sementsov-Ogievskiy wrote:
> 17.07.2020 16:45, Max Reitz wrote:
>> On 01.06.20 20:11, Vladimir Sementsov-Ogievskiy wrote:
>>> Refactor common path to use BlockCopyCallState pointer as parameter, to
>>> prepare it for use in asynchronous block-copy (at least, we'll need to
>>> run block-copy in a coroutine, passing the whole parameters as one
>>> pointer).
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>>   block/block-copy.c | 51 ++++++++++++++++++++++++++++++++++------------
>>>   1 file changed, 38 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/block/block-copy.c b/block/block-copy.c
>>> index 43a018d190..75882a094c 100644
>>> --- a/block/block-copy.c
>>> +++ b/block/block-copy.c
>>
>> [...]
>>
>>> @@ -646,16 +653,16 @@ out:
>>>    * it means that some I/O operation failed in context of _this_
>>> block_copy call,
>>>    * not some parallel operation.
>>>    */
>>> -int coroutine_fn block_copy(BlockCopyState *s, int64_t offset,
>>> int64_t bytes,
>>> -                            bool *error_is_read)
>>> +static int coroutine_fn block_copy_common(BlockCopyCallState
>>> *call_state)
>>>   {
>>>       int ret;
>>>         do {
>>> -        ret = block_copy_dirty_clusters(s, offset, bytes,
>>> error_is_read);
>>> +        ret = block_copy_dirty_clusters(call_state);
>>
>> It’s possible that much of this code will change in a future patch of
>> this series, but as it is, it might be nice to make
>> block_copy_dirty_clusters’s argument a const pointer so it’s clear that
>> the call to block_copy_wait_one() below will use the original @offset
>> and @bytes values.
> 
> Hm. I'm trying this, and it doesn't work:
> 
> block_copy_task_entry() wants to change call_state:
> 
>    t->call_state->failed = true;

Too bad :)

>> *shrug*
>>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>>
>>>             if (ret == 0) {
>>> -            ret = block_copy_wait_one(s, offset, bytes);
>>> +            ret = block_copy_wait_one(call_state->s,
>>> call_state->offset,
>>> +                                      call_state->bytes);
>>>           }
>>>             /*
>>
> 
>

signature.asc
Description: OpenPGP digital signature