Libguestfs - Apr 2020 - Re: [PATCH virt-v2v] v2v: Allow temporary directory to be set on a global basis.

On Thu, Apr 02, 2020 at 05:51:32PM +0200, Pino Toscano
wrote:
> On Thursday, 2 April 2020 17:30:39 CEST Richard W.M. Jones wrote:
> > On Thu, Apr 02, 2020 at 03:21:14PM +0200, Pino Toscano wrote:
> > > On Thursday, 2 April 2020 14:49:18 CEST Richard W.M. Jones wrote:
> > > > Previously we placed large files in g#get_cachedir ()
(usually
> > > > /var/tmp).  However the problem is this ties the libguestfs
appliance
> > > > and the virt-v2v overlay files to the same location.
> > > > 
> > > > When virt-v2v is run in a container, or any other situation
where
> > > > local storage is limited, it's helpful to be able to put
the overlay
> > > > files on an externally mounted PVC, which might be using NFS
and
> > > > shared between containers.  But putting the libguestfs
appliance on
> > > > NFS in a shared location is certainly not recommended.
> > > > 
> > > > This allows the two locations to be set separately:
> > > > 
> > > >   VIRT_V2V_TMPDIR - location of large temporary files, can
use NFS
> > > >                     and may be shared
> > > > 
> > > >   LIBGUESTFS_CACHEDIR - location of libguestfs appliance
> > > > 
> > > > Another motivation for this patch is to allow more reliable
cleanup of
> > > > temporary files by an external process, as described in the
updated
> > > > documentation.
> > > > ---
> > > 
> > > I do not understand the motivation behind this, which adds yet
another
> > > location with temporary files in addition to:
> > > - LIBGUESTFS_TMPDIR - $TMPDIR by default (which itself is /tmp by
> > >   default)
> > > - LIBGUESTFS_CACHEDIR - /var/tmp by default (with a .guestfs-$UID
> > >   subdirectory for the appliance)
> > > 
> > > Before this patch, almost all the temporary files are stored
directly
> > > or in subdirectories of $TMPDIR, except big files such as
overlays and
> > > OVA extracted content that are in CACHEDIR. With the proposed
changes,
> > > _all_ the temporary files will be in CACHEDIR, so there are the
> > > following problems:
> > > - this directory will be cluttered with a lot more files than
before
> > > - if it is shared, then other places where it is mounted will see
the
> > >   same files
> > > - if it is shared, then creating temporary files will possibly
mean
> > >   doing network I/O
> > > - if virt-v2v exits uncleantly, there will be a lot more files to
> > >   cleanup than now
> > > - even without being shared, /var/tmp is persistent unlike /tmp
(which
> > >   can be tmpfs-backed on some distros/setups), meaning old
temporary
> > >   files will linger way more
> > 
> > How about if we confine the change to just large files (ie. ones
> > which are currently placed in cachedir)?
> 
> This is already the case, isn't it?
> 
> > However the way that virt-v2v works at the moment means you cannot put
> > the large files (especially v2vovl*.qcow2) in a different place from
> > the libguestfs appliance.  This means that if you have only
"some"
> > space in /var/tmp -- enough for the appliance, but not enough for the
> > potentially much larger space required by v2vovl*.qcow2 with multiple
> > copies of virt-v2v running -- then you cannot separate the overlays to
> > another directory.  This isn't just a problem for containers.
> 
> /var/tmp is a temporary directory, hence it ought to have enough space
> for big temporary files. This is nothing special for libguestfs or
> virt-v2v.
It is not valid to assume that /var/tmp is arbitrarily big enough
for any files v2v might want to create. AFAIK, there's no real
rule about how large it is expected to be. Linux distros typically
don't do fine grained partitioning by default, so /var/tmp is often
the same filesystem as /, but it is not unusual for admins to have
/ relatively small, and have a separate data partition somewhere
which has the vast majority of storage space available to the host.

In the container world the / is indeed generally of a fairly
limited size, as that's an unpacked container image filesystem
usually on the local host storage.  Apps needing large storage
capacities will usually be given an explicit volume they can
use for this purpose.  NB, the provided volume may be persistent
across container boots, or it may be freshly formatted on every
boot. Whomever configures the container decides this based on
the usage reuqirements of whatever app is to be run in the
container.

> I do not see what makes v2vovl*.qcow2 files so special in this regard,
> requiring them to be handled specially than other big temporary files
> already stored by libguestfs/virt-v2v. Examples:
> - the appliance
> - the temporary directory for qemu when using the libvirt backend
> - the extracted content of an OVA, in case we cannot read it directly
> - the disks when exporting to glance (-o glance)
Out of those things, I think the appliance / QEMU temp dir are in
a different league to the disk images. They're going to be small
enough that I it wouldn't tax the storage capacity of /var/tmp.

