<...snip...>
Why all this happens I'm not sure. Bug in posix_fallocate()? Bug in configuration? I have to assume that when this code was first added NFS probably was still using smaller block sizes.
The code was introduced in 2013. Maybe it wasn't tested on NFS at all? It looks like a posix_fallocate bug to me.
Technically it was before that - the safezero function and it's supporting cast were moved into virfile.c in 2013; however, they were in [vir]util.c before that. It was first introduced in 2009 in commit id 'c29d0929' and beyond some minor changes/reword has stayed relatively unchanged. The one downside I see with it is that it's a build related either/or setup. That is the "HAVE_POSIX_FALLOCATE" dictates the usage and provides no fallback in the event that perhaps the result isn't expected or if for some reason posix_fallocate fails. "Theoretically" the safewrite option should/could be the fallback in the event it's determined that the posix_fallocate() didn't generate the right allocation size.
The other method we have (syscall(SYS_fallocate)) gives me EOPNOTSUPP - Operation not supported on transport endpoint.
Right - I saw that too.... The resize code followed the safezero() code at least with respect to the either/or with the HAVE_POSIX_FALLOCATE build conditional. It was introduced in commit id 'aa2a4cff'. It assumes that the "SYS_fallocate" syscall is present "and" will work. It is missing an option to safewrite() from current allocation to the new end of the file (eg, a call to safezero passing the the current offset).
Whether anyone has noticed or not beyond the virt-test which discovered the issue - I'm not sure. In any case, does anyone have feedback/thoughts for next steps? I can put together something that avoids posix_fallocate() for the create-as and resize paths.
I think reporting an error when preallocate is requested for a NFS pool makes sense, but that might be pretty annoying if something supplies the preallocate flag by default.
My comment was more geared towards my findings of the posix_fallocate() introduction in 2009. While I agree there's something flaky with posix_fallocate, I don't think an error is necessarily the best path. Maybe better "fall back code" - we could check the posix_fallocate() results (using stat) perhaps VIR_WARN that something is wrong before falling back to the mmap/safewrite options. John