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

2020-06-17 Thread Tomáš Golembiovský
On Tue, 16 Jun 2020 22:06:58 +0100
"Richard W.M. Jones"  wrote:

> On Tue, Jun 16, 2020 at 04:34:15PM +0200, Tomáš Golembiovský wrote:
> > On Wed, 10 Jun 2020 11:31:33 +0100
> > "Richard W.M. Jones"  wrote:
> >   
> > > I finally got access to the container.  This is how it's configured:
> > > 
> > > * / => an overlay fs.
> > > 
> > > There is sufficient space here, and there are no "funny" restrictions,
> > > to be able to create the libguestfs appliance.  I proved this by
> > > setting TMPDIR to a temporary directory under / and running
> > > libguestfs-test-tool.
> > > 
> > > There appears to be quite a lot of free space here, so in fact the
> > > v2vovl files could easily be stored here too.  (They only store the
> > > conversion delta, not the full guest images.)  
> > 
> > The thing is that nobody can guarantee that there will be enough space.
> > The underlying filesystem (not the mountpoint) is shared between all the
> > containers running on the host. This is the reason why we have a PV on
> > /var/tmp -- to make sure we have guaranteed free space.  
> 
> This must surely be a problem for all containers?  Do containers
> behave semi-randomly when the host starts to run out of space?  All
> containers must have to assume that there's some space available in
> /tmp or /var/tmp surely.

My understanding is that the container should not require significant
amount of free space for runtime. If you need a permanent data storage
or larger amount of temporary storage you need to use a volume.

