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

[John Snow <jsnow@redhat.com>]

On 01/22/2018 07:22 AM, Vladimir Sementsov-Ogievskiy wrote:
> 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
> 

Great, thanks!