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

[Richard W.M. Jones]

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 at 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/f6d4365364f2c90dde0166ae4355f74f28e112ff/plugins/file/file.c#L149

* 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