Re: [PATCH v2 7/7] block-copy: protect BlockCopyState .method fields - Vladimir Sementsov-Ogievskiy

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
	Emanuele Giuseppe Esposito <eesposit@redhat.com>,
	qemu-block@nongnu.org
Cc: John Snow <jsnow@redhat.com>, Kevin Wolf <kwolf@redhat.com>,
	Max Reitz <mreitz@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	qemu-devel@nongnu.org
Subject: Re: [PATCH v2 7/7] block-copy: protect BlockCopyState .method fields
Date: Fri, 28 May 2021 15:44:47 +0300	[thread overview]
Message-ID: <51212a88-375e-5591-fbe6-6eae3959f71b@virtuozzo.com> (raw)
In-Reply-To: <fc91616b-9d6f-dfee-d648-30828a13c1b8@redhat.com>

28.05.2021 14:01, Paolo Bonzini wrote:
> On 28/05/21 12:24, Paolo Bonzini wrote:
>>
>> It's still more complicated, because you need to add some kind of
>>
>>      method = s->method;
> 
> This would even have to be a separate, one-line critical section...
> 

Hm, so, we should set .use_copy_range in task, when it is initialized.

> 
>>      ret = block_copy_do_copy(..., method);
>>      if (ret < 0 && method <= COPY_RANGE_SMALL) {
>>          method = COPY_RANGE_READ_WRITE;
>>          ret = block_copy_do_copy(..., method);
>>          }
>>      lock();
>>          if (method == s->method) {
>>              /* compute new method */
>>          }
>>
>> which makes it more complicated than this patch IMO.  But yeah at least it's a viable alternative to the atomics.
> 
> 

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.

-- 
Best regards,
Vladimir