Re: [PATCH v13 00/15] backup-top filter driver for backup

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2201 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]