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

Vladimir Sementsov-Ogievskiy posted 6 patches 6 years, 3 months ago
Failed in applying to current master (apply log)
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(-)
[Qemu-devel] [PATCH v2 0/6] qmp dirty bitmap API
Posted by Vladimir Sementsov-Ogievskiy 6 years, 3 months ago
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(-)

-- 
2.11.1


Re: [Qemu-devel] [PATCH v2 0/6] qmp dirty bitmap API
Posted by John Snow 6 years, 3 months ago

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

Re: [Qemu-devel] [PATCH v2 0/6] qmp dirty bitmap API
Posted by Vladimir Sementsov-Ogievskiy 6 years, 3 months ago
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

-- 
Best regards,
Vladimir


Re: [Qemu-devel] [PATCH v2 0/6] qmp dirty bitmap API
Posted by Vladimir Sementsov-Ogievskiy 6 years, 3 months ago
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


Re: [Qemu-devel] [PATCH v2 0/6] qmp dirty bitmap API
Posted by John Snow 6 years, 3 months ago

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!

Re: [Qemu-devel] [PATCH v2 0/6] qmp dirty bitmap API
Posted by Vladimir Sementsov-Ogievskiy 6 years, 2 months ago
22.01.2018 20:23, John Snow wrote:
>
> 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!

Sorry for long delay, I was ill. Now I'm returning to these plans.

-- 
Best regards,
Vladimir