On Thu, Apr 17, 2025 at 04:17:55PM -0400, Stefan Hajnoczi wrote:
> On Thu, Apr 17, 2025 at 01:39:06PM -0500, Eric Blake wrote:
> > This patch is purely mechanical, changing bool want_zero into a new
> > enum BlockStatusMode. As of this patch, all implementations are
> > unchanged (the old want_zero==true is now mode==BDRV_BSTAT_PRECISE),
> > but the callers in io.c are set up so that future patches will be able
> > to differente between whether the caller cares more about allocation
>
> differentiate
>
> > or about reads-as-zero, for driver implementations that will actually
> > want to behave differently for those more-specific hints.
> >
> > As for the background why this patch is useful: right now, the
> > file-posix driver recognizes that if allocation is being queried, the
> > entire image can be reported as allocated (there is no backing file to
> > refer to) - but this throws away information on whether the entire
> > image reads as zero (trivially true if lseek(SEEK_HOLE) at offset 0
> > returns -ENXIO, a bit more complicated to prove if the raw file was
> > created with 'qemu-img create' since we intentionally allocate a small
> > chunk of all-zero data to help with alignment probing). The next
> > patches will add a generic algorithm for seeing if an entire file
> > reads as zeroes, as well as tweak the file-posix driver to react to
> > the new hints.
> >
> > +/* Modes for block status calls */
> > +enum BlockStatusMode {
> > + /*
> > + * Status should be as accurate as possible: _OFFSET_VALID
> > + * and_OFFSET_ZERO should each be set where efficiently possible,
>
> "and _OFFSET_ZERO"
>
> > + * extents may be smaller, and iteration through the entire block
> > + * device may take more calls.
> > + */
> > + BDRV_BSTAT_PRECISE,
> > +
> > + /*
> > + * The caller is primarily concerned about overall allocation:
> > + * favor larger *pnum, perhaps by coalescing extents and reporting
> > + * _DATA instead of _ZERO, and without needing to read data or
> > + * bothering with _OFFSET_VALID.
> > + */
> > + BDRV_BSTAT_ALLOCATED,
> > +
> > + /*
> > + * The caller is primarily concerned about whether the device
> > + * reads as zero: favor a result of _ZERO, even if it requires
> > + * reading a few sectors to verify, without needing _OFFSET_VALID.
> > + */
> > + BDRV_BSTAT_ZERO,
> > +};
>
> I have trouble understanding what the exact semantics are of these modes
> are. Would it be possible to pass flags to block status calls that can
> be ORed together instead: WANT_OFFSET_VALID, WANT_ZERO, etc? The flags
> would be orthogonal and easier to understand than modes that seem to
> combine multiple flag behaviors.
I can give that a try. If I'm understanding the request correctly, I
would map it as follows:
BDRV_BSTAT_PRECISE => WANT_ZERO | WANT_OFFSET_VALID | WANT_ALLOCATED
BDRV_BSTAT_ALLOCATED => WANT_ALLOCATED
BDRV_BSTAT_ZERO => WANT_ZERO
while still trying to keep it a mechanical conversion in this patch.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
