Libguestfs - Jul 2020 - Re: [PATCH nbdkit 2/2] tar: Rewrite the tar plugin (again), this time in C.

For context see these threads:
https://lists.gnu.org/archive/html/qemu-discuss/2020-06/threads.html#00053
https://lists.gnu.org/archive/html/qemu-block/2020-06/threads.html#01496

Rich.

This reverts commit 2d15e79f65764d9b0c68bea28ed6afbcbcc63467.
---
 configure.ac                                  |   2 +-
 plugins/tar/Makefile.am                       |  13 +-
 tests/Makefile.am                             |   6 +-
 plugins/tar/{nbdkit-tar-plugin.pod => tar.pl} | 119 +++++++++++++++++-
 plugins/tar/tar.py                            | 104 ---------------
 tests/test-dump-plugin.sh                     |   4 +-
 tests/test-help-plugin.sh                     |   4 +-
 tests/test-tar.sh                             |  26 ++--
 tests/test-version-plugin.sh                  |   4 +-
 wrapper.c                                     |  21 +---
 README                                        |   8 +-
 11 files changed, 149 insertions(+), 162 deletions(-)

diff --git a/configure.ac b/configure.ac
index bf389931..af7e6e3b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1222,7 +1222,7 @@ feature "nbd ....................................
" \
 feature "ssh .................................... " \
         test "x$HAVE_SSH_TRUE" = "x"
 feature "tar .................................... " \
-        test "x$HAVE_PYTHON_TRUE" = "x"
+        test "x$HAVE_PERL_TRUE" = "x"
 feature "torrent ................................ " \
         test "x$HAVE_TORRENT_TRUE" = "x"
 feature "vddk ................................... " \
diff --git a/plugins/tar/Makefile.am b/plugins/tar/Makefile.am
index 9e47e01d..09a6caa9 100644
--- a/plugins/tar/Makefile.am
+++ b/plugins/tar/Makefile.am
@@ -31,18 +31,19 @@
 
 include $(top_srcdir)/common-rules.mk
 
+source = tar.pl
+
 EXTRA_DIST = \
-	nbdkit-tar-plugin.pod \
-	tar.py \
+	$(source) \
 	$(NULL)
 
-if HAVE_PYTHON
+if HAVE_PERL
 
 plugin_SCRIPTS = nbdkit-tar-plugin
 CLEANFILES += nbdkit-tar-plugin
 
 # We have to do the rewriting here to avoid stupid exec_prefix.
-nbdkit-tar-plugin: tar.py
+nbdkit-tar-plugin: $(source)
 	rm -f $@ $@-t
 	sed 's,\@sbindir\@,${sbindir},g' < $< > $@-t
 	mv $@-t $@
@@ -53,8 +54,8 @@ if HAVE_POD
 man_MANS = nbdkit-tar-plugin.1
 CLEANFILES += $(man_MANS)
 
-nbdkit-tar-plugin.1: nbdkit-tar-plugin.pod
-	$(PODWRAPPER) --section=1 --man $@ \
+nbdkit-tar-plugin.1: $(source)
+	$(PODWRAPPER) --section=1 --name nbdkit-tar-plugin --man $@ \
 	    --html $(top_builddir)/html/$@.html \
 	    $<
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 51eb7660..16043f49 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -828,10 +828,10 @@ test_streaming_SOURCES = test-streaming.c
 test_streaming_CFLAGS = $(WARNINGS_CFLAGS) $(LIBNBD_CFLAGS)
 test_streaming_LDADD = $(LIBNBD_LIBS)
 
-# tar plugin test (written in python).
-if HAVE_PYTHON
+# tar plugin test (written in perl).
+if HAVE_PERL
 TESTS += test-tar.sh
-endif HAVE_PYTHON
+endif HAVE_PERL
 EXTRA_DIST += test-tar.sh
 
 # tmpdisk plugin test.
diff --git a/plugins/tar/nbdkit-tar-plugin.pod b/plugins/tar/tar.pl
similarity index 55%
rename from plugins/tar/nbdkit-tar-plugin.pod
rename to plugins/tar/tar.pl
index 24a2816b..9b05b83d 100644
--- a/plugins/tar/nbdkit-tar-plugin.pod
+++ b/plugins/tar/tar.pl
@@ -1,3 +1,8 @@
+#!@sbindir@/nbdkit perl
+# -*- perl -*-
+
+=pod
+
 =head1 NAME
 
 nbdkit-tar-plugin - read and write files inside tar files without unpacking
@@ -89,10 +94,11 @@ C<nbdkit-tar-plugin> first appeared in nbdkit 1.2.
 
 =head1 SEE ALSO
 
+L<https://github.com/libguestfs/nbdkit/blob/master/plugins/tar/tar.pl>,
 L<nbdkit(1)>,
 L<nbdkit-offset-filter(1)>,
 L<nbdkit-plugin(3)>,
-L<nbdkit-python-plugin(3)>,
+L<nbdkit-perl-plugin(3)>,
 L<nbdkit-xz-filter(1)>,
 L<tar(1)>.
 
@@ -105,3 +111,114 @@ Based on the virt-v2v OVA importer written by Tomáš
Golembiovský.
 =head1 COPYRIGHT
 
 Copyright (C) 2017-2020 Red Hat Inc.
