Re: [Qemu-block] [Qemu-devel] [PATCH RESEND v4] drive-mirror: add incremental mode

[Vladimir Sementsov-Ogievskiy]

12.03.2019 18:58, John Snow wrote:
> 
> 
> On 3/12/19 9:43 AM, Eric Blake wrote:
>> On 3/12/19 3:19 AM, Vladimir Sementsov-Ogievskiy wrote:
>>
>>>> So one important point about incremental backups is that you can
>>>> actually do them incrementally: The bitmap is effectively cleared at the
>>>> beginning of the backup process (a successor bitmap is installed that is
>>>> cleared and receives all changes; at the end of the backup, it either
>>>> replaces the old bitmap (on success) or is merged into it (on failure)).
>>>>    Therefore, you can do the next incremental backup by using the same 
>>>> bitmap.
>>>
>>> Hmm, I heard in some other thread that Eric finally decided to always start
>>> backup on copied bitmap in libvirt, so this logic is skipped. Do we need it
>>> for mirror? Sorry if I'm wrong, Eric, am I?
>>
>> You are correct that my current libvirt patches do incremental backup
>> mode with a temporary bitmap (so regardless of whether the temporary
>> bitmap is cleared on success or left alone is irrelevant, the temporary
>> bitmap goes away afterwards). But just because libvirt isn't using that
>> particular feature of qemu's incremental backups does not excuse qemu
>> from not thinking about other clients that might be impacted by doing
>> incremental backup with a live bitmap, where qemu management of the
>> bitmap matters.
>>
> 
> I'd prefer adding SYNC=DIFFERENTIAL to intuit that the bitmap isn't
> modified after use, if there's no reason to add an actual "incremental"
> mode to mirror.
> 
> Then it would be fine for mirror to implement differential and not
> incremental, and still use the bitmap.
> 

Agree, this is better.

But I have an idea: could we just add parameter @x-bitmap, independent of @sync 
?

In this case, we can get NONE mode reduced by bitmap, which may be usable too.
And FULL mode with bitmap would be what you called DIFFERENTIAL. Not sure about
could TOP mode with bitmap be meaningful.


Also it may be useful at some point to share bitmap between jobs, so we could

start backup(sync=none) from active disk to local temp node, and then mirror 
from temp
to target. Which gives backup which (1) don't slow down guest writes if target 
is far and slow
and (2) fast as it is mostly mirror, keeping in mind that mirror is faster than 
backup as it uses
async pattern and block_status.

And to make such a backup incremental we need to share dirty bitmap between 
backup and mirror,
and this bitmap should be cleared when something is copied out from source (it 
may be cleared
both by backup(sync=none) and mirror)... But I'm not sure that shared bitmap 
should be the same
as bitmap which initializes differential/incremental mode. And this is why I 
propose new parameter
to be experimental.

-- 
Best regards,
Vladimir