> 
> If we can guarantee that each container has 1 or 2 G of free space
> (doesn't seem unreasonable?) then virt-v2v should work fine and won't
> need any NFS mounts.

NFS is just one of the methods to provision a volume. There is also a
host-local provisioner that makes sure there is enough space on the
local storage -- which seems what you are suggesting -- but I do not
recall what were the arguments against using it.


> > > * /var/tmp => an NFS mount from a PVC
> > > 
> > > This is a large (2T) external NFS mount.  
> > 
> > I assume that is the free space in the underlying filesystem. From there
> > you should be guaranteed to "own" only 2GB (or something like that).
> >   
> > > I actually started two pods
> > > to see if they got the same NFS mount point, and they do.  Also I
> > > wrote files to /var/tmp in one pod and they were visible in the other.
> > > So this seems shared.  
> > 
> > You mean you run two pods based on some YAML template or you run two
> > pods from Kubevirt web UI?  
> 
> Two from a yaml template, however ...
> 
> > If you run the pods manually you may have
> > reused the existing PV/PVC. It is the web UI that should provision you
> > new scratch space for each pod. If that is not working then that is a
> > bug in Kubevirt.  
> 
> ... the PVC name was "v2v-conversion-temp" (and not some randomly
> generated name) suggesting that either the user must enter a new name
> every time or else they're all going to get the same NFS mount.
> Can you explain a bit more about how they get different mounts?

Oh, I see. That seems wrong then and is probably a bug in Kubevirt web
UI. Do you still have access to the testing environment?

> 
> > > Also it uses root squash (so root:root is
> > > mapped to 99:99).  
> > 
> > IMHO this is the main problem that I have been telling them about from
> > the start. Thanks for confirming it. Using root squash on the mount is
> > plain wrong.  
> 
> This is definitely the main problem, and is the direct cause of the
> error you were seeing.  I'm still not very confident that our locking
> will work reliably if two virt-v2v instances in different containers
> or pods see a shared /var/tmp.

They should not. If they do then it is a bug somewhere else and not in
virt-v2v.

Tomas

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


-- 
Tomáš Golembiovský 


___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

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

2020-06-16 Thread Richard W.M. Jones
On Tue, Jun 16, 2020 at 04:34:15PM +0200, Tomáš Golembiovský wrote:
> On Wed, 10 Jun 2020 11:31:33 +0100
> "Richard W.M. Jones"  wrote:
> 
> > I finally got access to the container.  This is how it's configured:
> > 
> > * / => an overlay fs.
> > 
> > There is sufficient space here, and there are no "funny" restrictions,
> > to be able to create the libguestfs appliance.  I proved this by
> > setting TMPDIR to a temporary directory under / and running
> > libguestfs-test-tool.
> > 
> > There appears to be quite a lot of free space here, so in fact the
> > v2vovl files could easily be stored here too.  (They only store the
> > conversion delta, not the full guest images.)
> 
> The thing is that nobody can guarantee that there will be enough space.
> The underlying filesystem (not the mountpoint) is shared between all the
> containers running on the host. This is the reason why we have a PV on
> /var/tmp -- to make sure we have guaranteed free space.

This must surely be a problem for all containers?  Do containers
behave semi-randomly when the host starts to run out of space?  All
containers must have to assume that there's some space available in
/tmp or /var/tmp surely.

If we can guarantee that each container has 1 or 2 G of free space
(doesn't seem unreasonable?) then virt-v2v should work fine and won't
need any NFS mounts.

> > * /var/tmp => an NFS mount from a PVC
> > 
> > This is a large (2T) external NFS mount.
> 
> I assume that is the free space in the underlying filesystem. From there
> you should be guaranteed to "own" only 2GB (or something like that).
> 
> > I actually started two pods
> > to see if they got the same NFS mount point, and they do.  Also I
> > wrote files to /var/tmp in one pod and they were visible in the other.
> > So this seems shared.
> 
> You mean you run two pods based on some YAML template or you run two
> pods from Kubevirt web UI?

Two from a yaml template, however ...

> If you run the pods manually you may have
> reused the existing PV/PVC. It is the web UI that should provision you
> new scratch space for each pod. If that is not working then that is a
> bug in Kubevirt.

... the PVC name was "v2v-conversion-temp" (and not some randomly
generated name) suggesting that either the user must enter a new name
every time or else they're all going to get the same NFS mount.
Can you explain a bit more about how they get different mounts?

> > Also it uses root squash (so root:root is
> > mapped to 99:99).
> 
> IMHO this is the main problem that I have been telling them about from
> the start. Thanks for confirming it. Using root squash on the mount is
> plain wrong.

This is definitely the main problem, and is the direct cause of the
error you were seeing.  I'm still not very confident that our locking
will work reliably if two virt-v2v instances in different containers
or pods see a shared /var/tmp.

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/

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

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

2020-06-16 Thread Tomáš Golembiovský
On Wed, 10 Jun 2020 11:31:33 +0100
"Richard W.M. Jones"  wrote:

> I finally got access to the container.  This is how it's configured:
> 
> * / => an overlay fs.
> 
> There is sufficient space here, and there are no "funny" restrictions,
> to be able to create the libguestfs appliance.  I proved this by
> setting TMPDIR to a temporary directory under / and running
> libguestfs-test-tool.
> 
> There appears to be quite a lot of free space here, so in fact the
> v2vovl files could easily be stored here too.  (They only store the
> conversion delta, not the full guest images.)

The thing is that nobody can guarantee that there will be enough space.
The underlying filesystem (not the mountpoint) is shared between all the
containers running on the host. This is the reason why we have a PV on
/var/tmp -- to make sure we have guaranteed free space.

> 
> * /var/tmp => an NFS mount from a PVC
> 
> This is a large (2T) external NFS mount.

I assume that is the free space in the underlying filesystem. From there
you should be guaranteed to "own" only 2GB (or something like that).

> I actually started two pods
> to see if they got the same NFS mount point, and they do.  Also I
> wrote files to /var/tmp in one pod and they were visible in the other.
> So this seems shared.

You mean you run two pods based on some YAML template or you run two
pods from Kubevirt web UI? If you run the pods manually you may have
reused the existing PV/PVC. It is the web UI that should provision you
new scratch space for each pod. If that is not working then that is a
bug in Kubevirt.


> Also it uses root squash (so root:root is
> mapped to 99:99).

IMHO this is the main problem that I have been telling them about from
the start. Thanks for confirming it. Using root squash on the mount is
plain wrong.

Tomas

> For both reasons this cannot be used for the
> appliance.  If it was mounted at another location it might be used for
> the v2vovl files.
> 
> I've attached the exact mount details at the end of this email.
> 
> My conclusion is that we could do one of two things:
> 
> Either:
> 
> (1) Easiest solution is simply not mount anything under /var/tmp, and
> let it be local storage.  Assuming all these containers are getting ~40G
> of local storage, that's more than enough for virt-v2v to run and
> store the appliance and overlays.  Everything should just work once
> you remove that /var/tmp mountpoint and leave it as local storage.
> 
> ie these lines are removed:
> - mountPath: /var/tmp
>   name: v2v-conversion-temp
> 
> Or:
> 
> (2) We could implement more fine-grained temporary directory control,
> allowing the appliance and v2vovl* files to be placed separately.
> However it would still be wrong to mount the place where libguestfs
> creates the appliance (by default /var/tmp) on NFS.
> 
> If you do this then you'd want to mount the large NFS storage
> somewhere else, and there would be a new environment variable
> (V2V_TMPDIR was my proposal IIRC) which you would point to the NFS
> mount.  /var/tmp would be local storage, and used for the appliance.
> (There are other ways to do this if for some reason /var/tmp must be NFS.)
> 
> Thanks Igor and Tomas for helping to get access to the environment.
> 
> Rich.
> 
> 
> Mount entries:
> 
> overlay on / type overlay 
> (rw,relatime,context="system_u:object_r:container_file_t:s0:c581,c761",lowerdir=/var/lib/containers/storage/overlay/l/R65BQQOII4EN66JKVROCRZX4DA:/var/lib/containers/storage/overlay/l/VK5ZPTQFJK7RG4DMBQ6IUDKVYS:/var/lib/containers/storage/overlay/l/QNYZ757HCAAQMJJZUZ6D452CSS,upperdir=/var/lib/containers/storage/overlay/76d93cb1256f566100ec2a7e5b5c4b84acc0bfa6a3cb4ebe0adbdb4a0ffc1a9c/diff,workdir=/var/lib/containers/storage/overlay/76d93cb1256f566100ec2a7e5b5c4b84acc0bfa6a3cb4ebe0adbdb4a0ffc1a9c/work)
> 
> [nfs-server]:/NFSv4_vol_cnv/ibragins-2-3.cnv-qe.rhcloud.com.pvs/pv9 on 
> /var/tmp type nfs4 
> (rw,relatime,vers=4.0,rsize=65536,wsize=65536,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.0.38,local_lock=none,addr=10.9.96.20)
> 
> 
> # df -h
> Filesystem
>Size  Used Avail Use% Mounted on
> overlay   
> 40G   17G   24G  41% /
> tmpfs 
> 64M 0   64M   0% /dev
> tmpfs 
>3.9G 0  3.9G   0% /sys/fs/cgroup
> shm   
> 64M 0   64M   0% /dev/shm
> tmpfs 
>3.9G  7.2M  3.9G   1% /etc/hostname
> tmpfs 

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

2020-06-16 Thread Richard W.M. Jones
On Tue, Jun 16, 2020 at 11:06:28AM +0200, Pino Toscano wrote:
> On Wednesday, 10 June 2020 12:31:33 CEST Richard W.M. Jones wrote:
> > I finally got access to the container.  This is how it's configured:
> > 
> > * / => an overlay fs.
> > 
> > There is sufficient space here, and there are no "funny" restrictions,
> > to be able to create the libguestfs appliance.  I proved this by
> > setting TMPDIR to a temporary directory under / and running
> > libguestfs-test-tool.
> > 
> > There appears to be quite a lot of free space here, so in fact the
> > v2vovl files could easily be stored here too.  (They only store the
> > conversion delta, not the full guest images.)
> > 
> > * /var/tmp => an NFS mount from a PVC
> > 
> > This is a large (2T) external NFS mount.  I actually started two pods
> > to see if they got the same NFS mount point, and they do.  Also I
> > wrote files to /var/tmp in one pod and they were visible in the other.
> > So this seems shared.  Also it uses root squash (so root:root is
> > mapped to 99:99).  For both reasons this cannot be used for the
> > appliance.  If it was mounted at another location it might be used for
> > the v2vovl files.
> > 
> > I've attached the exact mount details at the end of this email.
> > 
> > My conclusion is that we could do one of two things:
> > 
> > Either:
> > 
> > (1) Easiest solution is simply not mount anything under /var/tmp, and
> > let it be local storage.  Assuming all these containers are getting ~40G
> > of local storage, that's more than enough for virt-v2v to run and
> > store the appliance and overlays.  Everything should just work once
> > you remove that /var/tmp mountpoint and leave it as local storage.
> > 
> > ie these lines are removed:
> > - mountPath: /var/tmp
> >   name: v2v-conversion-temp
> > 
> > Or:
> > 
> > (2) We could implement more fine-grained temporary directory control,
> > allowing the appliance and v2vovl* files to be placed separately.
> > However it would still be wrong to mount the place where libguestfs
> > creates the appliance (by default /var/tmp) on NFS.
> > 
> > If you do this then you'd want to mount the large NFS storage
> > somewhere else, and there would be a new environment variable
> > (V2V_TMPDIR was my proposal IIRC) which you would point to the NFS
> > mount.  /var/tmp would be local storage, and used for the appliance.
> > (There are other ways to do this if for some reason /var/tmp must be NFS.)
> 
> Or:
> 
> (3) set LIBGUESTFS_CACHEDIR away from /var/tmp or NFS-mounted places,
> so we avoid any root_squash issue, and avoid any sharing of temporary
> files that linger after the container execution.

Sure, but it's kind of like the same as (1).  IMHO /var/tmp simply
shouldn't be shared and shouldn't be on NFS.  We can do (3) if for
some reason fixing the pod configuration cannot be done.

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

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



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

2020-06-16 Thread Pino Toscano
On Wednesday, 10 June 2020 12:31:33 CEST Richard W.M. Jones wrote:
> I finally got access to the container.  This is how it's configured:
> 
> * / => an overlay fs.
> 
> There is sufficient space here, and there are no "funny" restrictions,
> to be able to create the libguestfs appliance.  I proved this by
> setting TMPDIR to a temporary directory under / and running
> libguestfs-test-tool.
> 
> There appears to be quite a lot of free space here, so in fact the
> v2vovl files could easily be stored here too.  (They only store the
> conversion delta, not the full guest images.)
> 
> * /var/tmp => an NFS mount from a PVC
> 
> This is a large (2T) external NFS mount.  I actually started two pods
> to see if they got the same NFS mount point, and they do.  Also I
> wrote files to /var/tmp in one pod and they were visible in the other.
> So this seems shared.  Also it uses root squash (so root:root is
> mapped to 99:99).  For both reasons this cannot be used for the
> appliance.  If it was mounted at another location it might be used for
> the v2vovl files.
> 
> I've attached the exact mount details at the end of this email.
> 
> My conclusion is that we could do one of two things:
> 
> Either:
> 
> (1) Easiest solution is simply not mount anything under /var/tmp, and
> let it be local storage.  Assuming all these containers are getting ~40G
> of local storage, that's more than enough for virt-v2v to run and
> store the appliance and overlays.  Everything should just work once
> you remove that /var/tmp mountpoint and leave it as local storage.
> 
> ie these lines are removed:
> - mountPath: /var/tmp
>   name: v2v-conversion-temp
> 
> Or:
> 
> (2) We could implement more fine-grained temporary directory control,
> allowing the appliance and v2vovl* files to be placed separately.
> However it would still be wrong to mount the place where libguestfs
> creates the appliance (by default /var/tmp) on NFS.
> 
> If you do this then you'd want to mount the large NFS storage
> somewhere else, and there would be a new environment variable
> (V2V_TMPDIR was my proposal IIRC) which you would point to the NFS
> mount.  /var/tmp would be local storage, and used for the appliance.
> (There are other ways to do this if for some reason /var/tmp must be NFS.)

Or:

(3) set LIBGUESTFS_CACHEDIR away from /var/tmp or NFS-mounted places,
so we avoid any root_squash issue, and avoid any sharing of temporary
files that linger after the container execution.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

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

2020-06-10 Thread Richard W.M. Jones
I finally got access to the container.  This is how it's configured:

* / => an overlay fs.

There is sufficient space here, and there are no "funny" restrictions,
to be able to create the libguestfs appliance.  I proved this by
setting TMPDIR to a temporary directory under / and running
libguestfs-test-tool.

There appears to be quite a lot of free space here, so in fact the
v2vovl files could easily be stored here too.  (They only store the
conversion delta, not the full guest images.)

* /var/tmp => an NFS mount from a PVC

This is a large (2T) external NFS mount.  I actually started two pods
to see if they got the same NFS mount point, and they do.  Also I
wrote files to /var/tmp in one pod and they were visible in the other.
So this seems shared.  Also it uses root squash (so root:root is
mapped to 99:99).  For both reasons this cannot be used for the
appliance.  If it was mounted at another location it might be used for
the v2vovl files.

I've attached the exact mount details at the end of this email.

My conclusion is that we could do one of two things:

Either:

(1) Easiest solution is simply not mount anything under /var/tmp, and
let it be local storage.  Assuming all these containers are getting ~40G
of local storage, that's more than enough for virt-v2v to run and
store the appliance and overlays.  Everything should just work once
you remove that /var/tmp mountpoint and leave it as local storage.

ie these lines are removed:
- mountPath: /var/tmp
  name: v2v-conversion-temp

Or:

(2) We could implement more fine-grained temporary directory control,
allowing the appliance and v2vovl* files to be placed separately.
However it would still be wrong to mount the place where libguestfs
creates the appliance (by default /var/tmp) on NFS.

If you do this then you'd want to mount the large NFS storage
somewhere else, and there would be a new environment variable
(V2V_TMPDIR was my proposal IIRC) which you would point to the NFS
mount.  /var/tmp would be local storage, and used for the appliance.
(There are other ways to do this if for some reason /var/tmp must be NFS.)

Thanks Igor and Tomas for helping to get access to the environment.

Rich.


Mount entries:

overlay on / type overlay 
(rw,relatime,context="system_u:object_r:container_file_t:s0:c581,c761",lowerdir=/var/lib/containers/storage/overlay/l/R65BQQOII4EN66JKVROCRZX4DA:/var/lib/containers/storage/overlay/l/VK5ZPTQFJK7RG4DMBQ6IUDKVYS:/var/lib/containers/storage/overlay/l/QNYZ757HCAAQMJJZUZ6D452CSS,upperdir=/var/lib/containers/storage/overlay/76d93cb1256f566100ec2a7e5b5c4b84acc0bfa6a3cb4ebe0adbdb4a0ffc1a9c/diff,workdir=/var/lib/containers/storage/overlay/76d93cb1256f566100ec2a7e5b5c4b84acc0bfa6a3cb4ebe0adbdb4a0ffc1a9c/work)

[nfs-server]:/NFSv4_vol_cnv/ibragins-2-3.cnv-qe.rhcloud.com.pvs/pv9 on /var/tmp 
type nfs4 
(rw,relatime,vers=4.0,rsize=65536,wsize=65536,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.0.38,local_lock=none,addr=10.9.96.20)


# df -h
Filesystem  
 Size  Used Avail Use% Mounted on
overlay 
  40G   17G   24G  41% /
tmpfs   
  64M 0   64M   0% /dev
tmpfs   
 3.9G 0  3.9G   0% /sys/fs/cgroup
shm 
  64M 0   64M   0% /dev/shm
tmpfs   
 3.9G  7.2M  3.9G   1% /etc/hostname
tmpfs   
 3.9G  4.0K  3.9G   1% /data/input
devtmpfs
 3.9G 0  3.9G   0% /dev/kvm
/dev/mapper/coreos-luks-root-nocrypt
  40G   17G   24G  41% /etc/hosts
[nfs-server]:/NFSv4_vol_cnv/ibragins-2-3.cnv-qe.rhcloud.com.pvs/pv9   2.0T  
945G 1002G  49% /var/tmp
[nfs-server]:/NFSv4_vol_cnv/ibragins-2-3.cnv-qe.rhcloud.com.pvs/pv15  2.0T  
945G 1002G  49% /data/vm/disk1
tmpfs   
 3.9G   24K  3.9G   1% 
/run/secrets/kubernetes.io/serviceaccount


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


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

2020-04-09 Thread Tomáš Golembiovský
On Tue, Apr 07, 2020 at 04:31:57PM +0200, Pino Toscano wrote:
> On Tuesday, 7 April 2020 14:59:11 CEST Tomáš Golembiovský wrote:
> > 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.XX)
> > > > >   $ 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].
> 
> This may be the case, i.e. something else before even the supermin
> appliance check that triggers the creation of the cachedir.
> 
> In the end though, libguestfs prefers a supermin appliance before a
> fixed appliance; the whole logic is here:
> https://github.com/libguestfs/libguestfs/blob/c2c11382bbeb4500f3388a31ffd08cfc18b0de40/lib/appliance.c
> In particular, see the locate_or_build_appliance function and the
> comment before it, and contains_fixed_appliance &
> contains_supermin_appliance helper functions.