+
+=cut
+
+use strict;
+
+use Cwd qw(abs_path);
+use IO::File;
+
+my $tar;                        # Tar file.
+my $file;                       # File within the tar file.
+my $offset;                     # Offset within tar file.
+my $size;                       # Size of disk image within tar file.
+
+sub config
+{
+    my $k = shift;
+    my $v = shift;
+
+    if ($k eq "tar") {
+        $tar = abs_path ($v);
+    }
+    elsif ($k eq "file") {
+        $file = $v;
+    }
+    else {
+        die "unknown parameter $k";
+    }
+}
+
+# Check all the config parameters were set.
+sub config_complete
+{
+    die "tar or file parameter was not set\n"
+        unless defined $tar && defined $file;
+
+    die "$tar: file not found\n"
+        unless -f $tar;
+}
+
+# Find the extent of the file within the tar file.
+sub get_ready
+{
+    open (my $pipe, "-|", "tar",
"--no-auto-compress", "-tRvf", $tar, $file)
+        or die "$tar: could not open or parse tar file, see errors
above";
+    while (<$pipe>) {
+        if (/^block\s(\d+):\s\S+\s\S+\s(\d+)/) {
+            # Add one for the tar header, and multiply by the block size.
+            $offset = ($1 + 1) * 512;
+            $size = $2;
+            Nbdkit::debug ("tar: file: $file offset: $offset size:
$size")
+        }
+    }
+    close ($pipe);
+
+    die "offset or size could not be parsed.  Probably the tar file is not
a tar file or the file does not exist in the tar file.  See any errors
above.\n"
+        unless defined $offset && defined $size;
+}
+
+# Accept a connection from a client, create and return the handle
+# which is passed back to other calls.
+sub open
+{
+    my $readonly = shift;
+    my $mode = "<";
+    $mode = "+<" unless $readonly;
+    my $fh = IO::File->new;
+    $fh->open ($tar, $mode) or die "$tar: open: $!";
+    $fh->binmode;
+    my $h = { fh => $fh, readonly => $readonly };
+    return $h;
+}
+
+# Close the connection.
+sub close
+{
+    my $h = shift;
+    my $fh = $h->{fh};
+    $fh->close;
+}
+
+# Return the size.
+sub get_size
+{
+    my $h = shift;
+    return $size;
+}
+
+# Read.
+sub pread
+{
+    my $h = shift;
+    my $fh = $h->{fh};
+    my $count = shift;
+    my $offs = shift;
+    $fh->seek ($offset + $offs, 0) or die "seek: $!";
+    my $r;
+    $fh->read ($r, $count) or die "read: $!";
+    return $r;
+}
+
+# Write.
+sub pwrite
+{
+    my $h = shift;
+    my $fh = $h->{fh};
+    my $buf = shift;
+    my $count = length ($buf);
+    my $offs = shift;
+    $fh->seek ($offset + $offs, 0) or die "seek: $!";
+    print $fh ($buf);
+}
diff --git a/plugins/tar/tar.py b/plugins/tar/tar.py
deleted file mode 100644
index 10a8066e..00000000
--- a/plugins/tar/tar.py
+++ /dev/null
@@ -1,104 +0,0 @@
-#!@sbindir@/nbdkit python
-# -*- python -*-
-# 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.
-
-# Note this uses API v1 since we may wish to copy it and use it with
-# older versions of nbdkit.  Also it doesn't use get_ready() for the
-# same reason.
-
-import builtins
-import os.path
-import tarfile
-import nbdkit
-
-tar = None                      # Tar file.
-f = None                        # File within the tar file.
-offset = None                   # Offset of file within the tar file.
-size = None                     # Size of file within the tar file.
-
-def config(k, v):
-    global tar, f
-
-    if k == "tar":
-        tar = os.path.abspath(v)
-    elif k == "file":
-        f = v
-    else:
-        raise RuntimeError("unknown parameter: %s" % key)
-
-# Check all the config parameters were set.
-def config_complete():
-    global tar, f, offset, size
-
-    if tar is None or f is None:
-        raise RuntimeError("tar or file parameter was not set")
-    if not os.path.exists(tar):
-        raise RuntimeError("$s: file not found" % tar)
-
-    # Find the extent of the file within the tar file.
-    for m in tarfile.open(tar, mode='r:'):
-        if m.name == f:
-            offset = m.offset_data
-            size = m.size
-    if offset is None or size is None:
-        raise RuntimeError("offset or size could not be parsed.  Probably
the tar file is not a tar file or the file does not exist in the tar
file.")
-
-# Accept a connection from a client, create and return the handle
-# which is passed back to other calls.
-def open(readonly):
-    global tar
-    if readonly:
-        mode = 'rb'
-    else:
-        mode = 'r+b'
-    return { 'fh': builtins.open(tar, mode) }
-
-# Close the connection.
-def close(h):
-    h['fh'].close()
-
-# Return the size.
-def get_size(h):
-    global size
-    return size
-
-# Read.
-#
-# Python plugin thread model is always
-# NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS so seeking here is fine.
-def pread(h, count, offs):
-    global offset
-    h['fh'].seek(offset + offs)
-    return h['fh'].read(count)
-
-# Write.
-def pwrite(h, buf, offs):
-    global offset
-    h['fh'].seek(offset + offs)
-    h['fh'].write(buf)
diff --git a/tests/test-dump-plugin.sh b/tests/test-dump-plugin.sh
index 629182ef..0b4c1ce1 100755
--- a/tests/test-dump-plugin.sh
+++ b/tests/test-dump-plugin.sh
@@ -43,10 +43,10 @@ do_test ()
 {
     vg=; [ "$NBDKIT_VALGRIND" = "1" ] &&
vg="-valgrind"
     case "$1$vg" in
-        python-valgrind | ruby-valgrind | tar-valgrind | tcl-valgrind)
+        python-valgrind | ruby-valgrind | tcl-valgrind)
             echo "$0: skipping $1$vg because this language doesn't
support valgrind"
             ;;
-        example4*)
+        example4* | tar*)
             # These tests are written in Perl so we have to check that
             # the Perl plugin was compiled.
             if nbdkit perl --version; then run_test $1; fi
diff --git a/tests/test-help-plugin.sh b/tests/test-help-plugin.sh
index 65036f2e..7dc26ece 100755
--- a/tests/test-help-plugin.sh
+++ b/tests/test-help-plugin.sh
@@ -43,10 +43,10 @@ do_test ()
 {
     vg=; [ "$NBDKIT_VALGRIND" = "1" ] &&
vg="-valgrind"
     case "$1$vg" in
-        python-valgrind | ruby-valgrind | tar-valgrind | tcl-valgrind)
+        python-valgrind | ruby-valgrind | tcl-valgrind)
             echo "$0: skipping $1$vg because this language doesn't
support valgrind"
             ;;
-        example4*)
+        example4* | tar*)
             # These tests are written in Perl so we have to check that
             # the Perl plugin was compiled.
             if nbdkit perl --version; then run_test $1; fi
diff --git a/tests/test-tar.sh b/tests/test-tar.sh
index f1cebf19..c6d726c4 100755
--- a/tests/test-tar.sh
+++ b/tests/test-tar.sh
@@ -34,18 +34,14 @@ source ./functions.sh
 set -e
 set -x
 
-# Python scripts break valgrind.
-if [ "$NBDKIT_VALGRIND" = "1" ]; then
-    echo "$0: skipping Python test under valgrind."
-    exit 77
-fi
-
 requires test -f disk
 requires guestfish --version
+requires tar --version
 
-# The tar plugin is written in Python and uses the tarfile module.
-requires python --version
-requires python -c 'import tarfile'
+# The tar plugin requires some Perl modules, this checks if they are
+# installed.
+requires perl -MCwd -e 1
+requires perl -MIO::File -e 1
 
 sock=`mktemp -u`
 files="tar.pid tar.tar $sock"
@@ -58,15 +54,7 @@ tar cf tar.tar disk
 # Run nbdkit.
 start_nbdkit -P tar.pid -U $sock tar tar=tar.tar file=disk
 
-# Now see if we can open, read and write the disk from the tar file.
-guestfish -x --format=raw -a "nbd://?socket=$sock" -m /dev/sda1
<<EOF
-  # Check for existing file.
+# Now see if we can open the disk from the tar file.
+guestfish -x --ro --format=raw -a "nbd://?socket=$sock" -m /dev/sda1
<<EOF
   cat /hello.txt
-
-  # Write a new file.
-  write /test.txt "hello"
-  cat /test.txt
 EOF
-
-# Check that the tar file isn't corrupt.
-tar tvvf tar.tar
diff --git a/tests/test-version-plugin.sh b/tests/test-version-plugin.sh
index 8a4a2ece..7d2ab072 100755
--- a/tests/test-version-plugin.sh
+++ b/tests/test-version-plugin.sh
@@ -43,10 +43,10 @@ do_test ()
 {
     vg=; [ "$NBDKIT_VALGRIND" = "1" ] &&
vg="-valgrind"
     case "$1$vg" in
-        python-valgrind | ruby-valgrind | tar-valgrind | tcl-valgrind)
+        python-valgrind | ruby-valgrind | tcl-valgrind)
             echo "$0: skipping $1$vg because this language doesn't
support valgrind"
             ;;
-        example4*)
+        example4* | tar*)
             # These tests are written in Perl so we have to check that
             # the Perl plugin was compiled.
             if nbdkit perl --version; then run_test $1; fi
diff --git a/wrapper.c b/wrapper.c
index 24dab6ab..b1e2ce2f 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -78,20 +78,14 @@
 static const char **cmd;
 static size_t len;
 
-/* Plugins written in scripting languages need to be rewritten on the
- * command line in a different way from plugins written in C.  So we
- * have to list those here.
+/* Plugins written in scripting languages (only Perl right now) need
+ * to be rewritten on the command line in a different way from plugins
+ * written in C.  So we have to list those here.
  */
 static bool
 is_perl_plugin (const char *name)
 {
-  return strcmp (name, "example4") == 0;
-}
-
-static bool
-is_python_plugin (const char *name)
-{
-  return strcmp (name, "tar") == 0;
+  return strcmp (name, "example4") == 0 || strcmp (name,
"tar") == 0;
 }
 
 static void
@@ -269,13 +263,6 @@ main (int argc, char *argv[])
         passthru_format ("%s/plugins/%s/nbdkit-%s-plugin",
                          builddir, argv[optind], argv[optind]);
       }