Anything related to VM disk images is liable to be an order of
magnitude bigger, easily in the  100's of GB league.

With this POV, I would put  OVA contents, glance exports, and v2v
disk images in the same category, and would expect to be able to
explicitly control  where they are placed, independantly from anything
that goes in /var/tmp.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Monday, 6 April 2020 19:45:44 CEST Daniel P. Berrangé
wrote:
> On Thu, Apr 02, 2020 at 05:51:32PM +0200, Pino Toscano wrote:
> > On Thursday, 2 April 2020 17:30:39 CEST Richard W.M. Jones wrote:
> > > On Thu, Apr 02, 2020 at 03:21:14PM +0200, Pino Toscano wrote:
> > > > On Thursday, 2 April 2020 14:49:18 CEST Richard W.M. Jones
wrote:
> > > > > Previously we placed large files in g#get_cachedir ()
(usually
> > > > > /var/tmp).  However the problem is this ties the
libguestfs appliance
> > > > > and the virt-v2v overlay files to the same location.
> > > > > 
> > > > > When virt-v2v is run in a container, or any other
situation where
> > > > > local storage is limited, it's helpful to be able
to put the overlay
> > > > > files on an externally mounted PVC, which might be
using NFS and
> > > > > shared between containers.  But putting the libguestfs
appliance on
> > > > > NFS in a shared location is certainly not recommended.
> > > > > 
> > > > > This allows the two locations to be set separately:
> > > > > 
> > > > >   VIRT_V2V_TMPDIR - location of large temporary files,
can use NFS
> > > > >                     and may be shared
> > > > > 
> > > > >   LIBGUESTFS_CACHEDIR - location of libguestfs
appliance
> > > > > 
> > > > > Another motivation for this patch is to allow more
reliable cleanup of
> > > > > temporary files by an external process, as described in
the updated
> > > > > documentation.
> > > > > ---
> > > > 
> > > > I do not understand the motivation behind this, which adds
yet another
> > > > location with temporary files in addition to:
> > > > - LIBGUESTFS_TMPDIR - $TMPDIR by default (which itself is
/tmp by
> > > >   default)
> > > > - LIBGUESTFS_CACHEDIR - /var/tmp by default (with a
.guestfs-$UID
> > > >   subdirectory for the appliance)
> > > > 
> > > > Before this patch, almost all the temporary files are stored
directly
> > > > or in subdirectories of $TMPDIR, except big files such as
overlays and
> > > > OVA extracted content that are in CACHEDIR. With the
proposed changes,
> > > > _all_ the temporary files will be in CACHEDIR, so there are
the
> > > > following problems:
> > > > - this directory will be cluttered with a lot more files
than before
> > > > - if it is shared, then other places where it is mounted
will see the
> > > >   same files
> > > > - if it is shared, then creating temporary files will
possibly mean
> > > >   doing network I/O
> > > > - if virt-v2v exits uncleantly, there will be a lot more
files to
> > > >   cleanup than now
> > > > - even without being shared, /var/tmp is persistent unlike
/tmp (which
> > > >   can be tmpfs-backed on some distros/setups), meaning old
temporary
> > > >   files will linger way more
> > > 
> > > How about if we confine the change to just large files (ie. ones
> > > which are currently placed in cachedir)?
> > 
> > This is already the case, isn't it?
> > 
> > > However the way that virt-v2v works at the moment means you
cannot put
> > > the large files (especially v2vovl*.qcow2) in a different place
from
> > > the libguestfs appliance.  This means that if you have only
"some"
> > > space in /var/tmp -- enough for the appliance, but not enough for
the
> > > potentially much larger space required by v2vovl*.qcow2 with
multiple
> > > copies of virt-v2v running -- then you cannot separate the
overlays to
> > > another directory.  This isn't just a problem for containers.
> > 
> > /var/tmp is a temporary directory, hence it ought to have enough space
> > for big temporary files. This is nothing special for libguestfs or
> > virt-v2v.
> 
> It is not valid to assume that /var/tmp is arbitrarily big enough
> for any files v2v might want to create. AFAIK, there's no real
> rule about how large it is expected to be. Linux distros typically
> don't do fine grained partitioning by default, so /var/tmp is often
> the same filesystem as /, but it is not unusual for admins to have
> / relatively small, and have a separate data partition somewhere
> which has the vast majority of storage space available to the host.
> 
> In the container world the / is indeed generally of a fairly
> limited size, as that's an unpacked container image filesystem
> usually on the local host storage.  Apps needing large storage
> capacities will usually be given an explicit volume they can
> use for this purpose.  NB, the provided volume may be persistent
> across container boots, or it may be freshly formatted on every
> boot. Whomever configures the container decides this based on
> the usage reuqirements of whatever app is to be run in the
> container.
Sure, I agree with that.

