Libguestfs - Feb 2020 - [nbdkit PATCH] split: Add support for .extents

Copies somewhat from the file plugin, with the difference that we
always provide an .extents even if one or more of the split files does
not support SEEK_HOLE.

Testing is possible on a file system that supports sparse files when
using nbdsh.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

I'm going ahead and committing this, but am also posting this for
review in case we later find any problems with what I just pushed.

 plugins/split/Makefile.am   |   6 +-
 plugins/split/split.c       | 125 +++++++++++++++++++++++++++++++++++-
 tests/Makefile.am           |   3 +
 tests/test-split-extents.sh |  74 +++++++++++++++++++++
 4 files changed, 206 insertions(+), 2 deletions(-)
 create mode 100755 tests/test-split-extents.sh

diff --git a/plugins/split/Makefile.am b/plugins/split/Makefile.am
index 003f6d9..ceff349 100644
--- a/plugins/split/Makefile.am
+++ b/plugins/split/Makefile.am
@@ -1,5 +1,5 @@
 # nbdkit
-# Copyright (C) 2017 Red Hat Inc.
+# Copyright (C) 2017-2020 Red Hat Inc.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
@@ -42,12 +42,16 @@ nbdkit_split_plugin_la_SOURCES = \

 nbdkit_split_plugin_la_CPPFLAGS = \
 	-I$(top_srcdir)/include \
+	-I$(top_srcdir)/common/utils \
 	$(NULL)
 nbdkit_split_plugin_la_CFLAGS = $(WARNINGS_CFLAGS)
 nbdkit_split_plugin_la_LDFLAGS = \
 	-module -avoid-version -shared \
 	-Wl,--version-script=$(top_srcdir)/plugins/plugins.syms \
 	$(NULL)
+nbdkit_split_plugin_la_LIBADD = \
+	$(top_builddir)/common/utils/libutils.la \
+	$(NULL)

 if HAVE_POD

diff --git a/plugins/split/split.c b/plugins/split/split.c
index 68682b2..70fd4d4 100644
--- a/plugins/split/split.c
+++ b/plugins/split/split.c
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2017-2019 Red Hat Inc.
+ * Copyright (C) 2017-2020 Red Hat Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -42,13 +42,19 @@
 #include <errno.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <stdbool.h>

 #include <nbdkit-plugin.h>

+#include "cleanup.h"
+
 /* The files. */
 static char **filenames = NULL;
 static size_t nr_files = 0;

+/* Any callbacks using lseek must be protected by this lock. */
+static pthread_mutex_t lseek_lock = PTHREAD_MUTEX_INITIALIZER;
+
 static void
 split_unload (void)
 {
@@ -96,6 +102,7 @@ struct handle {
 struct file {
   uint64_t offset, size;
   int fd;
+  bool can_extents;
 };

 /* Create the per-connection handle. */
@@ -107,6 +114,7 @@ split_open (int readonly)
   size_t i;
   uint64_t offset;
   struct stat statbuf;
+  off_t r;

   h = malloc (sizeof *h);
   if (h == NULL) {
@@ -151,6 +159,20 @@ split_open (int readonly)

     nbdkit_debug ("file[%zu]=%s: offset=%" PRIu64 ",
size=%" PRIu64,
                   i, filenames[i], h->files[i].offset, h->files[i].size);
+
+#ifdef SEEK_HOLE
+    /* Test if this file supports extents. */
+    ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lseek_lock);
+    r = lseek (h->files[i].fd, 0, SEEK_DATA);
+    if (r == -1 && errno != ENXIO) {
+      nbdkit_debug ("disabling extents: lseek on %s: %m",
filenames[i]);
+      h->files[i].can_extents = false;
+    }
+    else
+      h->files[i].can_extents = true;
+#else
+    h->files[i].can_extents = false;
+#endif
   }
   h->size = offset;
   nbdkit_debug ("total size=%" PRIu64, h->size);
@@ -319,6 +341,104 @@ split_cache (void *handle, uint32_t count, uint64_t
offset, uint32_t flags)
 }
 #endif /* HAVE_POSIX_FADVISE */

