Re: [Qemu-devel] [PATCH v10 14/14] block/backup: use backup-top instead of write notifiers

[Max Reitz <mreitz@redhat.com>]

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

On 30.08.19 18:12, Vladimir Sementsov-Ogievskiy wrote:
> Drop write notifiers and use filter node instead.
> 
> = Changes =
> 
> 1. Add filter-node-name argument for backup qmp api. We have to do it
> in this commit, as 257 needs to be fixed.
> 
> 2. There are no more write notifiers here, so is_write_notifier
> parameter is dropped from block-copy paths.
> 
> 3. Intersecting requests handling changed, now synchronization between
> backup-top, backup and guest writes are all done in block/block-copy.c
> and works as follows:
> 
> On copy operation, we work only with dirty areas. If bits are dirty it
> means that there are no requests intersecting with this area. We clear
> dirty bits and take bdrv range lock (bdrv_co_try_lock) on this area to
> prevent further operations from interaction with guest (only with
> guest, as neither backup nor backup-top will touch non-dirty area). If
> copy-operation failed we set dirty bits back together with releasing
> the lock.
> 
> The actual difference with old scheme is that on guest writes we
> don't lock the whole region but only dirty-parts, and to be more
> precise: only dirty-part we are currently operate on. In old scheme
> guest write to non-dirty area (which may be safely ignored by backup)
> may wait for intersecting request, touching some other area which is
> dirty.
> 
> 4. To sync with in-flight requests at job finish we now have drained
> removing of the filter, we don't need rw-lock.
> 
> = Notes =
> 
> Note the consequence of three objects appearing: backup-top, backup job
> and block-copy-state:
> 
> 1. We want to insert backup-top before job creation, to behave similar
> with mirror and commit, where job is started upon filter.
> 
> 2. We also have to create block-copy-state after filter injection, as
> we don't want its source child be replaced by filter. Instead we want
> to keep BCS.source to be real source node, as we want to use
> bdrv_co_try_lock in CBW operations and it can't be used on filter, as
> on filter we already have in-flight (write) request from upper layer.
> 
> So, we firstly create inject backup-top, then create job and BCS. BCS
> is the latest just to not create extra variable for it. Finally we set
> bcs for backup-top filter.
> 
> = Iotest changes =
> 
> 56: op-blocker doesn't shoot now, as we set it on source, but then
> check on filter, when trying to start second backup.
> To keep the test we instead can catch another collision: both jobs will
> get 'drive0' job-id, as job-id parameter is unspecified. To prevent
> interleaving with file-posix locks (as they are dependent on config)
> let's use another target for second backup.
> 
> Also, it's obvious now that we'd like to drop this op-blocker at all
> and add a test-case for two backups from one node (to different
> destinations) actually works. But not in these series.
> 
> 257: The test wants to emulate guest write during backup. They should
> go to filter node, not to original source node, of course. Therefore we
> need to specify filter node name and use it.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  qapi/block-core.json       |   8 +-
>  include/block/block-copy.h |  10 +-
>  include/block/block_int.h  |   1 +
>  block/backup-top.c         |  14 +-
>  block/backup.c             | 113 +++-----------
>  block/block-copy.c         |  45 ++++--
>  block/replication.c        |   2 +-
>  blockdev.c                 |   1 +
>  tests/qemu-iotests/056     |   8 +-
>  tests/qemu-iotests/257     |   7 +-
>  tests/qemu-iotests/257.out | 306 ++++++++++++++++++-------------------
>  11 files changed, 238 insertions(+), 277 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


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