The important thing is still that that you need to have space for the
temporary files somewhere: be it /var/tmp, /mnt/scratch, whatever.
Because of this, and the fact that usually containers are created
fresh, the cache of the supermin appliance starts to make little sense,
and then a very simple solution is to point libguestfs to that extra
space:

  $ dir=$(mktemp --tmpdir -d /path/to/big/temporary/space/libguestfs.XXXXXX)
  $ export LIBGUESTGS_CACHEDIR=$dir
  $ export TMPDIR=$dir  # optionally
  $ libguestfs-test-tool
  [etc]
  $ rm -rf $dir

Easy to use, already doable, solves all the issues.

This whole problem started from a QE report on leftover files after
failed migrations: bz#1820282. What this report doesn't say, however,
is that beside the mentioned files that virt-v2v creates, there are
also leftover files that libguestfs itself creates. These files are
usually downloaded from the guest for the inspection, and generally not
that big compared to e.g. the overlays that virt-v2v creates.
Nonetheless, an abrupt exit of virt-v2v will leave those in place, and
they will still slowly fill up the space on /var/tmp (or whatever is
the location of $LIBGUESTFS_CACHEDIR).

Sure, creating a special directory for virt-v2v solves /some/ of the
issues, and most probably the ones that concern most from a disk space
POV. However, since we "uncovered" the issue and started to get our
hands on it, I don't think it would be ideal to create an ad-hoc
solution that solves just some.

-- 
Pino Toscano

On Tue, Apr 07, 2020 at 01:25:02PM +0200, Pino Toscano
wrote:
> The important thing is still that that you need to have space for the
> temporary files somewhere: be it /var/tmp, /mnt/scratch, whatever.
> Because of this, and the fact that usually containers are created
> fresh, the cache of the supermin appliance starts to make little sense,
> and then a very simple solution is to point libguestfs to that extra
> space:
> 
>   $ dir=$(mktemp --tmpdir -d
/path/to/big/temporary/space/libguestfs.XXXXXX)
>   $ export LIBGUESTGS_CACHEDIR=$dir
>   $ export TMPDIR=$dir  # optionally
>   $ libguestfs-test-tool
>   [etc]
>   $ rm -rf $dir
> 
> Easy to use, already doable, solves all the issues.
So AIUI there are a few problems with this (although I'm still
investigating and trying to make a local reproducer):

 - The external large space may be on NFS, with the usual problems
   there like root_squash, no SELinux labels, slowness.  This means
   it's not suitable for the appliance, but might nevertheless be
   suitable for overlay files.

 - The external large space may be shared with other containers, and
   I'm not convinced that our locking in supermin will be safe if
   multiple parallel instances start up at the same time.  We
   certainly never tested it, and don't currently advise it.

One may well say (and I might even agree): don't do that, or change
the NFS configuration, or don't use NFS.  But we may have to deal with
either a Kubernetes configuration as it exists already, or may find
that the tenant != Kubernetes admin.

Another issue, incidental to this but related, is that we cannot use
the fixed appliance because docker/podman have broken support for
sparse files.  (It's possible to configure libguestfs to use qcow2 for
fixed appliances, but we don't do this now, we never really tested it,
and it both greatly slows down and adds more dependencies to the
supermin build step.)
> This whole problem started from a QE report on leftover files after
> failed migrations: bz#1820282.
(I should note also there are two bugs which I personally think we can
solve with the same fix, but they are completely different bugs.)
> What this report doesn't say, however,
> is that beside the mentioned files that virt-v2v creates, there are
> also leftover files that libguestfs itself creates. These files are
> usually downloaded from the guest for the inspection, and generally not
> that big compared to e.g. the overlays that virt-v2v creates.
> Nonetheless, an abrupt exit of virt-v2v will leave those in place, and
> they will still slowly fill up the space on /var/tmp (or whatever is
> the location of $LIBGUESTFS_CACHEDIR).
I guess that small files being left around aren't really a problem.
The problem they have is large files being left around, and I can
understand why that would be an issue and not the small files.
> Sure, creating a special directory for virt-v2v solves /some/ of the
> issues, and most probably the ones that concern most from a disk space
> POV. However, since we "uncovered" the issue and started to get
our
> hands on it, I don't think it would be ideal to create an ad-hoc
> solution that solves just some.
Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v