To avoid this, the fixed appliance was created in a separate directory,
then the content of /usr/lib64/guestfs was removed and finally the fixed
appliance was moved there.

> 
> If you have a setup like:
> 
> /usr/lib64/guestfs
> ├── initrd
> ├── kernel
> ├── README.fixed
> ├── root
> └── supermin.d
> ├── base.tar.gz
> └── packages
> 
> with 

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

2020-04-07 Thread Pino Toscano
On Tuesday, 7 April 2020 14:59:11 CEST Tomáš Golembiovský wrote:
> 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.XX)
> > > >   $ 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].

This may be the case, i.e. something else before even the supermin
appliance check that triggers the creation of the cachedir.

In the end though, libguestfs prefers a supermin appliance before a
fixed appliance; the whole logic is here:
https://github.com/libguestfs/libguestfs/blob/c2c11382bbeb4500f3388a31ffd08cfc18b0de40/lib/appliance.c
In particular, see the locate_or_build_appliance function and the
comment before it, and contains_fixed_appliance &
contains_supermin_appliance helper functions.

If you have a setup like:

/usr/lib64/guestfs
├── initrd
├── kernel
├── README.fixed
├── root
└── supermin.d
├── base.tar.gz
└── packages

with LIBGUESTFS_PATH=/usr/lib64/guestfs, then the logic fill find first
the two files under supermin.d as supermin appliance, and not even
consider the fixed appliance there.

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

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

