[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v13 00/15] backup-top filter driver for backup
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH v13 00/15] backup-top filter driver for backup |
Date: |
Wed, 25 Sep 2019 19:28:06 +0000 |
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.
>
> On 20.09.2019 17:20, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> These series introduce backup-top driver. It's a filter-node, which
>> do copy-before-write operation. Mirror uses filter-node for handling
>> guest writes, let's move to filter-node (from write-notifiers) for
>> backup too.
>>
>> v11,v12 -> v13 changes:
>>
>> [v12 was two fixes in separate: [PATCH v12 0/2] backup: copy_range fixes]
>>
>> 01: new in v12, in v13 change comment
>> 02: in v12: add "Fixes: " to commit msg, in v13 add John's r-b
>> 05: rebase on 01
>> 07: rebase on 01. It still a clean movement, keep r-b
>>
>> Vladimir Sementsov-Ogievskiy (15):
>> block/backup: fix max_transfer handling for copy_range
>> block/backup: fix backup_cow_with_offload for last cluster
>> block/backup: split shareable copying part from backup_do_cow
>> block/backup: improve comment about image fleecing
>> block/backup: introduce BlockCopyState
>> block/backup: fix block-comment style
>> block: move block_copy from block/backup.c to separate file
>> block: teach bdrv_debug_breakpoint skip filters with backing
>> iotests: prepare 124 and 257 bitmap querying for backup-top filter
>> iotests: 257: drop unused Drive.device field
>> iotests: 257: drop device_add
>> block/io: refactor wait_serialising_requests
>> block: add lock/unlock range functions
>> block: introduce backup-top filter driver
>> block/backup: use backup-top instead of write notifiers
>>
>> qapi/block-core.json | 8 +-
>> block/backup-top.h | 37 ++
>> include/block/block-copy.h | 84 ++++
>> include/block/block_int.h | 5 +
>> block.c | 34 +-
>> block/backup-top.c | 240 ++++++++++++
>> block/backup.c | 440 ++++-----------------
>> block/block-copy.c | 346 ++++++++++++++++
>> block/io.c | 68 +++-
>> block/replication.c | 2 +-
>> blockdev.c | 1 +
>> block/Makefile.objs | 3 +
>> block/trace-events | 14 +-
>> tests/qemu-iotests/056 | 8 +-
>> tests/qemu-iotests/124 | 83 ++--
>> tests/qemu-iotests/257 | 91 ++---
>> tests/qemu-iotests/257.out | 714 ++++++++++++++--------------------
>> tests/qemu-iotests/iotests.py | 27 ++
>> 18 files changed, 1287 insertions(+), 918 deletions(-)
>> create mode 100644 block/backup-top.h
>> create mode 100644 include/block/block-copy.h
>> create mode 100644 block/backup-top.c
>> create mode 100644 block/block-copy.c
>>
- [PATCH v13 12/15] block/io: refactor wait_serialising_requests, (continued)
- [PATCH v13 12/15] block/io: refactor wait_serialising_requests, Vladimir Sementsov-Ogievskiy, 2019/09/20
- [PATCH v13 11/15] iotests: 257: drop device_add, Vladimir Sementsov-Ogievskiy, 2019/09/20
- [PATCH v13 10/15] iotests: 257: drop unused Drive.device field, Vladimir Sementsov-Ogievskiy, 2019/09/20
- [PATCH v13 09/15] iotests: prepare 124 and 257 bitmap querying for backup-top filter, Vladimir Sementsov-Ogievskiy, 2019/09/20
- [PATCH v13 15/15] block/backup: use backup-top instead of write notifiers, Vladimir Sementsov-Ogievskiy, 2019/09/20
- Re: [PATCH v13 00/15] backup-top filter driver for backup, Max Reitz, 2019/09/20
- [PATCH 16/15 v13] block/block-copy: fix block_copy, Vladimir Sementsov-Ogievskiy, 2019/09/25
- Re: [PATCH v13 00/15] backup-top filter driver for backup, Vladimir Sementsov-Ogievskiy, 2019/09/25
- Re: [PATCH v13 00/15] backup-top filter driver for backup,
Vladimir Sementsov-Ogievskiy <=