On 25/05/21 13:00, Vladimir Sementsov-Ogievskiy wrote:
Hmm. OK, let me think:
First look at block_copy_do_copy(). It's called only from
block_copy_task_entry. block_copy_task_entry() has
mutex-critical-section anyway around handling return value. That means
that we can simply move s->method modification logic to this already
existing critical section.
Next, block_copy_chunk_size() is called only from
block_copy_task_create(), where we should have critical section too.
block_copy_do_copy would have to release the mutex around the
reads/writes (including the bdrv_co_pwrite_zeroes that has nothing to do
with s->method).
There's also the "goto out" if the copy-range operation succeeds, which
makes things a bit more complicated. The goto suggests using
QEMU_WITH_LOCK_GUARD, but that doesn't work too well either, because
there are two accesses (one before the bdrv_co_copy_range and one after).
So I understand why you want to avoid atomics and I agree that they are
more complicated than the other solutions, on the other hand I think
this patch is the simplest code.
Paolo
So, no reason for atomics, as we already have critical sections.