-      /* Special plugins written in Python. */
-      else if (is_python_plugin (argv[optind])) {
-        passthru_format
("%s/plugins/python/.libs/nbdkit-python-plugin.so",
-                         builddir);
-        passthru_format ("%s/plugins/%s/nbdkit-%s-plugin",
-                         builddir, argv[optind], argv[optind]);
-      }
       else {
         passthru_format ("%s/plugins/%s/.libs/nbdkit-%s-plugin.so",
                          builddir, argv[optind], argv[optind]);
diff --git a/README b/README
index 88fecef6..4f584828 100644
--- a/README
+++ b/README
@@ -124,15 +124,15 @@ For the bittorrent plugin:
 
  - libtorrent-rasterbar (https://www.libtorrent.org)
 
-For the Perl and example4 plugins:
+For the Perl, example4 and tar plugins:
 
  - perl interpreter
 
  - perl development libraries
 
- - perl modules ExtUtils::Embed
+ - perl modules ExtUtils::Embed, IO::File and Cwd
 
-For the Python and tar plugins:
+For the Python plugin:
 
  - python interpreter (version 3 only)
 
@@ -140,8 +140,6 @@ For the Python and tar plugins:
 
  - python unittest to run the test suite
 
- - python tarfile.py (part of standard library)
-
 For the OCaml plugin:
 
  - OCaml >= 4.02.2
-- 
2.25.0

---
 plugins/tar/{tar.pl => nbdkit-tar-plugin.pod} | 145 ++-------
 configure.ac                                  |   2 -
 plugins/tar/Makefile.am                       |  41 +--
 tests/Makefile.am                             |  14 +-
 plugins/tar/tar.c                             | 286 ++++++++++++++++++
 tests/test-dump-plugin.sh                     |   2 +-
 tests/test-help-plugin.sh                     |   2 +-
 tests/test-tar-info.sh                        |  67 ++++
 tests/test-tar.sh                             |  22 +-
 tests/test-version-plugin.sh                  |   2 +-
 wrapper.c                                     |   2 +-
 .gitignore                                    |   1 -
 README                                        |   4 +-
 13 files changed, 427 insertions(+), 163 deletions(-)

diff --git a/plugins/tar/tar.pl b/plugins/tar/nbdkit-tar-plugin.pod
similarity index 52%
rename from plugins/tar/tar.pl
rename to plugins/tar/nbdkit-tar-plugin.pod
index 9b05b83d..589e114b 100644
--- a/plugins/tar/tar.pl
+++ b/plugins/tar/nbdkit-tar-plugin.pod
@@ -1,21 +1,16 @@
-#!@sbindir@/nbdkit perl
-# -*- perl -*-
-
-=pod
-
 =head1 NAME
 
 nbdkit-tar-plugin - read and write files inside tar files without unpacking
 
 =head1 SYNOPSIS
 
- nbdkit tar tar=FILENAME.tar file=PATH_INSIDE_TAR
+ nbdkit tar [tar=]FILENAME.tar file=PATH_INSIDE_TAR
 
 =head1 EXAMPLES
 
 =head2 Serve a single file inside a tarball
 
- nbdkit tar tar=file.tar file=some/disk.img
+ nbdkit tar file.tar file=some/disk.img
  guestfish --format=raw -a nbd://localhost
 
 =head2 Opening a disk image inside an OVA file
@@ -28,7 +23,7 @@ could access one file in an OVA like this:
  rhel.ovf
  rhel-disk1.vmdk
  rhel.mf
- $ nbdkit -r tar tar=rhel.ova file=rhel-disk1.vmdk
+ $ nbdkit -r tar rhel.ova file=rhel-disk1.vmdk
  $ guestfish --ro --format=vmdk -a nbd://localhost
 
 In this case the tarball is opened readonly (I<-r> option).  The
@@ -55,6 +50,27 @@ modified.  Without I<-r> writes will modify the tar
file.
 
 The disk image cannot be resized.
 
+=head1 PARAMETERS
+
+=over 4
+
+=item [B<tar=>]FILENAME.tar
+
+The path of the tarball.  This parameter is required.
+
+C<tar=> is a magic config key and may be omitted in most cases.
+See L<nbdkit(1)/Magic parameters>.
+
+=item B<file=>PATH_INSIDE_TAR
+
+The path of the file inside the tarball to serve.  This parameter is
+required.  It must exactly match the name stored in the tarball, so
+use S<C<tar tf filename.tar>>
+
+=back
+
+=head1 NOTE
+
 =head2 Alternatives to the tar plugin
 
 The tar plugin ought to be a filter so that you can extract files from
@@ -94,11 +110,9 @@ C<nbdkit-tar-plugin> first appeared in nbdkit 1.2.
 
 =head1 SEE ALSO
 
-L<https://github.com/libguestfs/nbdkit/blob/master/plugins/tar/tar.pl>,
 L<nbdkit(1)>,
 L<nbdkit-offset-filter(1)>,
 L<nbdkit-plugin(3)>,
-L<nbdkit-perl-plugin(3)>,
 L<nbdkit-xz-filter(1)>,
 L<tar(1)>.
 
@@ -111,114 +125,3 @@ Based on the virt-v2v OVA importer written by Tomáš
Golembiovský.
 =head1 COPYRIGHT
 
 Copyright (C) 2017-2020 Red Hat Inc.
-
-=cut
-
-use strict;
-
-use Cwd qw(abs_path);
-use IO::File;
-
-my $tar;                        # Tar file.
-my $file;                       # File within the tar file.
-my $offset;                     # Offset within tar file.
-my $size;                       # Size of disk image within tar file.
-
-sub config
-{
-    my $k = shift;
-    my $v = shift;
-
-    if ($k eq "tar") {
-        $tar = abs_path ($v);
-    }
-    elsif ($k eq "file") {
-        $file = $v;
-    }
-    else {
-        die "unknown parameter $k";
-    }
-}
-
-# Check all the config parameters were set.
-sub config_complete
-{
-    die "tar or file parameter was not set\n"
-        unless defined $tar && defined $file;
-
-    die "$tar: file not found\n"
-        unless -f $tar;
-}
-
-# Find the extent of the file within the tar file.
-sub get_ready
-{
-    open (my $pipe, "-|", "tar",
"--no-auto-compress", "-tRvf", $tar, $file)
-        or die "$tar: could not open or parse tar file, see errors
above";
-    while (<$pipe>) {
-        if (/^block\s(\d+):\s\S+\s\S+\s(\d+)/) {
-            # Add one for the tar header, and multiply by the block size.
-            $offset = ($1 + 1) * 512;
-            $size = $2;
-            Nbdkit::debug ("tar: file: $file offset: $offset size:
$size")
-        }
-    }
-    close ($pipe);
-
-    die "offset or size could not be parsed.  Probably the tar file is not
a tar file or the file does not exist in the tar file.  See any errors
above.\n"
-        unless defined $offset && defined $size;
-}
-
-# Accept a connection from a client, create and return the handle
-# which is passed back to other calls.
-sub open
-{
-    my $readonly = shift;
-    my $mode = "<";
-    $mode = "+<" unless $readonly;
-    my $fh = IO::File->new;
-    $fh->open ($tar, $mode) or die "$tar: open: $!";
-    $fh->binmode;
-    my $h = { fh => $fh, readonly => $readonly };
-    return $h;
-}
-
-# Close the connection.
-sub close
-{
-    my $h = shift;
-    my $fh = $h->{fh};
-    $fh->close;
-}
-
-# Return the size.
-sub get_size
-{
-    my $h = shift;
-    return $size;
-}
-
-# Read.
-sub pread
-{
-    my $h = shift;
-    my $fh = $h->{fh};
-    my $count = shift;
-    my $offs = shift;
-    $fh->seek ($offset + $offs, 0) or die "seek: $!";
-    my $r;
-    $fh->read ($r, $count) or die "read: $!";
-    return $r;
-}
-
-# Write.
-sub pwrite
-{
-    my $h = shift;
-    my $fh = $h->{fh};
-    my $buf = shift;
-    my $count = length ($buf);
-    my $offs = shift;
-    $fh->seek ($offset + $offs, 0) or die "seek: $!";
-    print $fh ($buf);
-}
diff --git a/configure.ac b/configure.ac
index af7e6e3b..237ecd75 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1221,8 +1221,6 @@ feature "nbd ....................................
" \
         test "x$HAVE_LIBNBD_TRUE" = "x"
 feature "ssh .................................... " \
         test "x$HAVE_SSH_TRUE" = "x"
-feature "tar .................................... " \
-        test "x$HAVE_PERL_TRUE" = "x"
 feature "torrent ................................ " \
         test "x$HAVE_TORRENT_TRUE" = "x"
 feature "vddk ................................... " \
diff --git a/plugins/tar/Makefile.am b/plugins/tar/Makefile.am
index 09a6caa9..14d57e04 100644
--- a/plugins/tar/Makefile.am
+++ b/plugins/tar/Makefile.am
@@ -1,5 +1,5 @@
 # nbdkit
-# Copyright (C) 2013-2020 Red Hat Inc.
+# 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
@@ -31,34 +31,37 @@
 
 include $(top_srcdir)/common-rules.mk
 
-source = tar.pl
+EXTRA_DIST = nbdkit-tar-plugin.pod
 
-EXTRA_DIST = \
-	$(source) \
-	$(NULL)
-
-if HAVE_PERL
+plugin_LTLIBRARIES = nbdkit-tar-plugin.la
 
-plugin_SCRIPTS = nbdkit-tar-plugin
-CLEANFILES += nbdkit-tar-plugin
+nbdkit_tar_plugin_la_SOURCES = \
+	tar.c \
+	$(top_srcdir)/include/nbdkit-plugin.h \
+	$(NULL)
 
-# We have to do the rewriting here to avoid stupid exec_prefix.
-nbdkit-tar-plugin: $(source)
-	rm -f $@ $@-t
-	sed 's,\@sbindir\@,${sbindir},g' < $< > $@-t
-	mv $@-t $@
-	chmod 0555 $@
+nbdkit_tar_plugin_la_CPPFLAGS = \
+	-I$(top_srcdir)/common/utils \
+	-I$(top_srcdir)/include \
+	-I. \
+	$(NULL)
+nbdkit_tar_plugin_la_CFLAGS = $(WARNINGS_CFLAGS)
+nbdkit_tar_plugin_la_LDFLAGS = \
+	-module -avoid-version -shared $(SHARED_LDFLAGS) \
+	-Wl,--version-script=$(top_srcdir)/plugins/plugins.syms \
+	$(NULL)
+nbdkit_tar_plugin_la_LIBADD = \
+	$(top_builddir)/common/utils/libutils.la \
+	$(NULL)
 
 if HAVE_POD
 
 man_MANS = nbdkit-tar-plugin.1
 CLEANFILES += $(man_MANS)
 
-nbdkit-tar-plugin.1: $(source)
-	$(PODWRAPPER) --section=1 --name nbdkit-tar-plugin --man $@ \
+nbdkit-tar-plugin.1: nbdkit-tar-plugin.pod
+	$(PODWRAPPER) --section=1 --man $@ \
 	    --html $(top_builddir)/html/$@.html \
 	    $<
 
 endif HAVE_POD
-
-endif
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 16043f49..9ab6a7af 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -828,11 +828,15 @@ test_streaming_SOURCES = test-streaming.c
 test_streaming_CFLAGS = $(WARNINGS_CFLAGS) $(LIBNBD_CFLAGS)
 test_streaming_LDADD = $(LIBNBD_LIBS)
 
-# tar plugin test (written in perl).
-if HAVE_PERL
-TESTS += test-tar.sh
-endif HAVE_PERL
-EXTRA_DIST += test-tar.sh
+# tar plugin test.
+TESTS += \
+	test-tar.sh \
+	test-tar-info.sh \
+	$(NULL)
+EXTRA_DIST += \
+	test-tar.sh \
+	test-tar-info.sh \
+	$(NULL)
 
 # tmpdisk plugin test.
 LIBGUESTFS_TESTS += test-tmpdisk
diff --git a/plugins/tar/tar.c b/plugins/tar/tar.c
new file mode 100644
index 00000000..fe298035
--- /dev/null
+++ b/plugins/tar/tar.c
@@ -0,0 +1,286 @@
+/* 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.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <inttypes.h>
+#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <assert.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include <nbdkit-plugin.h>
+
+#include "cleanup.h"
+#include "utils.h"
+
+static char *tarfile;           /* The tar file (tar= parameter). */
+static const char *file;        /* File within tar (file=). */
+static uint64_t offset, size;   /* Offset and size within tarball. */
+
+static void
+tar_unload (void)
+{
+  free (tarfile);
+}
+
+static int
+tar_config (const char *key, const char *value)
+{
+  if (strcmp (key, "tar") == 0) {
+    if (tarfile) {
+      nbdkit_error ("only one tar parameter can be given");
+      return -1;
+    }
+    tarfile = nbdkit_realpath (value);
+    if (tarfile == NULL)
+      return -1;
+  }
+  else if (strcmp (key, "file") == 0) {
+    if (file) {
+      nbdkit_error ("only one file parameter can be given");
+      return -1;
+    }
+    file = value;
+  }
+  else {
+    nbdkit_error ("unknown parameter '%s'", key);
+    return -1;
+  }
+
+  return 0;
+}
+
+static int
+tar_config_complete (void)
+{
+  if (tarfile == NULL || file == NULL) {
+    nbdkit_error ("you must supply the tar=<TARFILE> and
file=<FILENAME> "
+                  "parameters");
+    return -1;
+  }
+
+  return 0;
+}
+
+#define tar_config_help \
+  "[tar=]<TARBALL>     (required) The name of the tar file.\n"
\
+  "file=<FILENAME>                The path inside the tar file to
server."
+
+static int
+tar_get_ready (void)
+{
+  FILE *fp;
+  CLEANUP_FREE char *cmd = NULL;
+  size_t len = 0;
+  bool scanned_ok;
+  char s[256];
+
+  /* Construct the tar command to examine the tar file. */
+  fp = open_memstream (&cmd, &len);
+  if (fp == NULL) {
+    nbdkit_error ("open_memstream: %m");
+    return -1;
+  }
+  fprintf (fp, "LANG=C tar --no-auto-compress -tRvf ");
+  shell_quote (tarfile, fp);
+  fputc (' ', fp);
+  shell_quote (file, fp);
+  if (fclose (fp) == EOF) {
+    nbdkit_error ("memstream failed: %m");
+    return -1;
+  }
+
+  /* Run the command and read the first line of the output. */
+  nbdkit_debug ("%s", cmd);
+  fp = popen (cmd, "r");
+  if (fp == NULL) {
+    nbdkit_error ("tar: %m");
+    return -1;
+  }
+  scanned_ok = fscanf (fp, "block %" SCNu64 ": %*s %*s %"
SCNu64,
+                       &offset, &size) == 2;
+  /* We have to now read and discard the rest of the output until EOF. */
+  while (fread (s, sizeof s, 1, fp) > 0)
+    ;
+  if (pclose (fp) != 0) {
+    nbdkit_error ("tar subcommand failed, "
+                  "check that the file really exists in the
tarball");
+    return -1;
+  }
+
+  if (!scanned_ok) {
+    nbdkit_error ("unexpected output from the tar subcommand");
+    return -1;
+  }
+
+  /* Adjust the offset: Add 1 for the tar header, then multiply by the
+   * block size.
+   */
+  offset = (offset+1) * 512;
+
+  nbdkit_debug ("tar: offset %" PRIu64 ", size %" PRIu64,
offset, size);
+
+  /* Check it looks sensible.  XXX We ought to check it doesn't exceed
+   * the size of the tar file.
+   */
+  if (offset >= INT64_MAX || size >= INT64_MAX) {
+    nbdkit_error ("internal error: calculated offset and size are
wrong");
+    return -1;
+  }
+
+  return 0;
+}
+
+struct handle {
+  int fd;
+};
+
+static void *
+tar_open (int readonly)
+{
+  struct handle *h;
+
+  assert (offset > 0);     /* Cannot be zero because of tar header. */
+
+  h = calloc (1, sizeof *h);
+  if (h == NULL) {
+    nbdkit_error ("calloc: %m");
+    return NULL;
+  }
+  h->fd = open (tarfile, (readonly ? O_RDONLY : O_RDWR) | O_CLOEXEC);
+  if (h->fd == -1) {
+    nbdkit_error ("%s: %m", tarfile);
+    free (h);
+    return NULL;
+  }
+
+  return h;
+}
+
+#define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL
+
+/* Get the file size. */
+static int64_t
+tar_get_size (void *handle)
+{
+  return size;
+}
+
+/* Serves the same data over multiple connections. */
+static int
+tar_can_multi_conn (void *handle)
+{
+  return 1;
+}
+
+static int
+tar_can_cache (void *handle)
+{
+  /* Let nbdkit call pread to populate the file system cache. */
+  return NBDKIT_CACHE_EMULATE;
+}
+
+/* Read data from the file. */
+static int
+tar_pread (void *handle, void *buf, uint32_t count, uint64_t offs)
+{
+  struct handle *h = handle;
+
+  offs += offset;
+
+  while (count > 0) {
+    ssize_t r = pread (h->fd, buf, count, offs);
+    if (r == -1) {
+      nbdkit_error ("pread: %m");
+      return -1;
+    }
+    if (r == 0) {
+      nbdkit_error ("pread: unexpected end of file");
+      return -1;
+    }
+    buf += r;
+    count -= r;
+    offs += r;
+  }
+
+  return 0;
+}
+
+/* Write data to the file. */
+static int
+tar_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offs)
+{
+  struct handle *h = handle;
+
+  offs += offset;
+
+  while (count > 0) {
+    ssize_t r = pwrite (h->fd, buf, count, offs);
+    if (r == -1) {
+      nbdkit_error ("pwrite: %m");
+      return -1;
+    }
+    buf += r;
+    count -= r;
+    offs += r;
+  }
+
+  return 0;
+}
+
+static struct nbdkit_plugin plugin = {
+  .name              = "tar",
+  .longname          = "nbdkit tar plugin",
+  .version           = PACKAGE_VERSION,
+  .unload            = tar_unload,
+  .config            = tar_config,
+  .config_complete   = tar_config_complete,
+  .config_help       = tar_config_help,
+  .magic_config_key  = "tar",
+  .get_ready         = tar_get_ready,
+  .open              = tar_open,
+  .get_size          = tar_get_size,
+  .can_multi_conn    = tar_can_multi_conn,
+  .can_cache         = tar_can_cache,
+  .pread             = tar_pread,
+  .pwrite            = tar_pwrite,
+  .errno_is_preserved = 1,
+};
+
+NBDKIT_REGISTER_PLUGIN(plugin)
diff --git a/tests/test-dump-plugin.sh b/tests/test-dump-plugin.sh
index 0b4c1ce1..6eb25a65 100755
--- a/tests/test-dump-plugin.sh
+++ b/tests/test-dump-plugin.sh
@@ -46,7 +46,7 @@ do_test ()
         python-valgrind | ruby-valgrind | tcl-valgrind)
             echo "$0: skipping $1$vg because this language doesn't
support valgrind"
             ;;
-        example4* | tar*)
+        example4*)
             # These tests are written in Perl so we have to check that
             # the Perl plugin was compiled.
             if nbdkit perl --version; then run_test $1; fi
diff --git a/tests/test-help-plugin.sh b/tests/test-help-plugin.sh
index 7dc26ece..f0dfa7df 100755
--- a/tests/test-help-plugin.sh
+++ b/tests/test-help-plugin.sh
@@ -46,7 +46,7 @@ do_test ()
         python-valgrind | ruby-valgrind | tcl-valgrind)
             echo "$0: skipping $1$vg because this language doesn't
