From 532fef369fffdad6b158a720e248ed21cef20478 Mon Sep 17 00:00:00 2001 From: Oskari Saarenmaa Date: Mon, 30 Sep 2013 19:57:35 +0300 Subject: [PATCH] storage: fix file allocation behavior in file cloning Fixed the safezero call for allocating the rest of the file after cloning an existing volume; it used to always use a zero offset, causing it to only allocate the beginning of the file. Also modified file creation to try to use fallocate(2) to pre-allocate disk space before copying any data to make sure it fails early on if disk is full and makes sure we can skip zero blocks when copying file contents. If fallocate isn't available we will zero out the rest of the file after cloning and only use sparse cloning if client requested a lower allocation than the input volume's capacity. Signed-off-by: Oskari Saarenmaa --- configure.ac | 8 ++++---- src/storage/storage_backend.c | 35 ++++++++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index f168f894ce..a867933866 100644 --- a/configure.ac +++ b/configure.ac @@ -253,10 +253,10 @@ AC_CHECK_SIZEOF([long]) dnl Availability of various common functions (non-fatal if missing), dnl and various less common threadsafe functions -AC_CHECK_FUNCS_ONCE([cfmakeraw geteuid getgid getgrnam_r getmntent_r \ - getpwuid_r getuid kill mmap newlocale posix_fallocate posix_memalign \ - prlimit regexec sched_getaffinity setgroups setns setrlimit symlink \ - sysctlbyname]) +AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid getgrnam_r \ + getmntent_r getpwuid_r getuid kill mmap newlocale posix_fallocate \ + posix_memalign prlimit regexec sched_getaffinity setgroups setns \ + setrlimit symlink sysctlbyname]) dnl Availability of pthread functions (if missing, win32 threading is dnl assumed). Because of $LIB_PTHREAD, we cannot use AC_CHECK_FUNCS_ONCE. diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index b7edf85950..5f1bc66971 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -129,7 +129,7 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, int fd, unsigned long long *total, - int is_dest_file) + int want_sparse) { int inputfd = -1; int amtread = -1; @@ -191,7 +191,7 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, interval = ((wbytes > amtleft) ? amtleft : wbytes); int offset = amtread - amtleft; - if (is_dest_file && memcmp(buf+offset, zerobuf, interval) == 0) { + if (want_sparse && memcmp(buf+offset, zerobuf, interval) == 0) { if (lseek(fd, interval, SEEK_CUR) < 0) { ret = -errno; virReportSystemError(errno, @@ -315,6 +315,7 @@ static int createRawFile(int fd, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol) { + int need_alloc = 1; int ret = 0; unsigned long long remain; @@ -328,17 +329,41 @@ createRawFile(int fd, virStorageVolDefPtr vol, goto cleanup; } +#ifdef HAVE_FALLOCATE + /* Try to preallocate all requested disk space, but fall back to + * other methods if this fails with ENOSYS or EOPNOTSUPP. + * NOTE: do not use posix_fallocate; posix_fallocate falls back + * to writing zeroes block by block in case fallocate isn't + * available, and since we're going to copy data from another + * file it doesn't make sense to write the file twice. */ + if (fallocate(fd, 0, 0, vol->allocation) == 0) { + need_alloc = 0; + } else if (errno != ENOSYS && errno != EOPNOTSUPP) { + ret = -errno; + virReportSystemError(errno, + _("cannot allocate %llu bytes in file '%s'"), + vol->allocation, vol->target.path); + goto cleanup; + } +#endif + remain = vol->allocation; if (inputvol) { - ret = virStorageBackendCopyToFD(vol, inputvol, fd, &remain, 1); + /* allow zero blocks to be skipped if we've requested sparse + * allocation (allocation < capacity) or we have already + * been able to allocate the required space. */ + int want_sparse = (need_alloc == 0) || + (vol->allocation < inputvol->capacity); + + ret = virStorageBackendCopyToFD(vol, inputvol, fd, &remain, want_sparse); if (ret < 0) { goto cleanup; } } - if (remain) { - if (safezero(fd, 0, remain) < 0) { + if (remain && need_alloc) { + if (safezero(fd, vol->allocation - remain, remain) < 0) { ret = -errno; virReportSystemError(errno, _("cannot fill file '%s'"), vol->target.path);