Re: [PATCH v4 6/6] block-copy: atomic .cancelled and .finished fields in BlockCopyCallState

[Paolo Bonzini]

On 19/06/21 22:06, Vladimir Sementsov-Ogievskiy wrote:
> > -    assert(call_state->finished);
> > +    assert(qatomic_load_acquire(&call_state->finished));
>
> Hmm. Here qatomic_load_acquire protects nothing (assertion will crash if 
> not yet finished anyway). So, caller is double sure that block-copy is 
> finished.

It does.  If it returns true, you still want the load of finished to 
happen before the reads that follow.

Otherwise I agree with your remarks.

Paolo

> Also it's misleading: if we think that it do some protection, we are 
> doing wrong thing: assertions may be simply compiled out, we can't rely 
> on statements inside assert() to be executed.
>
> So, let's use simple qatomic_read here too.
>
> >       if (error_is_read) {
> >           *error_is_read = call_state->error_is_read;
> >       }