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

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

On Tue, Apr 07, 2020 at 01:18:47PM +0100, Richard W.M. Jones
wrote:
> 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.)
I feel like we ought to be able to solve the slow down issue with
qcow2, because libguestfs has a pretty narrow usage scenario.

Slowdown for qcow2 is related to the fact that you have the multiple
table lookups to find clusters in the qcow2 image. With bigger
cluster sizes the overhead is much lower, but at the cost of harming
write performance. Since the appliance is read-only this downside
won't apply.

If we create the appliance qcow2 with a large cluster size (1MB)
then there's very small L1/L2 tables, and we can set the cache size
large enough that the lookup is always in memory. There's possibly
other things we can do too if we get guidance from QEMU devs. I'd
really hope (expect) that for a read-only qcow2, we can get it to
match raw in terms of performance.

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 Tuesday, 7 April 2020 14:18:47 CEST Richard W.M. Jones
wrote:
> 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.
If that is the only big storage space attached to a container, I do
not see any alternative than use it, with all the caveats associated.

Also, if we take as possible scenario the situation where /var/tmp is
not that big, then we need to consider that may not be big enough to
even store the cached supermin appliance (which is more than
300/350MB).
>  - 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.
That's why my suggestion above creates a specific temporary directory
for each container: even with a shared /var/tmp, there will not be any
cache stepping up on each other toes. This is something that this
separate cachedir for virt-v2v does not solve at all.
> > 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.)
I still do not understand how these changes have anything to do with
bug 1814611, which in an offline discussion we found out that has
mostly two causes:
- the way the NFS storage is mounted over the /var/tmp in the
  container, so what you create as root is not really with the UID/GID
  expected
- the fixed appliance in the container was not actually used, and thus
  a rebuilt of the the supermin appliance was attempted, failing due
  to the first issue

Can you please explain me exactly how switching the location of
temporary files (that were not mentioned in the bug at all) will fix
this situation?
> > 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.
Nobody is saying that the leftover files are not a problem. I'm saying
that also the small files are a sort of problem -- sure, less critical,
however still there and ready to show up any time, especially if the
concern is the space of /var/tmp.

-- 
Pino Toscano

On Tue, Apr 07, 2020 at 01:25:39PM +0100, Daniel P. Berrangé
wrote:
> On Tue, Apr 07, 2020 at 01:18:47PM +0100, Richard W.M. Jones wrote:
> > 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.)
> 
> I feel like we ought to be able to solve the slow down issue with
> qcow2, because libguestfs has a pretty narrow usage scenario.
I realize I was wrong here actually.

I meant the slow down in creating it not using it :-)

Currently supermin creates the image by truncating a raw file to the
required size and opening it directly with libext2fs.  If we use qcow2
we'd need the extra qemu-img convert step.

https://github.com/libguestfs/supermin/blob/master/src/ext2fs-c.c

Normally this is a critical path because it's the bit you wait for
when you're waiting for libguestfs to start up the first time.

However as we're talking about the fixed appliance, of course this is
done when the container is built, so it wouldn't affect the critical
path.

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 Tue, Apr 07, 2020 at 02:33:20PM +0200, Pino Toscano
wrote:
> On Tuesday, 7 April 2020 14:18:47 CEST Richard W.M. Jones wrote:
> > 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.
> 
> If that is the only big storage space attached to a container, I do
> not see any alternative than use it, with all the caveats associated.
I have to aggree with this. You cannot avoid situations where the
appliance is on a network drive. If there are any inherent risks the
best you can do is let user know about those (documentation?).
> 
> Also, if we take as possible scenario the situation where /var/tmp is
> not that big, then we need to consider that may not be big enough to
> even store the cached supermin appliance (which is more than
> 300/350MB).
> 
> >  - 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.
> 
> That's why my suggestion above creates a specific temporary directory
> for each container: even with a shared /var/tmp, there will not be any
> cache stepping up on each other toes. This is something that this
> separate cachedir for virt-v2v does not solve at all.
Currently we don't share the temporary volume between instances in
Kubevirt, but the assumption that this can happen is valid and should be
considered.
> 
> > > 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.)
> 
> I still do not understand how these changes have anything to do with
> bug 1814611, which in an offline discussion we found out that has
> mostly two causes:
> - the way the NFS storage is mounted over the /var/tmp in the
>   container, so what you create as root is not really with the UID/GID
>   expected
> - the fixed appliance in the container was not actually used, and thus
>   a rebuilt of the the supermin appliance was attempted, failing due
>   to the first issue
I am still not convinced this is the case. Based on the logs I shared in
private email I still believe that the fixed appliance was used
properly. You assumed that the appliance is not used because the cache
directory is being created, but as I also pointed out the cache
directory created in all situations because qemu temporary files are
stored there [1][2].
> 
> Can you please explain me exactly how switching the location of
> temporary files (that were not mentioned in the bug at all) will fix
> this situation?
For the particular BZ 1814611, if we keep the fixed appliance we can
move the cache directory to something else than /var/tmp (just for the
qemu files). We would keep /var/tmp only for overlay files. But this is
more of a hack than intended usage I guess.
> 
> > > 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.
> 
> Nobody is saying that the leftover files are not a problem. I'm saying
> that also the small files are a sort of problem -- sure, less critical,
> however still there and ready to show up any time, especially if the
> concern is the space of /var/tmp.
Isn't that the reason why things like tmpreaper exist?

    Tomas


[1]
https://github.com/libguestfs/libguestfs/blob/c2c11382bbeb4500f3388a31ffd08cfc18b0de40/lib/qemu.c#L162
[2]
https://github.com/libguestfs/libguestfs/blob/c2c11382bbeb4500f3388a31ffd08cfc18b0de40/lib/launch-direct.c#L417

On Tue, Apr 07, 2020 at 02:33:20PM +0200, Pino Toscano
wrote:
> On Tuesday, 7 April 2020 14:18:47 CEST Richard W.M. Jones wrote:
> > 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.
> 
> If that is the only big storage space attached to a container, I do
> not see any alternative than use it, with all the caveats associated.
> 
> Also, if we take as possible scenario the situation where /var/tmp is
> not that big, then we need to consider that may not be big enough to
> even store the cached supermin appliance (which is more than
> 300/350MB).
In another message in this thread, Rich indicates the appliance is
being built at the time the container image itself is built. As such
the appliance will already be present in /var/tmp when the container
starts, so there's no disk space issue for it. The issue of lack of
space only applies to files created after the container is running.


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 :|