Re: [PATCH 1/2] blockdev: qmp_transaction: harden transaction properties for bitmaps

[Vladimir Sementsov-Ogievskiy]

On 04.09.23 11:31, Andrey Zhadchenko wrote:
> Unlike other transaction commands, bitmap operations do not drain target
> bds. If we have an IOThread, this may result in some inconsistencies, as
> bitmap content may change during transaction command.
> Add bdrv_drained_begin()/end() to bitmap operations.
>
> Signed-off-by: Andrey Zhadchenko<andrey.zhadchenko@virtuozzo.com>

Hi!

First, please always include cover letter when more than 1 patch.

Next. Hmm. Good idea, but I'm afraid that's still not enough.

Assume you have two BSs A and B in two different iothreads. So, the sequence 
may be like this:

1. drain_begin A

2. do operation with bitmap in A

3. guest writes to B, B is modified and bitmap in B is modified as well

4. drain_begin B

5. do operation with bitmap in B

6. drain_end B

7. drain_end A

User may expect, that all the operations are done atomically in relation to any 
guest IO operations. And if operations are dependent, the intermediate write 
[3] make break the result.

So, we should drain all participant drives during the whole transactions. The 
simplest solution is bdrv_drain_all_begin() / bdrv_drain_all_end() pair in 
qmp_transaction(), could we start with it?

--
Best regards,
Vladimir