This series is based on v2 of my "block: Deal with filters" series:
Based-on: <20180809223117.7846-1-mre...@redhat.com>
The bulk of this series (14 of the patches here, in fact) makes more of
the coroutined mirror I started with my active mirror series. For this,
mirror_perform() is translated into a coroutine version that actually
looks like coroutines are a nice thing to have (a single AioContext
lock, a common clean-up path, and a mirror_co_copy() that does both read
and write in a single function (like it is supposed to)).
Another thing I have wanted to implement is a read-write-blocking mode.
write-blocking (active mirror) copies data to the target whenever you
write data to the mirror node -- so once the mirror bitmap is clean, it
will stay clean, and completing the job is guaranteed to be basically
instantenous. read-write-blocking would also copy data whenever you
read from the mirror node, because we have already read the data right
now, so we might as well copy it to the target now.
Instead of implementing it actually as a new copy-mode (which is how I
used to do, but I decided that was boring), I opted for implementing an
interesting .bdrv_co_block_status() function that allows using COR for
the purpose: You simply put a COR node on top of your write-blocking
mirror node, and then all the data read will be copied to the target
through COR (as long as it is not already cleanly mirrored).
The benefit I see with this approach is that is just looks nice. Why
implement an actual new copy-mode when we can just an existing block
layer function for the purpose?
The drawbacks I see are these:
(1) mirror in write-blocking mode has to interpret
BDRV_REQ_WRITE_UNCHANGED in a special manner, or this doesn't really
work. With patch 16 of this series, such requests are not written
to the source, but only to the target. You might find that weird.
(2) We currently don't have a good way of inserting a COR node at
runtime...
(3) You may break your data if you do funny stuff. Now this is of
course the interesting point.
Mirror works (by design) even when you attach a parent to the source
directly, i.e., not going through the mirror node. That is because
mirror uses the source's dirty bitmap functionality and does not
build its own. But if you do this with COR/write-blocking, then the
COR driver may issue a request while some other parent is issuing
another request immediately to the source, which may cause source
and target to go out of sync.
But... Technically this is already an issue with write-blocking
alone. We reset the bitmap only in do_sync_target_write(), after we
have already written to the source. So if another of source's
parents writes to source between our write and the bitmap reset, we
lose that information.
So I suppose when using write-blocking, the mirror driver may not
share the WRITE permission?
(4) The same issue exists with COR itself, actually... It can only
share the WRITE permission because COR requests are serializing, but
that only works on a single layer. With a filter between COR and
the allocating node, you get the same issue of someone being able to
slip in a request between the COR read and the COR allocation.
Maybe we actually want COR not to share WRITE either?
So in my opinion the main drawback is that this design reveals current
weaknesses and bugs of the block layer.
Max Reitz (17):
iotests: Try reading while mirroring in 156
mirror: Make wait_for_any_operation() coroutine_fn
mirror: Pull *_align_for_copy() from *_co_read()
mirror: Remove bytes_handled, part 1
mirror: Remove bytes_handled, part 2
mirror: Create mirror_co_perform()
mirror: Make mirror_co_zero() nicer
mirror: Make mirror_co_discard() nicer
mirror: Lock AioContext in mirror_co_perform()
mirror: Create mirror_co_alloc_qiov()
mirror: Inline mirror_write_complete(), part 1
mirror: Put QIOV locally into mirror_co_read
mirror: Linearize mirror_co_read()
mirror: Inline mirror_iteration_done()
mirror: Release AioCtx before queue_restart_all()
mirror: Support COR with write-blocking
iotests: Add test for active mirror with COR
block/mirror.c | 483 ++++++++++++++++++++-----------------
tests/qemu-iotests/151 | 56 +++++
tests/qemu-iotests/151.out | 4 +-
tests/qemu-iotests/156 | 7 +
tests/qemu-iotests/156.out | 3 +
5 files changed, 334 insertions(+), 219 deletions(-)
--
2.17.1
