[libvirt] [PATCH v2 0/3] Wrong allocation size displayed for NFS volumes

[John Ferlan]

Changes over v1 - different tact as more research discovers that the
posix_fallocate() does not perform the correct pre-allocation of space
for an NFS backed file/disk, some details of the findings can be found:

http://www.redhat.com/archives/libvir-list/2014-August/msg00367.html

The first patch in this series will refactor the safezero to allow for
more fallback than the current code. Initially implemented as a series
of three 'safezero()' functions in commit id 'c29d0929' using build
conditionals to determine which of the 3 is being used.

The refactored code will have one function that will make a series of
calls rather the just failing on whatever is built into the system.
The first is a global virFileFdPosixFallocate() with the build conditional 
controlling only whether the function runs or will just return -1.  It
will also be re-used by the Volume Resize code in a future patch.

If on creation the virFileFdPosixFallocate() fails, attempts will be made
to then try the mmap() method (if it exists), and then fall back to the
slow, old, safewrite of zero filled buffers. The mmap function will follow
the format of the posix_fallocate insomuch as if HAVE_MMAP is defined,
then that will be attempted or a -1 will be returned immediately.

The major change of this patch over the prior code is the fallback. One
other minor change is if the virFileFdPosixFallocate() tries to call
posix_fallocate() and fails, then a VIR_WARN will be used to indicate
something went wrong. Previously, the code would return errno and eventually
that would filter back to a caller which would use the errno in a sys
error message. This I think only is odd when it comes to the resize path.

The second patch looks to resolve the first issue discovered by:

https://bugzilla.redhat.com/show_bug.cgi?id=1077068

It does this by checking the returned size/len of the file in the
posix_fallocate() call and if it does not match a VIR_WARN will be
posted and a failure returned allowing either the mmap or write of
zero filled buffers to the file.

The third patch changes the virStorageFileResize() logic to follow
the safezero() logic with respect to build conditionals. Also, rather
than reinvent the wheel, it will reuse the virFileFdPosixFallocate().
The new static API resizeSysFallocate() will return -1 if the build
conditionals are not met.

A current "downside" is by calling virFileFdPosixFallocate() twice
(once directly and the other indirectly through safezero), the
VIR_WARN will be displayed twice, but that should only affect someone
trying to debug things.


NOTE:
Investigation and testing while creating this series also showed it's
possible to modify the NFS Pool Start code to add a "-o wsize=4096" to
the MOUNT command in order to force smaller write's.  While that did fix
the symptoms, the fix just didn't "feel right". The block size of the
"source" export was 4096 bytes, while the block size of the "target"
file was 1048576 bytes (current NFSv4 wsize default value when not
specified). Whether those play into what posix_fallocate() does I
am not sure, but if it does and that's fixed, then the way these
patches are coded up - it won't matter. Compared to perhaps needing
to revisit or document heavily how to set some new field in the pool
XML source - this just seems more right.

John Ferlan (3):
  virfile: Refactor safezero, introduce virFileFdPosixFallocate
  virfile: Check returned size from virFileFdPosixFallocate
  virstoragefile: Refactor virStorageFileResize

 src/libvirt_private.syms  |  1 +
 src/util/virfile.c        | 58 ++++++++++++++++++++++++++++++++++++++---------
 src/util/virfile.h        |  2 ++
 src/util/virstoragefile.c | 52 +++++++++++++++++++++++++++---------------
 4 files changed, 84 insertions(+), 29 deletions(-)

-- 
1.9.3