Re: [Qemu-devel] [PATCH v2 0/6] qmp dirty bitmap API

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

22.01.2018 12:20, Vladimir Sementsov-Ogievskiy wrote:
> 20.01.2018 02:30, John Snow wrote:
>>
>> On 01/16/2018 07:54 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> Hi all.
>>>
>>> There are three qmp commands, needed to implement external backup API.
>>>
>>> Using these three commands, client may do all needed bitmap 
>>> management by
>>> hand:
>>>
>>> on backup start we need to do a transaction:
>>>   {disable old bitmap, create new bitmap}
>>>
>>> on backup success:
>>>   drop old bitmap
>>>
>>> on backup fail:
>>>   enable old bitmap
>>>   merge new bitmap to old bitmap
>>>   drop new bitmap
>>>
>>> v2: fix merge command deadlock
>>>    add new patches: 1 and 6
>>>
>>> Vladimir Sementsov-Ogievskiy (6):
>>>    block: maintain persistent disabled bitmaps
>>>    block/dirty-bitmap: add lock to bdrv_enable/disable_dirty_bitmap
>>>    qapi: add block-dirty-bitmap-enable/disable
>>>    qmp: transaction support for block-dirty-bitmap-enable/disable
>>>    qapi: add block-dirty-bitmap-merge
>>>    qapi: add disabled parameter to block-dirty-bitmap-add
>>>
>>>   qapi/block-core.json         |  92 ++++++++++++++++++++++-
>>>   qapi/transaction.json        |   4 +
>>>   block/qcow2.h                |   2 +-
>>>   include/block/dirty-bitmap.h |   3 +-
>>>   block/dirty-bitmap.c         |  42 ++++++-----
>>>   block/qcow2-bitmap.c         |  12 +--
>>>   block/qcow2.c                |   2 +-
>>>   blockdev.c                   | 169 
>>> +++++++++++++++++++++++++++++++++++++++++--
>>>   8 files changed, 287 insertions(+), 39 deletions(-)
>>>
>> Fails to apply to master (b384cd95) on patch four and five. Only
>> contextual problems, I've patched it up and I'll review that.
>>
>> (mirrored here if you want to check my rebase work:
>> https://github.com/jnsnow/qemu/tree/vlad-review)
>>
>> Since I was full of such bad and stupid ideas last time, I'd like
>> someone else to look over this one for design and I'll just review it
>> for accuracy.
>>
>> --js
>
> Thank you for review, John!
>
> Ok, so, I'll going to:
>
> - take patch 1 into migration and respin it today (I hope) with test 
> about qcow2-based bitmap migration disabled.
> - separate fixes and refactoring from here (locking + _bitmap_clear 
> transaction), send them separately
> - than, make test for external backup and respin these series with it
>

changed to:

1. send patch 1/6 separately with the whole reasoning[done], as it 
blocks two series, wait for accepting
2. respin postcopy series
3. finish up discussion on bitmap locking under "[PATCH v9 03/13] 
block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap"
4. separate fixes and refactoring from here (locking + _bitmap_clear 
transaction), send them separately
5. make test for external backup and respin these series with it

2 depends on 1
4 depends on 3
5 depends on 1 and 4

-- 
Best regards,
Vladimir