support valgrind"
             ;;
-        example4* | tar*)
+        example4*)
             # These tests are written in Perl so we have to check that
             # the Perl plugin was compiled.
             if nbdkit perl --version; then run_test $1; fi
diff --git a/tests/test-tar-info.sh b/tests/test-tar-info.sh
new file mode 100755
index 00000000..efaa8ec8
--- /dev/null
+++ b/tests/test-tar-info.sh
@@ -0,0 +1,67 @@
+#!/usr/bin/env bash
+# nbdkit
+# 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
+# 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.
+
+# Test that qemu-img info works on a qcow2 file in a tar file.
+
+source ./functions.sh
+set -e
+set -x
+
+requires test -f disk
+requires guestfish --version
+requires tar --version
+requires qemu-img --version
+requires qemu-img info --output=json /dev/null
+requires jq --version
+requires stat --version
+
+disk=tar-info-disk.qcow2
+out=tar-info.out
+tar=tar-info.tar
+files="$disk $out $tar"
+rm -f $files
+cleanup_fn rm -f $files
+
+# Create a tar file containing a known qcow2 file.
+qemu-img convert -f raw disk -O qcow2 $disk
+tar cf $tar $disk
+
+# Run nbdkit.
+nbdkit -U - tar $tar file=$disk --run 'qemu-img info --output=json
$nbd' > $out
+cat $out
+
+# Check various fields in the input.
+# Virtual size must be the same as the size of the original raw disk.
+test "$( jq -r -c '.["virtual-size"]' $out )" -eq
"$( stat -c %s disk )"
+
+# Format must be qcow2.
+test "$( jq -r -c '.["format"]' $out )" =
"qcow2"
diff --git a/tests/test-tar.sh b/tests/test-tar.sh
index c6d726c4..3164b826 100755
--- a/tests/test-tar.sh
+++ b/tests/test-tar.sh
@@ -38,23 +38,27 @@ requires test -f disk
 requires guestfish --version
 requires tar --version
 
