[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RESEND v4] drive-mirror: add incremental mode
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH RESEND v4] drive-mirror: add incremental mode |
Date: |
Wed, 13 Mar 2019 12:52:04 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
On 3/12/19 12:32 PM, Vladimir Sementsov-Ogievskiy wrote:
> 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 ?
>
It's a thought. Fam's original sketch for bitmaps included a separate
parameter for how to clean up the bitmap at the end, but it was removed
because we didn't have use cases for it at the time.
> In this case, we can get NONE mode reduced by bitmap, which may be usable too.
What do you have in mind for the use case?
> 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.
>
In effect, a disk-backed buffer so we don't lose performance on slow
network links, right?
> 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.
>
This could be very cool; how does this vision fit in with the vision for
a unified block-copy job? (i.e. unifying mirror, stream, backup and commit.)
Worth thinking about, because we also want the ability to "resume"
mirror jobs, too.
Regardless, I think the correct thing to do in the short term is to add
a differential sync, since we already have sync modes that determine how
to use the bitmap, and this would be part of a newer paradigm that we
can introduce later.
--js