Re: [libvirt] [PATCH v2 3/3] virstoragefile: Refactor virStorageFileResize

On 08/21/2014 11:53 AM, Ján Tomko wrote: > On 08/11/2014 10:30 PM, John Ferlan wrote: >> Currently virStorageFileResize() function uses build conditionals to >> choose either the posix_fallocate() or mmap() with no fallback in order >> to preallocate the space in the newly resized file. >> >> This patch will modify the logic in order to allow fallbacks in the event >> that posix_fallocate() or the sys_fallocate syscall() doesn't work properly. >> The fallback will be to use the slow safewrite of zero filled buffers to >> the file. >> >> Use the virFileFdPosixFallocate() rather than a local function. >> >> Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; >> --- >> src/util/virstoragefile.c | 52 +++++++++++++++++++++++++++++++---------------- >> 1 file changed, 34 insertions(+), 18 deletions(-) >> >> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c >> index 5b6b2f5..4d37de1 100644 >> --- a/src/util/virstoragefile.c >> +++ b/src/util/virstoragefile.c >> @@ -1086,6 +1086,39 @@ virStorageFileChainGetBroken(virStorageSourcePtr chain, >> return 0; >> } >> >> +static int >> +resizeSysFallocate(const char *path, >> + int fd, >> + off_t offset, >> + off_t len) >> +{ >> + int rc = -1; >> +#if HAVE_SYS_SYSCALL_H && defined(SYS_fallocate) >> + if ((rc = syscall(SYS_fallocate, fd, 0, offset, len)) != 0) { >> + virReportSystemError(errno, >> + _("Failed to pre-allocate space for " >> + "file '%s'"), path); > > I think this should not log an error, since we have a fallback. > VIR_DEBUG maybe? > Yep - right. Although to match the other path, VIR_WARN... >> + } >> +#endif >> + return rc; >> +} >> + >> +static int >> +doResize(const char *path, >> + int fd, >> + off_t offset, >> + off_t len) >> +{ >> + if (virFileFdPosixFallocate(fd, offset, len) == 0 || >> + resizeSysFallocate(path, fd, offset, len) == 0 || >> + safezero(fd, offset, len) == 0) >> + return 0; >> + >> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", >> + _("preallocate is not supported on this platform")); >> + return -1; > > safezero is always supported. And this function should do: > return safezero(fd, offset, len); > > Hmm.. Perhaps a better way to do this would be to modify safezero() to add a 4th boolean parameter "resize" and make the "safezero_mmap()" be the false side and the check/call to SYS_fallocate() be the true side. That way all the logic resides in virfile.c