Re: [Qemu-devel] [PATCH for-2.12 0/4] qmp dirty bitmap API

[John Snow]

On 11/17/2017 03:22 AM, Vladimir Sementsov-Ogievskiy wrote:
> 17.11.2017 06:10, John Snow wrote:
>>
>> On 11/16/2017 03:17 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> 16.11.2017 00:20, John Snow wrote:
>>>> On 11/13/2017 11:20 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
>>>>>
>>>> Can you give me an example of how you expect these commands to be used,
>>>> and why they're required?
>>>>
>>>> I'm a little weary about how error-prone these commands might be and the
>>>> potential for incorrect usage seems... high. Why do we require them?
>>> It is needed for incremental backup. It looks like bad idea to export
>>> abdicate/reclaim functionality, it is simpler
>>> and clearer to allow user to merge/enable/disable bitmaps by hand.
>>>
>>> usage is like this:
>>>
>>> 1. we have dirty bitmap bitmap0 for incremental backup.
>>>
>>> 2. prepare image fleecing (create temporary image with backing=our_disk)
>>> 3. in qmp transaction:
>>>     - disable bitmap0
>>>     - create bitmap1
>>>     - start image fleecing (backup sync=none our_disk -> temp_disk)
>> This could probably just be its own command, though:
>>
>> block-job-fleece node=foobar bitmap=bitmap0 etc=etera etc=etera
>>
>> Could handle forking the bitmap. I'm not sure what the arguments would
>> look like, but we could name the NBD export here, too. (Assuming the
>> server is already started and we just need to create the share.)
>>
>> Then, we can basically do what mirror does:
>>
>> (1) Cancel
>> (2) Complete
>>
>> Cancel would instruct QEMU to keep the bitmap changes (i.e. roll back),
>> and Complete would instruct QEMU to discard the changes.
>>
>> This way we don't need to expose commands like split or merge that will
>> almost always be dangerous to use over QMP.
>>
>> In fact, a fleecing job would be really convenient even without a
>> bitmap, because it'd still be nice to have a convenience command for it.
>> Using an existing infrastructure and understood paradigm is just a bonus.
> 
> 1. If I understand correctly, Kevin and Max said in their report in
> Prague about new block-job approach,
>   using filter nodes, so I'm not sure that this is a good Idea to
> introduce now new old-style block-job, where we can
>   do without it.
> 

We could do without it, but it might be a lot better to have everything
wrapped up in a command that's easy to digest instead of releasing 10
smaller commands that have to be executed in a very specific way in
order to work correctly.

I'm thinking about the complexity of error checking here with all the
smaller commands, versus error checking on a larger workflow we
understand and can quality test better.

I'm not sure that filter nodes becoming the new normal for block jobs
precludes our ability to use the job-management API as a handle for
managing the lifetime of a long-running task like fleecing, but I'll
check with Max and Kevin about this.

> 2. there is the following scenario: customers needs a possibility to
> create a backup of data changed since some
> point in time. So, maintaining several static and one (or several) activ
> bitmaps with a possiblity of merge some of them
> and create a backup using this merged bitmap may be convenient.
> 

I think the ability to copy bitmaps and issue differential backups would
be sufficient in all cases I could think of...

> 3. And I don't see any danger here: we already can create, delete and
> clear dirty bitmaps through qmp. Some additional
> operations should not be bad.
> 

Not bad, just risk of misuse for little benefit, IMO. I'm wary of
raising the external complexity too much. I'd like to keep the API as
simple as I can.

> 4. Interesting: "disabled" field looks like never used..

Ah! That's true. It was in an early draft and Stefan noticed that there
was no reason to let users disable or enable bitmaps in the first
version of the feature set, so those QMP commands (along with copy) got
removed to keep the feature set smaller.

The mechanisms that paid attention to a bitmap being enabled or not
remained behind, though.

> 
>>
>>> 4. export temp_disk (it looks like a snapshot) and bitmap0 through NBD
>>>
>>> === external tool can get bitmap0 and then copy some clusters through
>>> NBD ===
>>>
>>> on successful backup external tool can drop bitmap0. or can save it and
>>> reuse somehow
>>>
>>> on failed backup external tool can do the following:
>>>     - enable bitmap0
>>>     - merge bitmap1 to bitmap0
>>>     - drop bitmap1
>>>
> 
> 
> -- 
> Best regards,
> Vladimir
>