-# The tar plugin requires some Perl modules, this checks if they are
-# installed.
-requires perl -MCwd -e 1
-requires perl -MIO::File -e 1
-
 sock=`mktemp -u`
 files="tar.pid tar.tar $sock"
 rm -f $files
 cleanup_fn rm -f $files
 
-# Create a tar file containing the disk image.
-tar cf tar.tar disk
+# Create a tar file containing the disk image plus some other random
+# files that hopefully will be ignored.
+tar cf tar.tar test-tar.sh Makefile disk Makefile.am
+tar tvvf tar.tar
 
 # Run nbdkit.
 start_nbdkit -P tar.pid -U $sock tar tar=tar.tar file=disk
 
-# Now see if we can open the disk from the tar file.
-guestfish -x --ro --format=raw -a "nbd://?socket=$sock" -m /dev/sda1
<<EOF
+# Now see if we can open, read and write the disk from the tar file.
+guestfish -x --format=raw -a "nbd://?socket=$sock" -m /dev/sda1
<<EOF
   cat /hello.txt
+
+  # Write a new file.
+  write /test.txt "hello"
+  cat /test.txt
 EOF
+
+# Check that the tar file isn't corrupt.
+tar tvvf tar.tar
diff --git a/tests/test-version-plugin.sh b/tests/test-version-plugin.sh
index 7d2ab072..c397afc2 100755
--- a/tests/test-version-plugin.sh
+++ b/tests/test-version-plugin.sh
@@ -46,7 +46,7 @@ do_test ()
         python-valgrind | ruby-valgrind | tcl-valgrind)
             echo "$0: skipping $1$vg because this language doesn't
support valgrind"
             ;;
-        example4* | tar*)
+        example4*)
             # These tests are written in Perl so we have to check that
             # the Perl plugin was compiled.
             if nbdkit perl --version; then run_test $1; fi
diff --git a/wrapper.c b/wrapper.c
index b1e2ce2f..c27afae0 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -85,7 +85,7 @@ static size_t len;
 static bool
 is_perl_plugin (const char *name)
 {
-  return strcmp (name, "example4") == 0 || strcmp (name,
"tar") == 0;
+  return strcmp (name, "example4") == 0;
 }
 
 static void
