On Thu, Apr 24, 2025 at 12:42:45AM +0800, Sunny Zhu wrote:
> on Thu, 17 Apr 2025 13:39:13 -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.
> >
> > @@ -452,12 +474,21 @@ static unsigned mirror_perform(MirrorBlockJob *s,
> > int64_t offset,
> >
> > switch (mirror_method) {
> > case MIRROR_METHOD_COPY:
> > + if (s->zero_bitmap) {
> > + bitmap_clear(s->zero_bitmap, offset / s->granularity,
> > + DIV_ROUND_UP(bytes, s->granularity));
> > + }
> > co = qemu_coroutine_create(mirror_co_read, op);
> > break;
> > case MIRROR_METHOD_ZERO:
> > + /* s->zero_bitmap handled in mirror_co_zero */
> > co = qemu_coroutine_create(mirror_co_zero, op);
> > break;
> > case MIRROR_METHOD_DISCARD:
> > + if (s->zero_bitmap) {
> > + bitmap_clear(s->zero_bitmap, offset / s->granularity,
> > + DIV_ROUND_UP(bytes, s->granularity));
> > + }
> > co = qemu_coroutine_create(mirror_co_discard, op);
> > break;
> > default:
> >
>
> If we have performed the skip-zero operation, it should not be constrained
> by mirror job bandwidth limits. Therefore, it is preferable to exclude it
> from rate limiting.
Indeed, that makes sense. And it may impact the iotests: test 194
should have a smaller amount of bytes transferred, due to skipping
zeroes, so I may need to hoist the filtering that I added in the later
patch for iotest mirror-sparse into common code.
>
> bool skip_write_zero = false;
>
> io_bytes = mirror_perform(s, offset, io_bytes, mirror_method,
> &skip_write_zero);
> if (skip_write_zero || (mirror_method != MIRROR_METHOD_COPY &&
> write_zeroes_ok)) {
> io_bytes_acct = 0;
> } ..
>
Thanks; that's helpful. I'll incorporate it into v3.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