2020-04-07 Thread Richard W.M. Jones
On Tue, Apr 07, 2020 at 02:16:49PM +0100, Daniel P. Berrangé wrote:
> 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.XX)
> > > >   $ 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.

To clarify: there are two different configs we're considering.  The
"fixed appliance" one is where we prebuild the appliance and put it in
the container (but it runs into the problem with sparse files not
being supported properly, since it is a very large sparse file).  The
other is the normal one where we run virt-v2v and the appliance is
created and cached the first time.

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

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



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

2020-04-07 Thread Daniel P . Berrangé
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.XX)
> > >   $ 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 :|

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



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

2020-04-07 Thread Tomáš Golembiovský
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.XX)
> > >   $ 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 

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

2020-04-07 Thread Richard W.M. Jones
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/

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



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

2020-04-07 Thread Pino Toscano
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.XX)
> >   $ 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

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

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

2020-04-07 Thread Daniel P . Berrangé
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 :|

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



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

2020-04-07 Thread Richard W.M. Jones
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.XX)
>   $ 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

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



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

2020-04-07 Thread Pino Toscano
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 

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

2020-04-06 Thread Daniel P . Berrangé
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

In a container world /var/tmp usually won't be persistent. Each
boot of the container will typically start with a pristine copy of
the filesystem, so a fresh & empty /var/tmp. Any changes to the
/var/tmp, or anywhere else in the default root filesystem tree
will get thrown away at container shutdown.

