Re: [libvirt] [PATCH] storage: fix file allocation behavior in file cloning

2013-10-04 Thread Michal Privoznik
On 30.09.2013 18:57, Oskari Saarenmaa wrote:
 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 o...@ohmu.fi
 ---
  the safezero mmap issue fixed in my previous patch never showed up because
  all safezero calls previously had 0 offset (or maybe everyone's using
  posix_fallocate)
 
  configure.ac  |  8 
  src/storage/storage_backend.c | 35 ++-
  2 files changed, 34 insertions(+), 9 deletions(-)

ACKed and pushed.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] storage: fix file allocation behavior in file cloning

2013-09-30 Thread Oskari Saarenmaa
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 o...@ohmu.fi
---
 the safezero mmap issue fixed in my previous patch never showed up because
 all safezero calls previously had 0 offset (or maybe everyone's using
 posix_fallocate)

 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 553015a..220d95b 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 b7edf85..5f1bc66 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);
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list