diff --git a/.gitignore b/.gitignore
index f02d19dc..48a5302e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -79,7 +79,6 @@ plugins/*/*.3
 /plugins/ocaml/nbdkit-ocamlexample-plugin.so
 /plugins/rust/Cargo.lock
 /plugins/rust/target
-/plugins/tar/nbdkit-tar-plugin
 /plugins/tmpdisk/default-command.c
 /podwrapper.pl
 /server/local/nbdkit.pc
diff --git a/README b/README
index 4f584828..7733761e 100644
--- a/README
+++ b/README
@@ -124,13 +124,13 @@ For the bittorrent plugin:
 
  - libtorrent-rasterbar (https://www.libtorrent.org)
 
-For the Perl, example4 and tar plugins:
+For the Perl and example4 plugins:
 
  - perl interpreter
 
  - perl development libraries
 
- - perl modules ExtUtils::Embed, IO::File and Cwd
+ - perl modules ExtUtils::Embed
 
 For the Python plugin:
 
-- 
2.25.0

On Sun, Jun 28, 2020 at 02:02:51PM +0100, Richard W.M. Jones
wrote:
> +static void *
> +tar_open (int readonly)
> +{
> +  struct handle *h;
> +
> +  assert (offset > 0);     /* Cannot be zero because of tar header. */
> +
> +  h = calloc (1, sizeof *h);
> +  if (h == NULL) {
> +    nbdkit_error ("calloc: %m");
> +    return NULL;
> +  }
> +  h->fd = open (tarfile, (readonly ? O_RDONLY : O_RDWR) | O_CLOEXEC);
> +  if (h->fd == -1) {
> +    nbdkit_error ("%s: %m", tarfile);
> +    free (h);
> +    return NULL;
> +  }
> +
> +  return h;
> +}
Oops, I forgot to close the file descriptor, so this needs an
accompanying close callback:

static void
tar_close (void *handle)
{
  struct handle *h = handle;

  close (h->fd);
}

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

