ping? Tks - John On 08/11/2014 04:30 PM, John Ferlan wrote: > 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(-) > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list