Re: [PATCH 0/2] block-copy: small fix and refactor

[Emanuele Giuseppe Esposito <eesposit@redhat.com>]

On 07/06/2021 18:18, Vladimir Sementsov-Ogievskiy wrote:
> 07.06.2021 18:16, Emanuele Giuseppe Esposito wrote:
>>
>>
>> On 07/06/2021 17:10, Kevin Wolf wrote:
>>> Am 03.06.2021 um 09:38 hat Paolo Bonzini geschrieben:
>>>> On 02/06/21 14:21, Kevin Wolf wrote:
>>>>> Am 02.06.2021 um 11:13 hat Stefan Hajnoczi geschrieben:
>>>>>> On Fri, May 28, 2021 at 05:16:26PM +0300, Vladimir 
>>>>>> Sementsov-Ogievskiy wrote:
>>>>>>> Hi all!
>>>>>>>
>>>>>>> This is my suggestion how to refactor block-copy to avoid extra 
>>>>>>> atomic
>>>>>>> operations in
>>>>>>> "[PATCH v2 0/7] block-copy: protect block-copy internal structures"
>>>>>>>
>>>>>>> Vladimir Sementsov-Ogievskiy (2):
>>>>>>>     block-copy: fix block_copy_task_entry() progress update
>>>>>>>     block-copy: refactor copy_range handling
>>>>>>>
>>>>>>>    block/block-copy.c | 79 
>>>>>>> +++++++++++++++++++++++++++++++---------------
>>>>>>>    1 file changed, 53 insertions(+), 26 deletions(-)
>>>>>>
>>>>>> I posted suggestions for the doc comment on Patch 2, otherwise:
>>>>>>
>>>>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>>>>>
>>>>> Thanks, fixed up the comment accordingly and applied to the block
>>>>> branch.
>>>>
>>>> I'm a bit confused.  Vladimir said in his review of Emanuele's patches
>>>> that he was okay with patch 7 and that he would rebase this
>>>> refactoring on top of it.
>>>>
>>>> Vladimir's main complaint for the s->method state machine was the
>>>> extra lines of code.  Here we have just as many new lines of code and
>>>> new parameters that are passed by reference.  Kevin, can you please
>>>> look at Emanuele's patches and possibly unqueue the second patch here?
>>>> It seems to me that it should have been tagged as RFC.
>>>
>>> Sorry, I was not aware that Vladimir intended to rebase this one. This
>>> has already landed in master, so if rebasing the other patch is a real
>>> problem, we'd have to revert this one first.
>>>
>> It shouldn't be a problem, I have already rebased on top of it. I will 
>> re-spin a new series with this and other minor (and hopefully final) 
>> fixes soon.
>>
> 
> Thanks, and sorry for the mess!
> 
> Hmm, actually, I said
> 
>> OK, I'm OK with patch as is. Finally I can refactor it later on top if 
>> needed.. I'll try now do some refactoring, you'll probably want to 
>> base on it, or vise-versa, I'll rebase it later on top of these patches. 
> 
> So, I considered both variants. Then I sent patches, everybody in CC, 
> everybody were silent.
> 
> 
> Honestly, I'm a bit confused too. I find my complains valid 
> (independently of me being "I'm OK and can refactor later") and you 
> agreed with them in general. I'm an author and maintainer of the 
> component. I do refactoring that makes it simple to follow my 
> suggestion. So for me it's a bit like doing your work for you. And you 
> ask to roll-back it.

I think it's useless to discuss about these things now. I rebased, all 
is clear and I am positive that in the next version we will have 
something that makes everyone happy :) and if not, feel free to comment it!

Emanuele

> 
> Still, misunderstanding and the mess with two parallel conflicting 
> series is my fault, sorry for this. At least I should have answered to 
> your series when Stefan gave an r-b to my series.
>