On Thu, Apr 17, 2025 at 01:39:13PM -0500, Eric Blake wrote: > When mirroring, the goal is to ensure that the destination reads the > same as the source; this goal is met whether the destination is sparse > or fully-allocated. However, if the destination cannot efficiently > write zeroes, then any time the mirror operation wants to copy zeroes > from the source to the destination (either during the background over > sparse regions when doing a full mirror, or in the foreground when the > guest actively writes zeroes), we were causing the destination to > fully allocate that portion of the disk, even if it already read as > zeroes. > > The effect is especially pronounced when the source is a raw file. > That's because when the source is a qcow2 file, the dirty bitmap only > visits the portions of the source that are allocated, which tend to be > non-zero. But when the source is a raw file, > bdrv_co_is_allocated_above() reports the entire file as allocated so > mirror_dirty_init sets the entire dirty bitmap, and it is only later > during mirror_iteration that we change to consulting the more precise > bdrv_co_block_status_above() to learn where the source reads as zero. > > Remember that since a mirror operation can write a cluster more than > once (every time the guest changes the source, the destination is also > changed to keep up), we can't take the shortcut of relying on > s->zero_target (which is static for the life of the job) in > mirror_co_zero() to see if the destination is already zero, because > that information may be stale. Any solution we use must be dynamic in > the face of the guest writing or discarding a cluster while the mirror > has been ongoing. > > We could just teach mirror_co_zero() to do a block_status() probe of > the destination, and skip the zeroes if the destination already reads > as zero, but we know from past experience that extra block_status() > calls are not always cheap (tmpfs, anyone?), especially when they are > random access rather than linear. Use of block_status() of the source > by the background task in a linear fashion is not our bottleneck (it's > a background task, after all); but since mirroring can be done while > the source is actively being changed, we don't want a slow > block_status() of the destination to occur on the hot path of the > guest trying to do random-access writes to the source. > > So this patch takes a slightly different approach: any time we have to > transfer the full image, we know that mirror_dirty_init() is _already_ > doing a pre-zero pass over the entire destination. Therefore, if we > track which clusters of the destination are zero at any given moment, > we don't have to do a block_status() call on the destination, but can > instead just refer to the zero bitmap associated with the job. > > With this patch, if I create a raw sparse destination file, connect it > with QMP 'blockdev-add' while leaving it at the default "discard": > "ignore", then run QMP 'blockdev-mirror' with "sync": "full", the > destination remains sparse rather than fully allocated. > > Signed-off-by: Eric Blake <ebl...@redhat.com> > --- > block/mirror.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 65 insertions(+), 5 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
signature.asc
Description: PGP signature