NB It is technically possible to start a container, stop it, and then
boot it against from the same FS instance, but it isn't very common
to do that IME.

Extra storage will be made available to the container at admin
specified paths, and though (as a user) you don't typically know
what kind of storage this is, most commonly it will be something
network backed, whether glusterfs, or nfs, or ceph, while the
default / will usually be local storage. 

> That said, it is not clear to me why /var/tmp should be shared among
> containers.

I wouldn't ever expect to see /var/tmp shared in containers. Even the
add-on persistent storage that you can make available is usually
dedicated to a single container & wiped / freshly formatted.

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

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



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

2020-04-06 Thread Daniel P . Berrangé
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 

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

2020-04-03 Thread Daniel P . Berrangé
On Fri, Apr 03, 2020 at 12:53:14PM +0100, Richard W.M. Jones 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.
> > 
> > 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)
> 
> "Because containers" is the only answer I can give to this.
> Containers have weird constraints, in that /var/tmp may be
> big, but not big enough for the overlay files.
> 
> I still think the principle of being able to put the
> libguestfs appliance and the other big temporary files
> in different places is not a bad one.

Yeah, it makes conceptual sense to me to be able to control where
v2v puts its temporary disks explicitly, and not have this location
tied to libguestfs' temporary location since they are separate
components.

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

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



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

