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 >>>>>> 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?
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 none-mode-mirror :) > >> 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? Yes > >> 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. No objections)
