[Libguestfs] [PATCH 1/3] file: Avoid unsupported fallocate() calls

[Nir Soffer]

When using file systems not supporting ZERO_RANGE (e.g. NFS 4.2) or
block device on kernel < 4.9, we used to call fallocate() for every
zero, fail with EOPNOTSUPP, and fallback to manual zeroing.  When
trimming, we used to try unsupported fallocate() on every call.

Change file handle to remember if punching holes or zeroing range are
supported, and avoid unsupported calls.

- can_trim changed to report the actual capability once we tried to
  punch a hole. I don't think this is useful yet.

- zero changed to:
  1. If we can punch hole and may trim, try PUNCH_HOLE
  2. If we can zero range, try ZERO_RANGE
  3. Fall back to manual writing

- trim changed to:
  1. If we can punch hole, try PUNCH_HOLE
  2. Succeed
---
 plugins/file/file.c | 96 ++++++++++++++++++++++++++++-----------------
 1 file changed, 59 insertions(+), 37 deletions(-)

diff --git a/plugins/file/file.c b/plugins/file/file.c
index 3bb4d17..8bb9a2d 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -33,6 +33,7 @@
 
 #include <config.h>
 
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -118,6 +119,8 @@ file_config_complete (void)
 /* The per-connection handle. */
 struct handle {
   int fd;
+  bool can_punch_hole;
+  bool can_zero_range;
 };
 
 /* Create the per-connection handle. */
@@ -146,6 +149,18 @@ file_open (int readonly)
     return NULL;
   }
 
+#ifdef FALLOC_FL_PUNCH_HOLE
+  h->can_punch_hole = true;
+#else
+  h->can_punch_hole = false;
+#endif
+
+#ifdef FALLOC_FL_ZERO_RANGE
+  h->can_zero_range = true;
+#else
+  h->can_zero_range = false;
+#endif
+
   return h;
 }
 
@@ -189,19 +204,15 @@ file_get_size (void *handle)
   return statbuf.st_size;
 }
 
+/* Trim is advisory, but we prefer to advertise it only when we can actually
+ * (attempt to) punch holes. Before we tried to punch a hole, report true if
+ * FALLOC_FL_PUNCH_HOLE is defined before we did the first call. Once we tried
+ * to punch a hole, report the actual cappability of the underlying file. */
 static int
 file_can_trim (void *handle)
 {
-  /* Trim is advisory, but we prefer to advertise it only when we can
-   * actually (attempt to) punch holes.  Since not all filesystems
-   * support all fallocate modes, it would be nice if we had a way
-   * from fpathconf() to definitively learn what will work on a given
-   * fd for a more precise answer; oh well.  */
-#ifdef FALLOC_FL_PUNCH_HOLE
-  return 1;
-#else
-  return 0;
-#endif
+  struct handle *h = handle;
+  return h->can_punch_hole;
 }
 
 /* Read data from the file. */
@@ -252,34 +263,42 @@ file_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset)
 static int
 file_zero (void *handle, uint32_t count, uint64_t offset, int may_trim)
 {
-#if defined(FALLOC_FL_PUNCH_HOLE) || defined(FALLOC_FL_ZERO_RANGE)
   struct handle *h = handle;
-#endif
   int r = -1;
 
 #ifdef FALLOC_FL_PUNCH_HOLE
-  if (may_trim) {
+  if (h->can_punch_hole && may_trim) {
     r = do_fallocate (h->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
                       offset, count);
-    if (r == -1 && errno != EOPNOTSUPP) {
+    if (r == 0)
+      return r;
+
+    if (errno != EOPNOTSUPP) {
       nbdkit_error ("zero: %m");
+      return r;
     }
-    /* PUNCH_HOLE is older; if it is not supported, it is likely that
-       ZERO_RANGE will not work either, so fall back to write. */
-    return r;
+
+    h->can_punch_hole = false;
   }
 #endif
 
 #ifdef FALLOC_FL_ZERO_RANGE
-  r = do_fallocate (h->fd, FALLOC_FL_ZERO_RANGE, offset, count);
-  if (r == -1 && errno != EOPNOTSUPP) {
-    nbdkit_error ("zero: %m");
+  if (h->can_zero_range) {
+    r = do_fallocate (h->fd, FALLOC_FL_ZERO_RANGE, offset, count);
+    if (r== 0)
+      return r;
+
+    if (errno != EOPNOTSUPP) {
+      nbdkit_error ("zero: %m");
+      return r;
+    }
+
+    h->can_zero_range = false;
   }
-#else
-  /* Trigger a fall back to writing */
-  errno = EOPNOTSUPP;
 #endif
 
+  /* Trigger a fall back to writing */
+  errno = EOPNOTSUPP;
   return r;
 }
 
@@ -301,27 +320,30 @@ file_flush (void *handle)
 static int
 file_trim (void *handle, uint32_t count, uint64_t offset)
 {
-  int r = -1;
 #ifdef FALLOC_FL_PUNCH_HOLE
   struct handle *h = handle;
+  int r = -1;
+
+  if (h->can_punch_hole) {
+    r = do_fallocate (h->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
+                      offset, count);
+    if (r == -1) {
+      /* Trim is advisory; we don't care if it fails for anything other
+       * than EIO or EPERM. */
+      if (errno == EPERM || errno == EIO) {
+        nbdkit_error ("fallocate: %m");
+        return r;
+      }
+
+      if (errno == EOPNOTSUPP)
+        h->can_punch_hole = false;
 
-  /* Trim is advisory; we don't care if it fails for anything other
-   * than EIO or EPERM. */
-  r = do_fallocate (h->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
-                    offset, count);
-  if (r < 0) {
-    if (errno != EPERM && errno != EIO) {
       nbdkit_debug ("ignoring failed fallocate during trim: %m");
-      r = 0;
     }
-    else
-      nbdkit_error ("fallocate: %m");
   }
-#else
-  /* Based on .can_trim, this should not be reached. */
-  errno = EOPNOTSUPP;
 #endif
-  return r;
+
+  return 0;
 }
 
 static struct nbdkit_plugin plugin = {
-- 
2.17.1