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

2021-05-28 Thread Vladimir Sementsov-Ogievskiy
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,

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

2021-05-28 Thread Vladimir Sementsov-Ogievskiy
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... Or atomic operation.. What I don't like that all

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

2021-05-28 Thread Paolo Bonzini
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... Paolo ret = block_copy_do_copy(..., method); if (ret < 0 && method <=

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

2021-05-28 Thread Paolo Bonzini
On 26/05/21 19:18, Vladimir Sementsov-Ogievskiy wrote: It's actually the original idea of block_copy_do_copy() function: do only simple copy, don't interact with the state. It's a kind of wrapper on top of bdrv_co. So, actually updating s->use_copy_range here was a bad idea. It's still

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

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
26.05.2021 17:48, Paolo Bonzini wrote: 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

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

2021-05-26 Thread Paolo Bonzini
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

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

2021-05-25 Thread Vladimir Sementsov-Ogievskiy
25.05.2021 13:18, Emanuele Giuseppe Esposito wrote: On 21/05/2021 19:10, Vladimir Sementsov-Ogievskiy wrote: 18.05.2021 13:07, Emanuele Giuseppe Esposito wrote: With tasks and calls lock protecting all State fields, .method is the last BlockCopyState field left unprotected. Set it as atomic.

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

2021-05-25 Thread Emanuele Giuseppe Esposito
On 21/05/2021 19:10, Vladimir Sementsov-Ogievskiy wrote: 18.05.2021 13:07, Emanuele Giuseppe Esposito wrote: With tasks and calls lock protecting all State fields, .method is the last BlockCopyState field left unprotected. Set it as atomic. Signed-off-by: Emanuele Giuseppe Esposito OK,

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

2021-05-21 Thread Vladimir Sementsov-Ogievskiy
18.05.2021 13:07, Emanuele Giuseppe Esposito wrote: With tasks and calls lock protecting all State fields, .method is the last BlockCopyState field left unprotected. Set it as atomic. Signed-off-by: Emanuele Giuseppe Esposito OK, in 06 some things are out of coroutine. Here could we just