On Sun, Jun 28, 2020 at 4:03 PM Richard W.M. Jones <rjones@redhat.com>
wrote:
...
> +
> +static int
> +tar_get_ready (void)
> +{
> +  FILE *fp;
> +  CLEANUP_FREE char *cmd = NULL;
> +  size_t len = 0;
> +  bool scanned_ok;
> +  char s[256];
> +
> +  /* Construct the tar command to examine the tar file. */
> +  fp = open_memstream (&cmd, &len);
> +  if (fp == NULL) {
> +    nbdkit_error ("open_memstream: %m");
> +    return -1;
> +  }
> +  fprintf (fp, "LANG=C tar --no-auto-compress -tRvf ");
Using -R is nice, but is block size documented?

Also --block-number would be nicer.
> +  shell_quote (tarfile, fp);
This is questionable. Why use string and quote the string instead of using
argv directly?
> +  fputc (' ', fp);
> +  shell_quote (file, fp);
> +  if (fclose (fp) == EOF) {
> +    nbdkit_error ("memstream failed: %m");
> +    return -1;
> +  }
> +
> +  /* Run the command and read the first line of the output. */
> +  nbdkit_debug ("%s", cmd);
> +  fp = popen (cmd, "r");
> +  if (fp == NULL) {
> +    nbdkit_error ("tar: %m");
> +    return -1;
> +  }
> +  scanned_ok = fscanf (fp, "block %" SCNu64 ": %*s %*s
%" SCNu64,
> +                       &offset, &size) == 2;
> +  /* We have to now read and discard the rest of the output until EOF. */
> +  while (fread (s, sizeof s, 1, fp) > 0)
> +    ;
> +  if (pclose (fp) != 0) {
> +    nbdkit_error ("tar subcommand failed, "
> +                  "check that the file really exists in the
tarball");
> +    return -1;
> +  }
> +
> +  if (!scanned_ok) {
> +    nbdkit_error ("unexpected output from the tar subcommand");
> +    return -1;
> +  }
> +
> +  /* Adjust the offset: Add 1 for the tar header, then multiply by the
> +   * block size.
> +   */
> +  offset = (offset+1) * 512;
> +
> +  nbdkit_debug ("tar: offset %" PRIu64 ", size %"
PRIu64, offset, size);
> +
> +  /* Check it looks sensible.  XXX We ought to check it doesn't exceed
> +   * the size of the tar file.
> +   */
> +  if (offset >= INT64_MAX || size >= INT64_MAX) {
> +    nbdkit_error ("internal error: calculated offset and size are
wrong");
> +    return -1;
> +  }
> +
> +  return 0;
> +}
> +
> +struct handle {
> +  int fd;
> +};
> +
> +static void *
> +tar_open (int readonly)
> +{
> +  struct handle *h;
> +
> +  assert (offset > 0);     /* Cannot be zero because of tar header. */
> +
> +  h = calloc (1, sizeof *h);
> +  if (h == NULL) {
> +    nbdkit_error ("calloc: %m");
> +    return NULL;
> +  }
> +  h->fd = open (tarfile, (readonly ? O_RDONLY : O_RDWR) | O_CLOEXEC);
> +  if (h->fd == -1) {
> +    nbdkit_error ("%s: %m", tarfile);
> +    free (h);
> +    return NULL;
> +  }
> +
> +  return h;
> +}
> +
> +#define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL
> +
> +/* Get the file size. */
> +static int64_t
> +tar_get_size (void *handle)
> +{
> +  return size;
> +}
> +
> +/* Serves the same data over multiple connections. */
> +static int
> +tar_can_multi_conn (void *handle)
> +{
> +  return 1;
> +}
> +
> +static int
> +tar_can_cache (void *handle)
> +{
> +  /* Let nbdkit call pread to populate the file system cache. */
> +  return NBDKIT_CACHE_EMULATE;
> +}
> +
> +/* Read data from the file. */
> +static int
> +tar_pread (void *handle, void *buf, uint32_t count, uint64_t offs)
This should be identical to file plugin, on? can we reuse the same code
for reading files?
> +{
> +  struct handle *h = handle;
> +
> +  offs += offset;
> +
> +  while (count > 0) {
> +    ssize_t r = pread (h->fd, buf, count, offs);
> +    if (r == -1) {
> +      nbdkit_error ("pread: %m");
> +      return -1;
> +    }
> +    if (r == 0) {
> +      nbdkit_error ("pread: unexpected end of file");
> +      return -1;
> +    }
> +    buf += r;
> +    count -= r;
> +    offs += r;
> +  }
> +
> +  return 0;
> +}
> +
> +/* Write data to the file. */
> +static int
> +tar_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offs)
> +{
> +  struct handle *h = handle;
> +
> +  offs += offset;
> +
> +  while (count > 0) {
> +    ssize_t r = pwrite (h->fd, buf, count, offs);
> +    if (r == -1) {
> +      nbdkit_error ("pwrite: %m");
> +      return -1;
> +    }
> +    buf += r;
> +    count -= r;
> +    offs += r;
> +  }
> +
> +  return 0;
> +}
> +
> +static struct nbdkit_plugin plugin = {
> +  .name              = "tar",
> +  .longname          = "nbdkit tar plugin",
> +  .version           = PACKAGE_VERSION,
> +  .unload            = tar_unload,
> +  .config            = tar_config,
> +  .config_complete   = tar_config_complete,
> +  .config_help       = tar_config_help,
> +  .magic_config_key  = "tar",
> +  .get_ready         = tar_get_ready,
> +  .open              = tar_open,
> +  .get_size          = tar_get_size,
> +  .can_multi_conn    = tar_can_multi_conn,
> +  .can_cache         = tar_can_cache,
> +  .pread             = tar_pread,
> +  .pwrite            = tar_pwrite,
> +  .errno_is_preserved = 1,
> +};
> +
> +NBDKIT_REGISTER_PLUGIN(plugin)
> diff --git a/tests/test-dump-plugin.sh b/tests/test-dump-plugin.sh
> index 0b4c1ce1..6eb25a65 100755
> --- a/tests/test-dump-plugin.sh
> +++ b/tests/test-dump-plugin.sh
> @@ -46,7 +46,7 @@ do_test ()
>          python-valgrind | ruby-valgrind | tcl-valgrind)
>              echo "$0: skipping $1$vg because this language
doesn't support valgrind"
>              ;;
> -        example4* | tar*)
> +        example4*)
>              # These tests are written in Perl so we have to check that
>              # the Perl plugin was compiled.
>              if nbdkit perl --version; then run_test $1; fi
> diff --git a/tests/test-help-plugin.sh b/tests/test-help-plugin.sh
> index 7dc26ece..f0dfa7df 100755
> --- a/tests/test-help-plugin.sh
> +++ b/tests/test-help-plugin.sh
> @@ -46,7 +46,7 @@ do_test ()
>          python-valgrind | ruby-valgrind | tcl-valgrind)
>              echo "$0: skipping $1$vg because this language
doesn't support valgrind"
>              ;;
> -        example4* | tar*)
> +        example4*)
>              # These tests are written in Perl so we have to check that
>              # the Perl plugin was compiled.
>              if nbdkit perl --version; then run_test $1; fi
> diff --git a/tests/test-tar-info.sh b/tests/test-tar-info.sh
> new file mode 100755
> index 00000000..efaa8ec8
> --- /dev/null
> +++ b/tests/test-tar-info.sh
> @@ -0,0 +1,67 @@
> +#!/usr/bin/env bash
> +# nbdkit
> +# 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
> +# 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.
> +
> +# Test that qemu-img info works on a qcow2 file in a tar file.
> +
> +source ./functions.sh
> +set -e
> +set -x
> +
> +requires test -f disk
> +requires guestfish --version
> +requires tar --version
> +requires qemu-img --version
> +requires qemu-img info --output=json /dev/null
> +requires jq --version
> +requires stat --version
> +
> +disk=tar-info-disk.qcow2
> +out=tar-info.out
> +tar=tar-info.tar
> +files="$disk $out $tar"
> +rm -f $files
> +cleanup_fn rm -f $files
> +
> +# Create a tar file containing a known qcow2 file.
> +qemu-img convert -f raw disk -O qcow2 $disk
> +tar cf $tar $disk
> +
> +# Run nbdkit.
> +nbdkit -U - tar $tar file=$disk --run 'qemu-img info --output=json
$nbd' > $out
> +cat $out
> +
> +# Check various fields in the input.
> +# Virtual size must be the same as the size of the original raw disk.
> +test "$( jq -r -c '.["virtual-size"]' $out )"
-eq "$( stat -c %s disk )"
> +
> +# Format must be qcow2.
> +test "$( jq -r -c '.["format"]' $out )" =
"qcow2"
> diff --git a/tests/test-tar.sh b/tests/test-tar.sh
> index c6d726c4..3164b826 100755
> --- a/tests/test-tar.sh
> +++ b/tests/test-tar.sh
> @@ -38,23 +38,27 @@ requires test -f disk
>  requires guestfish --version
>  requires tar --version
>
> -# The tar plugin requires some Perl modules, this checks if they are
> -# installed.
> -requires perl -MCwd -e 1
> -requires perl -MIO::File -e 1
> -
>  sock=`mktemp -u`
>  files="tar.pid tar.tar $sock"
>  rm -f $files
>  cleanup_fn rm -f $files
>
> -# Create a tar file containing the disk image.
> -tar cf tar.tar disk
> +# Create a tar file containing the disk image plus some other random
> +# files that hopefully will be ignored.
> +tar cf tar.tar test-tar.sh Makefile disk Makefile.am
> +tar tvvf tar.tar
>
>  # Run nbdkit.
>  start_nbdkit -P tar.pid -U $sock tar tar=tar.tar file=disk
>
> -# Now see if we can open the disk from the tar file.
> -guestfish -x --ro --format=raw -a "nbd://?socket=$sock" -m
/dev/sda1 <<EOF
> +# Now see if we can open, read and write the disk from the tar file.
> +guestfish -x --format=raw -a "nbd://?socket=$sock" -m /dev/sda1
<<EOF
>    cat /hello.txt
> +
> +  # Write a new file.
> +  write /test.txt "hello"
> +  cat /test.txt
>  EOF
> +
> +# Check that the tar file isn't corrupt.
> +tar tvvf tar.tar
> diff --git a/tests/test-version-plugin.sh b/tests/test-version-plugin.sh
> index 7d2ab072..c397afc2 100755
> --- a/tests/test-version-plugin.sh
> +++ b/tests/test-version-plugin.sh
> @@ -46,7 +46,7 @@ do_test ()
>          python-valgrind | ruby-valgrind | tcl-valgrind)
>              echo "$0: skipping $1$vg because this language
doesn't support valgrind"
>              ;;
> -        example4* | tar*)
> +        example4*)
>              # These tests are written in Perl so we have to check that
>              # the Perl plugin was compiled.
>              if nbdkit perl --version; then run_test $1; fi
> diff --git a/wrapper.c b/wrapper.c
> index b1e2ce2f..c27afae0 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -85,7 +85,7 @@ static size_t len;
>  static bool
>  is_perl_plugin (const char *name)
>  {
> -  return strcmp (name, "example4") == 0 || strcmp (name,
"tar") == 0;
> +  return strcmp (name, "example4") == 0;
>  }
>
>  static void
> diff --git a/.gitignore b/.gitignore
> index f02d19dc..48a5302e 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -79,7 +79,6 @@ plugins/*/*.3
>  /plugins/ocaml/nbdkit-ocamlexample-plugin.so
>  /plugins/rust/Cargo.lock
>  /plugins/rust/target
> -/plugins/tar/nbdkit-tar-plugin
>  /plugins/tmpdisk/default-command.c
>  /podwrapper.pl
>  /server/local/nbdkit.pc
> diff --git a/README b/README
> index 4f584828..7733761e 100644
> --- a/README
> +++ b/README
> @@ -124,13 +124,13 @@ For the bittorrent plugin:
>
>   - libtorrent-rasterbar (https://www.libtorrent.org)
>
> -For the Perl, example4 and tar plugins:
> +For the Perl and example4 plugins:
>
>   - perl interpreter
>
>   - perl development libraries
>
> - - perl modules ExtUtils::Embed, IO::File and Cwd
> + - perl modules ExtUtils::Embed
>
>  For the Python plugin:
>
> --
> 2.25.0
>

On 6/28/20 8:02 AM, Richard W.M. Jones wrote:
> ---
>   plugins/tar/{tar.pl => nbdkit-tar-plugin.pod} | 145 ++-------
>   configure.ac                                  |   2 -
>   plugins/tar/Makefile.am                       |  41 +--
>   tests/Makefile.am                             |  14 +-
>   plugins/tar/tar.c                             | 286 ++++++++++++++++++
>   tests/test-dump-plugin.sh                     |   2 +-
>   tests/test-help-plugin.sh                     |   2 +-
>   tests/test-tar-info.sh                        |  67 ++++
>   tests/test-tar.sh                             |  22 +-
>   tests/test-version-plugin.sh                  |   2 +-
>   wrapper.c                                     |   2 +-
>   .gitignore                                    |   1 -
>   README                                        |   4 +-
>   13 files changed, 427 insertions(+), 163 deletions(-)
> 
>   
> - nbdkit tar tar=FILENAME.tar file=PATH_INSIDE_TAR
> + nbdkit tar [tar=]FILENAME.tar file=PATH_INSIDE_TAR
I'm considering a followup patch to allow file=exportname as a magic 
filename, similarly to how we did in the ext2 filter.
> +=item B<file=>PATH_INSIDE_TAR
> +
> +The path of the file inside the tarball to serve.  This parameter is
> +required.  It must exactly match the name stored in the tarball, so
> +use S<C<tar tf filename.tar>>
However, I'm a bit worried about how it would work if the tarfile itself 
includes a file named 'exportname' in the top directory of the tarfile. 
A quick test confirms my worry:

$ cd /tmp
$ touch exportname
$ tar cf f.tar exportname
$ tar tf f.tar
exportname
$ LANG=C tar --no-auto-compress -tRvf f.tar exportname
block 0: -rw-rw-r-- eblake/eblake     0 2020-07-06 13:37 exportname
block 1: ** Block of NULs **
$ LANG=C tar --no-auto-compress -tRvf f.tar ./exportname
block 1: ** Block of NULs **
tar: ./exportname: Not found in archive
tar: Exiting with failure status due to previous errors

so if we like the idea, we'd have to allow the user to specify 
mutually-exclusive config parameters: either file= to something within 
the file, or exportname=on to allow the client to choose, where we then 
validate that exactly one of those two options is configured.
> +++ b/plugins/tar/Makefile.am
> @@ -1,5 +1,5 @@
>   # nbdkit
> -# Copyright (C) 2013-2020 Red Hat Inc.
> +# Copyright (C) 2018-2020 Red Hat Inc.
Interesting change in dates.
> +++ b/plugins/tar/tar.c
> @@ -0,0 +1,286 @@
> +/* nbdkit
> + * Copyright (C) 2018-2020 Red Hat Inc.
> + *
> +static int
> +tar_config_complete (void)
> +{
> +  if (tarfile == NULL || file == NULL) {
> +    nbdkit_error ("you must supply the tar=<TARFILE> and
file=<FILENAME> "
> +                  "parameters");
> +    return -1;
> +  }
> +
> +  return 0;
> +}
> +
> +#define tar_config_help \
> +  "[tar=]<TARBALL>     (required) The name of the tar
file.\n" \
> +  "file=<FILENAME>                The path inside the tar file
to server."
Should '(required)' be listed on both lines?  (Not necessarily on the 
second, if we go with the exportname=on option)
> +
> +static int
> +tar_get_ready (void)
> +{
> +  FILE *fp;
> +  CLEANUP_FREE char *cmd = NULL;
> +  size_t len = 0;
> +  bool scanned_ok;
> +  char s[256];
> +
> +  /* Construct the tar command to examine the tar file. */
> +  fp = open_memstream (&cmd, &len);
> +  if (fp == NULL) {
> +    nbdkit_error ("open_memstream: %m");
> +    return -1;
> +  }
> +  fprintf (fp, "LANG=C tar --no-auto-compress -tRvf ");
Use of --no-auto-compress is specific to GNU tar, do we care?

