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

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

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.

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.

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
>