On 10/31/21 09:13, Pádraig Brady wrote:
The attached uses statfs()->f_type which is usually available,
to avoid using SEEK_DATA on ZFS. This should be fairly lightweight I
think,
and only used for files that might be sparse.
Couldn't we be even lazier about invoking fstatfs? It needs to be
invoked only if the file appears to be sparse (as you mentioned), and
also only if lseek+SEEK_DATA fails (with errno==ENXIO) at a position
less than the file's length, and also at most once per input file
descriptor.
When copying multiple files (e.g., cp -r) we could also cache the
previous file's st_dev and ZFS status, so in the typical case we could
skip fstatfs entirely except for the first sparsish file that satisfies
the abovementioned criteria.
Also, as I understand it the problem occurs with OpenZFS but not on
Solaris or its descendants, so we don't need to do any of this if __sun
is defined.
Also, if we're going to use fstatfs it'd probably be better to hoist all
the fstatfs portability mess out of stat.c and into a new file (say,
gl/lib/pstatfs.c) that gives us a portable way of getting statfs-like
data out of filesystems, and using that new file in both stat.c and copy.c.
One more idea: I've seen reports that the problem goes away if you use
'read' to read just the first byte of the false hole; that might be a
simpler workaround than getting into the fstatfs portability mess.
We could also disable SEEK_DATA for remote file systems,
but that would be a pity since SEEK_DATA seems especially useful there.
Absolutely.
Don't know if you're following
<https://github.com/openzfs/zfs/issues/11900> but rincebrain reports a
working workaround on the OpenZFS side tho more testing needs to be
done. This OpenZFS bugs affects GNU tar and star (and surely other
programs) as well as coreutils, and we may be better off overall if we
merely ask people to fix their ZFS implementation rather than our trying
to work around the bug in coreutils.
