[Libguestfs] [nbdkit PATCH v2] Introduce cacheextents filter

[Martin Kletzander]

On Thu, May 16, 2019 at 08:42:43AM +0100, Richard W.M. Jones wrote:
>
>Apply the attached patch on top of yours which contains miscellaneous
>fixes.  If you use ‘./configure --enable-gcc-warnings’ then it will
>enable GCC warnings.
>

Thanks.  Again, too much used to libvirt codebase where those are being enabled
by default for builds from git.

>The test doesn't really test anything.  If extents are being cached
>then I have two ideas about how you could see that: Either (1) you
>could look at debug messages, the second extents call shouldn't be
>processed at the plugin layer.  Or (2) write a small nbdkit-sh-plugin
>script which should see the first extent request and not the second
>one (and even test that a subsequent write should kill the cache so
>another extent request is seen by the plugin).
>

Good point with the write.  The test checks that there is only one request to
the plugin for extents before the log filter is applied after the cacheextents
filter.  If I remove the cacheextents one, the log will contain multiple
requests for extents for which the grep will return 4 and the test will fail.

Anyway for the write the sh plugin will help, so I'll convert it to that.

>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/

>diff --git a/filters/cacheextents/cacheextents.c b/filters/cacheextents/cacheextents.c
>index 01c0328..6cd22a7 100644
>--- a/filters/cacheextents/cacheextents.c
>+++ b/filters/cacheextents/cacheextents.c
>@@ -50,9 +50,6 @@
> /* This lock protects the global state. */
> static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
>
>-/* The real size of the underlying plugin. */
>-static uint64_t size;
>-
> /* Cached extents from the last extents () call and its start and end for the
>    sake of simplicity. */
> struct nbdkit_extents *cache_extents;
>@@ -131,9 +128,8 @@ cacheextents_extents (struct nbdkit_next_ops *next_ops, void *nxdata,
>   }
>
>   nbdkit_debug ("cacheextents: cache miss");
>-  int r = next_ops->extents (nxdata, count, offset, flags, extents, err);
>-  if (r == -1)
>-    return r;
>+  if (next_ops->extents (nxdata, count, offset, flags, extents, err) == -1)
>+    return -1;
>
>   return cacheextents_fill (extents, err);
> }
>diff --git a/tests/Makefile.am b/tests/Makefile.am
>index a2bbaea..aaf7450 100644
>--- a/tests/Makefile.am
>+++ b/tests/Makefile.am
>@@ -366,9 +366,7 @@ test_oldstyle_LDADD = libtest.la $(LIBGUESTFS_LIBS)
> endif HAVE_LIBGUESTFS
>
> # Test export flags.
>-TESTS += \
>-	test-cacheextents.sh \
>-	test-eflags.sh
>+TESTS += test-eflags.sh
>
> # common disk image shared with several tests
> if HAVE_GUESTFISH
>@@ -868,6 +866,9 @@ TESTS += \
> endif HAVE_GUESTFISH
> TESTS += test-cache-max-size.sh
>
>+# cacheextents filter test.
>+TESTS += test-cacheextents.sh
>+
> # cow filter test.
> if HAVE_GUESTFISH
> TESTS += test-cow.sh
>diff --git a/tests/test-cacheextents.sh b/tests/test-cacheextents.sh
>index fd0bd1c..b9145ed 100755
>--- a/tests/test-cacheextents.sh
>+++ b/tests/test-cacheextents.sh
>@@ -38,6 +38,10 @@ requires diff --version
> requires grep --version
> requires qemu-img --version
>
>+files="test-cacheextents-actual.log test-cacheextents-expected.log"
>+rm -f $files
>+cleanup_fn rm -f $files
>+
> nbdkit \
>     -U - \
>     --filter=cacheextents \

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190516/2a302ded/attachment.sig>