+#ifdef SEEK_HOLE
+static int64_t
+do_extents (struct file *file, uint32_t count, uint64_t offset,
+            bool req_one, struct nbdkit_extents *extents)
+{
+  int64_t r = 0;
+  uint64_t end = offset + count;
+
+  do {
+    off_t pos;
+
+    pos = lseek (file->fd, offset, SEEK_DATA);
+    if (pos == -1) {
+      if (errno == ENXIO) {
+        /* The current man page does not describe this situation well,
+         * but a proposed change to POSIX adds these words for ENXIO:
+         * "or the whence argument is SEEK_DATA and the offset falls
+         * within the final hole of the file."
+         */
+        pos = end;
+      }
+      else {
+        nbdkit_error ("lseek: SEEK_DATA: %" PRIu64 ": %m",
offset);
+        return -1;
+      }
+    }
+
+    /* We know there is a hole from offset to pos-1. */
+    if (pos > offset) {
+      if (nbdkit_add_extent (extents, offset + file->offset, pos - offset,
+                             NBDKIT_EXTENT_HOLE | NBDKIT_EXTENT_ZERO) == -1)
+        return -1;
+      r += pos - offset;
+      if (req_one)
+        break;
+    }
+
+    offset = pos;
+    if (offset >= end)
+      break;
+
+    pos = lseek (file->fd, offset, SEEK_HOLE);
+    if (pos == -1) {
+      nbdkit_error ("lseek: SEEK_HOLE: %" PRIu64 ": %m",
offset);
+      return -1;
+    }
+
+    /* We know there is data from offset to pos-1. */
+    if (pos > offset) {
+      if (nbdkit_add_extent (extents, offset + file->offset, pos - offset,
+                             0 /* allocated data */) == -1)
+        return -1;
+      r += pos - offset;
+      if (req_one)
+        break;
+    }
+
+    offset = pos;
+  } while (offset < end);
+
+  return r;
+}
+
+static int
+split_extents (void *handle, uint32_t count, uint64_t offset,
+               uint32_t flags, struct nbdkit_extents *extents)
+{
+  struct handle *h = handle;
+  const bool req_one = flags & NBDKIT_FLAG_REQ_ONE;
+
+  while (count > 0) {
+    struct file *file = get_file (h, offset);
+    uint64_t foffs = offset - file->offset;
+    uint64_t max;
+    int64_t r;
+
+    max = file->size - foffs;
+    if (max > count)
+      max = count;
+
+    if (file->can_extents) {
+      ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lseek_lock);
+      max = r = do_extents (file, max, foffs, req_one, extents);
+    }
+    else
+      r = nbdkit_add_extent (extents, offset, max, 0 /* allocated data */);
+    if (r == -1)
+      return -1;
+    count -= max;
+    offset += max;
+    if (req_one)
+      break;
+  }
+
+  return 0;
+}
+#endif /* SEEK_HOLE */
+
 static struct nbdkit_plugin plugin = {
   .name              = "split",
   .version           = PACKAGE_VERSION,
@@ -334,6 +454,9 @@ static struct nbdkit_plugin plugin = {
   .pwrite            = split_pwrite,
 #if HAVE_POSIX_FADVISE
   .cache             = split_cache,
+#endif
+#ifdef SEEK_HOLE
+  .extents           = split_extents,
 #endif
   /* In this plugin, errno is preserved properly along error return
    * paths from failed system calls.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 60583a0..ea6b147 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -193,6 +193,7 @@ EXTRA_DIST = \
 	test-sh-extents.sh \
 	test-single.sh \
 	test-single-from-file.sh \
+	test-split-extents.sh \
 	test-start.sh \
 	test-random-sock.sh \
 	test-tls.sh \
@@ -672,6 +673,8 @@ test_split_SOURCES = test-split.c
 test_split_CFLAGS = $(WARNINGS_CFLAGS) $(LIBNBD_CFLAGS)
 test_split_LDADD = $(LIBNBD_LIBS)

+TESTS += test-split-extents.sh
+
 # ssh plugin test.
 if HAVE_SSH
 # Uses ‘disk’ so we have to make it conditional on guestfish.
diff --git a/tests/test-split-extents.sh b/tests/test-split-extents.sh
new file mode 100755
index 0000000..da385a4
--- /dev/null
+++ b/tests/test-split-extents.sh
@@ -0,0 +1,74 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2018-2020 Red Hat Inc.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+# Concatenating sparse and data files should have observable extents.
+
+source ./functions.sh
+set -e
+set -x
+
+requires nbdsh --base-allocation -c 'exit(not h.supports_uri())'
+requires truncate --help
+requires stat --help
+
+files="test-split-extents.1 test-split-extents.2"
+rm -f $files
+cleanup_fn rm -f $files
+
+# Create two files, each half data and half sparse
+truncate --size=512k test-split-extents.1
+if test "$(stat -c %b test-split-extents.1)" != 0; then
+    echo "$0: unable to create sparse file, skipping this test"
+    exit 77
+fi
+printf %$((512*1024))d 1 >> test-split-extents.1
+printf %$((512*1024))d 1 >> test-split-extents.2
+truncate --size=1M test-split-extents.2
+
+# Test the split plugin
+nbdkit -v -U - split test-split-extents.1 test-split-extents.2 \
+       --run 'nbdsh --base-allocation --uri $uri -c "
+entries = []
+def f (metacontext, offset, e, err):
+    global entries
+    assert err.value == 0
+    assert metacontext == nbd.CONTEXT_BASE_ALLOCATION
+    entries = e
+h.block_status (2 * 1024 * 1024, 0, f)
+assert entries == [ 512 * 1024, 3,
+                    1024 * 1024, 0,
+                    512 * 1024, 3 ]
+entries = []
+# With req one, extents stop at file boundaries
+h.block_status (1024 * 1024, 768 * 1024, f, nbd.CMD_FLAG_REQ_ONE)
+assert entries == [ 256 * 1024, 0 ]
+       "'
-- 
2.24.1

Upstream already, and fine so ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/