On 25.09.19 21:28, Vladimir Sementsov-Ogievskiy wrote: > ops, I've sent unfinished message > > On 25.09.2019 22:19, Vladimir Sementsov-Ogievskiy wrote: >> Ogh :( >> >> And I realized that there is bigger problem with design: >> >> Assume failed copy in filter request: we want to mark bits dirty again >> and release range lock on source.. But if we have some write reguests >> in parallel, they may already passed backup-top filter, and they are >> only waiting for range lock. When lock is free the will go on and will >> not see bitmap changes.. >> >> That means that we can't use range lock: waiting request must wait on >> backup-top level, but range lock will not work on it, as they will >> interfer with original write request. > > With such design we can't mark bits dirty again. We can switch to other > behavior: on failed block-copy in filter just cancel the whole > block-job.. But actually I think both behaviors should be available for > user: > 1. if backup is important, better to fail guest writes if needed > 2. if guest is important, better to fail backup job if failed to do > copy-before-write > >> >> I have to rething it somehow, a kind of "intersecting requests" possibly >> will be kept. I still don't like that current backup write-notifier >> locks the whole region, even non-dirty bits, instead we should lock only >> the region which we are handling at the moment. >> >> Patches 01-11 are still good themselves, as a preparation, let's keep them >> >> Patches 12-13 are good, but range lock is not appropriate for backup.. >> May be they will be used for rewriting copy-on-read filter to copy in >> filter code.. Still I'm not sure, as COR should work through block-copy >> finally, and may possibly reuse same locking. > > better drop 12-13 for now > > Patch 14 is good, let's keep it. It has correct abort() in > backup_top_cbw(), it's not dependent on 12-13, and it's waiting for > corrected combining of backup-top, backup and block-copy. > > And patch 15 is bad, I'll rewrite it. So, 16 is not needed too. I’ll drop 12 – 15. 14 is dead code without 15, so I’d rather not keep it, actually.
Mxa
signature.asc
Description: OpenPGP digital signature