2020-04-03 Thread Richard W.M. Jones
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.
> 
> 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)

"Because containers" is the only answer I can give to this.
Containers have weird constraints, in that /var/tmp may be
big, but not big enough for the overlay files.

I still think the principle of being able to put the
libguestfs appliance and the other big temporary files
in different places is not a bad one.

(CC-ing Dan, and Igor who is the bug reporter)

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



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

2020-04-02 Thread Pino Toscano
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.

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)

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

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

2020-04-02 Thread Richard W.M. Jones
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)?

> That said, it is not clear to me why /var/tmp should be shared among
> containers.

I also don't think /var/tmp should be shared, nor use NFS.

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.

Another issue here is that with kubenetes it's not up to the tenant to
choose what backing store is used for volume mounts.  So if the
administrator of the cluster set it up with NFS, then that's what you
have to use.  (However using NFS for /var/tmp is still wrong).

That's the problem we're trying to solve.

> > diff --git a/docs/virt-v2v.pod b/docs/virt-v2v.pod
> > index ed95fdc8e..dbfd10cad 100644
> > --- a/docs/virt-v2v.pod
> > +++ b/docs/virt-v2v.pod
> > @@ -1206,8 +1206,9 @@ possible.
> >  
> >  =head3 Disk space
> >  
> > -Virt-v2v places potentially large temporary files in C<$TMPDIR> (which
> > -is F if you don't set it).  Using tmpfs is a bad idea.
> 
> Regardless of this patch, this bit is not correct:
> - libguestfs does not places large files in $TMPDIR (but in CACHEDIR)
> - $TMPDIR is not /var/tmp by default, but /tmp

Yes this needs fixing separately.  I forgot $TMPDIR is not
related to /var/tmp.

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

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



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

2020-04-02 Thread Pino Toscano
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

That said, it is not clear to me why /var/tmp should be shared among
containers.

> diff --git a/docs/virt-v2v.pod b/docs/virt-v2v.pod
> index ed95fdc8e..dbfd10cad 100644
> --- a/docs/virt-v2v.pod
> +++ b/docs/virt-v2v.pod
> @@ -1206,8 +1206,9 @@ possible.
>  
>  =head3 Disk space
>  
> -Virt-v2v places potentially large temporary files in C<$TMPDIR> (which
> -is F if you don't set it).  Using tmpfs is a bad idea.

Regardless of this patch, this bit is not correct:
- libguestfs does not places large files in $TMPDIR (but in CACHEDIR)
- $TMPDIR is not /var/tmp by default, but /tmp

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs