[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH RESEND v4] drive-mirror: add increm
Re: [Qemu-block] [Qemu-devel] [PATCH RESEND v4] drive-mirror: add incremental mode
Thu, 14 Mar 2019 06:52:40 +0000
On 13.03.2019 19:52, John Snow wrote:
> 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
>>>>> Hmm, I heard in some other thread that Eric finally decided to always
>>>>> backup on copied bitmap in libvirt, so this logic is skipped. Do we need
>>>>> 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
> What do you have in mind for the use case?
I think about backup(sync=none) which lacks bitmap support for the idea
described below. For mirror - I don't know) To imagine this, I need to
know original reason for incremental-mirror and original reason for
>> And FULL mode with bitmap would be what you called DIFFERENTIAL. Not sure
>> 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.)
My current plan is to finish backup top filter first. Then may be
implement disk-backed buffer inside backup, to don't start two jobs. And
then make backup as fast as mirror. Or, may be it would be backup-top
filter, who will do most of work, so it should be fast too..
I often think about unifying jobs code, but when I try to dig in, it
becomes too difficult. I think, we should start from separating common
code to some kind of library, separate file. And first thing to separate
seems to be an async copying loop backed by coroutines and
block_status.. To share it between all jobs and qemu-img convert. But
even here, there are differences, for example, mirror is the only job
which needs several iterations of the loop. Backup needs to share
block_status results with backup_top filter, to make it useful for
copy-before-write operations.. Mirror needs block_status of active disk
and backup actually need block_status of point-in-time..
And mirror code is tooo difficult, it's a problem too;)
But there must be some generic part which we may separate to start from.
And the idea is to start from unifying code-path, not interface.
> 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.