[Libguestfs] [PATCH nbdkit] New filter: gzip

[Richard W.M. Jones]

On Fri, Jul 10, 2020 at 09:20:29AM -0500, Eric Blake wrote:
> >+++ b/filters/tar/nbdkit-tar-filter.pod
> >@@ -42,11 +42,13 @@ server use:
> >   nbdkit -r curl https://example.com/file.tar \
> >          --filter=tar tar-entry=disk.img
> >-=head2 Open an xz-compressed tar file (read-only)
> >+=head2 Open an gzip-compressed tar file (read-only)
> 
> Should this heading s/gzip-// ?

Yes that's a mistake - fixed.

> >+++ b/tests/Makefile.am
> >@@ -557,23 +557,6 @@ EXTRA_DIST += test-floppy.sh
> >  TESTS += test-full.sh
> >  EXTRA_DIST += test-full.sh
> >-# gzip plugin test.
> >-if HAVE_MKE2FS_WITH_D
> >-if HAVE_ZLIB
> >-LIBGUESTFS_TESTS += test-gzip
> >-check_DATA += disk.gz
> >-CLEANFILES += disk.gz
> 
> Is it worth keeping this around until we actually retire the plugin?

OK I'll extend the test so it tests both.

> >+  /* Get the size of the underlying plugin. */
> >+  compressed_size = next_ops->get_size (nxdata);
> 
> If we wanted, we could save this...
> 
> 
> >+/* Similar to above, whatever the plugin says, extents are not
> >+ * supported.
> >+ */
> >+static int
> >+gzip_can_extents (struct nbdkit_next_ops *next_ops, void *nxdata,
> >+                  void *handle)
> 
> Well, since we stored it as a temp file, we could actually report
> extents off of lseek(SEEK_HOLE).  But that can be added separately,
> if we want it (it goes back to your notion of a library for
> file-based accessor functions).

Sounds tricky :-)

> >+  /* We must call underlying get_size even though we don't use the
> >+   * result, because it caches the plugin size in server/backend.c.
> >+   */
> >+  if (next_ops->get_size (nxdata) == -1)
> >+    return -1;
> 
> ...and verify here that the underlying plugin isn't changing size.
> But probably not necessary.

I added the test anyway because it's easy to do.

> With the tar filter, you HAD to call next_ops->get_size() per
> connection, because you called next_ops->pread() per connection.
> But with this filter, you don't actually have to call
> next_ops->get_size(), because your only use of next_ops->pread() is
> during the initial decompression, and the rest of this filter reads
> from local cache rather than deferring to the plugin.

Good point.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html