Should we allow a 'tar=/path/to/tar' parameter during .config to allow a
user to point to an alternative tar?
> +  shell_quote (tarfile, fp);
> +  fputc (' ', fp);
> +  shell_quote (file, fp);
> +  if (fclose (fp) == EOF) {
> +    nbdkit_error ("memstream failed: %m");
> +    return -1;
> +  }
> +
> +  /* Run the command and read the first line of the output. */
> +  nbdkit_debug ("%s", cmd);
> +  fp = popen (cmd, "r");
> +  if (fp == NULL) {
> +    nbdkit_error ("tar: %m");
> +    return -1;
> +  }
> +  scanned_ok = fscanf (fp, "block %" SCNu64 ": %*s %*s
%" SCNu64,
> +                       &offset, &size) == 2;
fscanf() parsing an integer is subject to undefined behavior on 
overflow.  (Yes, I know it is a pre-existing and prevalent issue...)
> +  /* We have to now read and discard the rest of the output until EOF. */
> +  while (fread (s, sizeof s, 1, fp) > 0)
> +    ;
> +  if (pclose (fp) != 0) {
> +    nbdkit_error ("tar subcommand failed, "
> +                  "check that the file really exists in the
tarball");
> +    return -1;
> +  }
If we add exportname support, we'll have to defer checking for file 
existence until during .open.
> +
> +  if (!scanned_ok) {
> +    nbdkit_error ("unexpected output from the tar subcommand");
> +    return -1;
> +  }
> +
> +  /* Adjust the offset: Add 1 for the tar header, then multiply by the
> +   * block size.
> +   */
> +  offset = (offset+1) * 512;
Is 512 always correct, or can a tar created with -b > 1 cause issues?
> +
> +  nbdkit_debug ("tar: offset %" PRIu64 ", size %"
PRIu64, offset, size);
> +
> +  /* Check it looks sensible.  XXX We ought to check it doesn't exceed
> +   * the size of the tar file.
> +   */
> +  if (offset >= INT64_MAX || size >= INT64_MAX) {
> +    nbdkit_error ("internal error: calculated offset and size are
wrong");
> +    return -1;
> +  }
> +
> +  return 0;
> +}
> +
> +struct handle {
> +  int fd;
> +};
> +
> +static void *
> +tar_open (int readonly)
> +{
> +  struct handle *h;
> +
> +  assert (offset > 0);     /* Cannot be zero because of tar header. */
> +
> +  h = calloc (1, sizeof *h);
> +  if (h == NULL) {
> +    nbdkit_error ("calloc: %m");
> +    return NULL;
> +  }
> +  h->fd = open (tarfile, (readonly ? O_RDONLY : O_RDWR) | O_CLOEXEC);
Should we really be opening a different fd per client, or can we get 
away with opening only one fd during .get_ready and sharing it among all 
clients?

The unguarded use of O_CLOEXEC makes sense, but may cause compilation 
issues on Haiku.
> +  if (h->fd == -1) {
> +    nbdkit_error ("%s: %m", tarfile);
> +    free (h);
> +    return NULL;
> +  }
> +
> +  return h;
> +}
> +
> +#define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL
> +
> +/* Get the file size. */
> +static int64_t
> +tar_get_size (void *handle)
> +{
> +  return size;
> +}
> +
> +/* Serves the same data over multiple connections. */
> +static int
> +tar_can_multi_conn (void *handle)
> +{
> +  return 1;
> +}
Needs a tweak if we add exportname=on
> +
> +static int
> +tar_can_cache (void *handle)
> +{
> +  /* Let nbdkit call pread to populate the file system cache. */
> +  return NBDKIT_CACHE_EMULATE;
> +}
> +
> +/* Read data from the file. */
> +static int
> +tar_pread (void *handle, void *buf, uint32_t count, uint64_t offs)
> +{
> +  struct handle *h = handle;
> +
> +  offs += offset;
> +
> +  while (count > 0) {
> +    ssize_t r = pread (h->fd, buf, count, offs);
> +    if (r == -1) {
> +      nbdkit_error ("pread: %m");
> +      return -1;
> +    }
> +    if (r == 0) {
> +      nbdkit_error ("pread: unexpected end of file");
> +      return -1;
> +    }
> +    buf += r;
> +    count -= r;
> +    offs += r;
> +  }
> +
> +  return 0;
> +}
> +
> +/* Write data to the file. */
> +static int
> +tar_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offs)
> +{
> +  struct handle *h = handle;
> +
> +  offs += offset;
> +
> +  while (count > 0) {
> +    ssize_t r = pwrite (h->fd, buf, count, offs);
Does this always work even when the tar file was created with sparse 
file support?  For that matter, if the tar file was created with sparse 
file contents, are we even guaranteed that a contiguous offset of the 
tar file is blindly usable as the data to serve?
> +    if (r == -1) {
> +      nbdkit_error ("pwrite: %m");
> +      return -1;
> +    }
> +    buf += r;
> +    count -= r;
> +    offs += r;
> +  }
> +
> +  return 0;
> +}
> +
> +static struct nbdkit_plugin plugin = {
> +  .name              = "tar",
> +  .longname          = "nbdkit tar plugin",
> +  .version           = PACKAGE_VERSION,
> +  .unload            = tar_unload,
> +  .config            = tar_config,
> +  .config_complete   = tar_config_complete,
> +  .config_help       = tar_config_help,
> +  .magic_config_key  = "tar",
> +  .get_ready         = tar_get_ready,
> +  .open              = tar_open,
> +  .get_size          = tar_get_size,
> +  .can_multi_conn    = tar_can_multi_conn,
> +  .can_cache         = tar_can_cache,
> +  .pread             = tar_pread,
> +  .pwrite            = tar_pwrite,
> +  .errno_is_preserved = 1,
No .extents makes sense if the tar file does not record sparseness, but 
may be needed if we support sparse tar files.

$ truncate --size=1M large
$ echo 'hi' >> large
$ tar cSf f.tar large
$ ls -l large f.tar
-rw-rw-r--. 1 eblake eblake   10240 Jul  6 13:57 f.tar
-rw-rw-r--. 1 eblake eblake 1048579 Jul  6 13:48 large
$ LANG=C tar --no-auto-compress -tRvf f.tar large
block 0: -rw-rw-r-- eblake/eblake 1048579 2020-07-06 13:48 large
block 2: ** Block of NULs **

Yep, we need to special-case sparse tar files :(

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org