[PATCH nbdkit 0/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 Sun, Jun 28, 2020 at 04:46:59PM +0300, Nir Soffer wrote:
On Sun, Jun 28, 2020 at 4:03 PM Richard W.M. Jones <rjones@redhat.com> wrote: ...
+ fprintf (fp, "LANG=C tar --no-auto-compress -tRvf ");
Using -R is nice, but is block size documented?
Also --block-number would be nicer.
Yes the block size is always 512 bytes: https://www.gnu.org/software/tar/manual/html_node/Blocking.html By the way, FreeBSD tar will not work (and presumably other non-GNU tar), so I guess we should probably try to probe tar and come up with a nicer error message: https://www.freebsd.org/cgi/man.cgi?tar(1)
+ shell_quote (tarfile, fp);
This is questionable. Why use string and quote the string instead of using argv directly?
Convenience basically. The alternative is to fork and exec the tar command in a subprocess, while reading from a pipe that we have created, which is a pain to write correctly.
+/* 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?
Yes and no. A few months ago I actually had a proposal to unify all the "file-like" plugins. I believe the latest posted version was this one: https://www.redhat.com/archives/libguestfs/2020-April/msg00083.html but I have some updates in a branch here. Anyway when I was looking at this rewrite I considered resurrecting this work and building on top of it. However it doesn't work directly because:
+{ + struct handle *h = handle; + + offs += offset;
you have to add an offset to each request. Of course we could make a generic file ops which supported offsets (and also a non-FUA extension to deal with the tmpdisk plugin), but it all gets a bit complicated and not as generic as it initially looked. For completeness here are the other ways I investigated: * Make generic read, write, zero, trim, extents operations on local files. The file-like plugins would consume those directly, after doing any adjustments eg for offset. Unfortunately the operations that you have to write are not very "pure" and depend on a bunch of state across calls and you end up not having much common code: https://github.com/libguestfs/nbdkit/blob/f6d4365364f2c90dde0166ae4355f74f28... * Have the new tar plugin re-exec nbdkit and run the ordinary file plugin + offset filter. After experience with the VDDK plugin which does this through necessity, I don't want to go there right now. See this file to understand the kind of complexity this introduces: https://github.com/libguestfs/nbdkit/blob/master/plugins/vddk/reexec.c * Instead of writing a tar plugin, write a wrapper script which hands off to file plugin + offset filter. From the user's point of view the script would work a lot differently from other plugins. So no good ideas so far. 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 6/28/20 4:35 PM, Richard W.M. Jones wrote:
For completeness here are the other ways I investigated:
* Make generic read, write, zero, trim, extents operations on local files. The file-like plugins would consume those directly, after doing any adjustments eg for offset. Unfortunately the operations that you have to write are not very "pure" and depend on a bunch of state across calls and you end up not having much common code: https://github.com/libguestfs/nbdkit/blob/f6d4365364f2c90dde0166ae4355f74f28...
* Have the new tar plugin re-exec nbdkit and run the ordinary file plugin + offset filter. After experience with the VDDK plugin which does this through necessity, I don't want to go there right now. See this file to understand the kind of complexity this introduces: https://github.com/libguestfs/nbdkit/blob/master/plugins/vddk/reexec.c
* Instead of writing a tar plugin, write a wrapper script which hands off to file plugin + offset filter. From the user's point of view the script would work a lot differently from other plugins.
So no good ideas so far.
How hard is it to write a tar filter instead of a tar plugin (similar to how we moved ext2 from plugin to filter)? - During .get_ready, we have to find some way to read the underlying plugin to feed a pipeline to tar to decode what the file contains (we can't just hand the file to tar, but instead have to feed it data through stdin; but the amount of work is no different: tar really does have to read the entire image during 'tar tRvf'). - There would no longer be a tar=... parameter, rather that is the role of the plugin - We'd have to rename the file=... parameter to something that won't conflict with the most common use of having the file plugin serve the tar file - But once we've done the .get_ready scan of the entire tar file, we can then service offsets to the plugin similarly to how the offset filter works -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Mon, Jul 06, 2020 at 02:08:26PM -0500, Eric Blake wrote:
On 6/28/20 4:35 PM, Richard W.M. Jones wrote:
For completeness here are the other ways I investigated:
* Make generic read, write, zero, trim, extents operations on local files. The file-like plugins would consume those directly, after doing any adjustments eg for offset. Unfortunately the operations that you have to write are not very "pure" and depend on a bunch of state across calls and you end up not having much common code: https://github.com/libguestfs/nbdkit/blob/f6d4365364f2c90dde0166ae4355f74f28...
* Have the new tar plugin re-exec nbdkit and run the ordinary file plugin + offset filter. After experience with the VDDK plugin which does this through necessity, I don't want to go there right now. See this file to understand the kind of complexity this introduces: https://github.com/libguestfs/nbdkit/blob/master/plugins/vddk/reexec.c
* Instead of writing a tar plugin, write a wrapper script which hands off to file plugin + offset filter. From the user's point of view the script would work a lot differently from other plugins.
So no good ideas so far.
How hard is it to write a tar filter instead of a tar plugin (similar to how we moved ext2 from plugin to filter)?
- During .get_ready, we have to find some way to read the underlying plugin to feed a pipeline to tar to decode what the file contains (we can't just hand the file to tar, but instead have to feed it data through stdin; but the amount of work is no different: tar really does have to read the entire image during 'tar tRvf'). - There would no longer be a tar=... parameter, rather that is the role of the plugin - We'd have to rename the file=... parameter to something that won't conflict with the most common use of having the file plugin serve the tar file - But once we've done the .get_ready scan of the entire tar file, we can then service offsets to the plugin similarly to how the offset filter works
Yes this sounds plausible. 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
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
On Mon, Jul 06, 2020 at 01:58:35PM -0500, Eric Blake wrote:
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.
Right, I think that would be the way to do it, to make it fully general, and to avoid any possibility that someone turns on the exportname feature by accident (which could be a security issue).
+++ 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.
I think that was a copy/paste error effectively. I copied the Makefile from an existing C plugin, rather than attempting to rewrite the old Perl-based one.
+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)
Yes, that's a bug (until exportname is implemented) - will fix soon.
+ +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?
I copied that from the existing Perl plugin, but note that -R is also not supported by non-GNU tar.
Should we allow a 'tar=/path/to/tar' parameter during .config to allow a user to point to an alternative tar?
Yes, this is a potential enhancement, especially where GNU tar might be called "gtar" (as it is on FreeBSD).
+ 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?
Nir actually raised this issue before and I convinced myself at the time that the block size is always 512 bytes. I didn't notice the -b option til now however, but I don't know if it affects this block size of not, but I'd say it's vanishingly unlikely that there exist any OVA files in the wild that use this feature (and if they do, even less likely that anything can read them).
+ 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 original code actually opened the file once, but it had the disadvantage that you had to always open the file for writes (because you don't know if the client will request read-only or not). But that breaks if the tar file itself cannot be opened for writing. So I believe it is necessary to open it per handle, unless you can think of another way to get around that..
The unguarded use of O_CLOEXEC makes sense, but may cause compilation issues on Haiku.
I'll check this.
+/* 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?
We've used the same technique on thousands of OVA files without hitting any problem so far, so I guess that ...
$ 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 :(
.. OVAs don't use this feature. I wonder if it's a GNU feature? 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/
On 7/6/20 3:44 PM, Richard W.M. Jones wrote:
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?
We've used the same technique on thousands of OVA files without hitting any problem so far, so I guess that ...
$ 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 :(
.. OVAs don't use this feature. I wonder if it's a GNU feature?
-S exists in both gtar and in star (Joerg Schilly's tar [1]; Joerg often complains on the GNU tar mailing list that his 'star' is more powerful than GNU tar), but sparse support is indeed missing in other tar implementations (technically, POSIX only specifies 'pax', not 'tar' [2], although the underlying data format is supposed to be compatible enough, and pax lacks -S...). [1] https://docs.oracle.com/cd/E71197_01/SAMCR/star.1m.htm [2] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/pax.html -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
participants (3)
-
Eric Blake -
Nir Soffer -
Richard W.M. Jones