On Fri, 9 May 2025 15:40:25 -0500, Eric Blake wrote: > QEMU has an optimization for a just-created drive-mirror destination > that is not possible for blockdev-mirror (which can't create the > destination) - any time we know the destination starts life as all > zeroes, we can skip a pre-zeroing pass on the destination. Recent > patches have added an improved heuristic for detecting if a file > contains all zeroes, and we plan to use that heuristic in upcoming > patches. But since a heuristic cannot quickly detect all scenarios, > and there may be cases where the caller is aware of information that > QEMU cannot learn quickly, it makes sense to have a way to tell QEMU > to assume facts about the destination that can make the mirror > operation faster. Given our existing example of "qemu-img convert > --target-is-zero", it is time to expose this override in QMP for > blockdev-mirror as well. > > This patch results in some slight redundancy between the older > s->zero_target (set any time mode==FULL and the destination image was > not just created - ie. clear if drive-mirror is asking to skip the > pre-zero pass) and the newly-introduced s->target_is_zero (in addition > to the QMP override, it is set when drive-mirror creates the > destination image); this will be cleaned up in the next patch. > > There is also a subtlety that we must consider. When drive-mirror is > passing target_is_zero on behalf of a just-created image, we know the > image is sparse (skipping the pre-zeroing keeps it that way), so it > doesn't matter whether the destination also has "discard":"unmap" and > "detect-zeroes":"unmap". But now that we are letting the user set the > knob for target-is-zero, if the user passes a pre-existing file that > is fully allocated, it is fine to leave the file fully allocated under > "detect-zeroes":"on", but if the file is open with > "detect-zeroes":"unmap", we should really be trying harder to punch > holes in the destination for every region of zeroes copied from the > source. The easiest way to do this is to still run the pre-zeroing > pass (turning the entire destination file sparse before populating > just the allocated portions of the source), even though that currently > results in double I/O to the portions of the file that are allocated. > A later patch will add further optimizations to reduce redundant > zeroing I/O during the mirror operation. > > Since "target-is-zero":true is designed for optimizations, it is okay > to silently ignore the parameter rather than erroring if the user ever > sets the parameter in a scenario where the mirror job can't exploit it > (for example, when doing "sync":"top" instead of "sync":"full", we > can't pre-zero, so setting the parameter won't make a speed > difference). > > Signed-off-by: Eric Blake <ebl...@redhat.com> > Acked-by: Markus Armbruster <arm...@redhat.com> > > --- > > v4: hoist earlier in series. QMP design is unchanged, but logic in > mirror_dirty_init is different (in many aspects simpler, but now > catering to "detect-zeroes":"unmap") so Acked-by on QMP kept, but > Reviewed-by dropped. > --- > qapi/block-core.json | 8 +++++++- > include/block/block_int-global-state.h | 3 ++- > block/mirror.c | 27 ++++++++++++++++++++++---- > blockdev.c | 18 ++++++++++------- > tests/unit/test-block-iothread.c | 2 +- > 5 files changed, 44 insertions(+), 14 deletions(-)
Reviewed-by: Sunny Zhu <sunnyz...@qq.com>
