[PATCH virt-v2v] v2v: Allow temporary directory to be set on a global basis.
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. --- docs/virt-v2v.pod | 21 +++++++++++++++++---- v2v/input_ova.ml | 4 ++-- v2v/input_vmx.ml | 3 +-- v2v/nbdkit.ml | 3 +-- v2v/output_glance.ml | 3 +-- v2v/output_null.ml | 5 ++--- v2v/output_rhv_upload.ml | 3 +-- v2v/parse_ova.ml | 5 ++--- v2v/python_script.ml | 3 +-- v2v/utils.ml | 8 ++++++-- v2v/utils.mli | 4 ++++ v2v/v2v.ml | 15 ++++++--------- 12 files changed, 44 insertions(+), 33 deletions(-) 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</var/tmp> if you don't set it). Using tmpfs is a bad idea. +Virt-v2v places potentially large temporary files in +C<$VIRT_V2V_TMPDIR> (or F</var/tmp>, see also L</ENVIRONMENT VARIBLES> +below). Using tmpfs is a bad idea. For each guest disk, an overlay is stored temporarily. This stores the changes made during conversion, and is used as a cache. The @@ -1554,10 +1555,22 @@ conversion. =over 4 +=item C<VIRT_V2V_TMPDIR> + +=item C<LIBGUESTFS_CACHEDIR> + =item C<TMPDIR> -Location of the temporary directory used for the potentially large -temporary overlay file. +Location of the temporary directory. This is used for the potentially +large temporary overlay files. Also for miscellaneous temporary +files. These environment variables are checked in order: +C<VIRT_V2V_TMPDIR>, C<LIBGUESTFS_CACHEDIR>, C<TMPDIR>. If none are +set then F</var/tmp> is used. + +To reliably ensure temporary files are cleaned up (for example in case +virt-v2v crashes) you should create a randomly named directory under +F</var/tmp>, set C<VIRT_V2V_TMPDIR> to point to this directory, then +when virt-v2v exits remove the directory. See the L</Disk space> section above. diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml index 5d3bece18..86f81e1f9 100644 --- a/v2v/input_ova.ml +++ b/v2v/input_ova.ml @@ -132,8 +132,8 @@ class input_ova ova = object (* The spec allows the file to be gzip-compressed, in * which case we must uncompress it into a temporary. *) - let temp_dir = (open_guestfs ())#get_cachedir () in - let new_filename = Filename.temp_file ~temp_dir "ova" ".vmdk" in + let new_filename = + Filename.temp_file ~temp_dir:Utils.tmpdir "ova" ".vmdk" in unlink_on_exit new_filename; let cmd = sprintf "zcat %s > %s" (quote filename) (quote new_filename) in diff --git a/v2v/input_vmx.ml b/v2v/input_vmx.ml index f1d143e97..5a1804182 100644 --- a/v2v/input_vmx.ml +++ b/v2v/input_vmx.ml @@ -389,8 +389,7 @@ and find_nics vmx = class input_vmx input_password input_transport arg = let tmpdir = - let base_dir = (open_guestfs ())#get_cachedir () in - let t = Mkdtemp.temp_dir ~base_dir "vmx." in + let t = Mkdtemp.temp_dir ~base_dir:tmpdir "vmx." in rmdir_on_exit t; t in object diff --git a/v2v/nbdkit.ml b/v2v/nbdkit.ml index 65317f9b9..188647f65 100644 --- a/v2v/nbdkit.ml +++ b/v2v/nbdkit.ml @@ -105,8 +105,7 @@ let add_filter_if_available cmd filter = let run_unix cmd = (* Create a temporary directory where we place the socket and PID file. *) let tmpdir = - let base_dir = (open_guestfs ())#get_cachedir () in - let t = Mkdtemp.temp_dir ~base_dir "v2vnbdkit." in + let t = Mkdtemp.temp_dir ~base_dir:tmpdir "v2vnbdkit." in (* tmpdir must be readable (but not writable) by "other" so that * qemu can open the sockets. *) diff --git a/v2v/output_glance.ml b/v2v/output_glance.ml index 0a9e91818..fce223333 100644 --- a/v2v/output_glance.ml +++ b/v2v/output_glance.ml @@ -33,8 +33,7 @@ class output_glance () = * to write to a temporary file. XXX *) let tmpdir = - let base_dir = (open_guestfs ())#get_cachedir () in - let t = Mkdtemp.temp_dir ~base_dir "glance." in + let t = Mkdtemp.temp_dir ~base_dir:tmpdir "glance." in rmdir_on_exit t; t in object diff --git a/v2v/output_null.ml b/v2v/output_null.ml index 3528da50a..5ce30d557 100644 --- a/v2v/output_null.ml +++ b/v2v/output_null.ml @@ -50,7 +50,7 @@ let can_use_qemu_null_co_device () = (* We actually attempt to convert a raw file to the null-co device * using a JSON URL. *) - let tmp = Filename.temp_file "v2vqemunullcotst" ".img" in + let tmp = Filename.temp_file ~temp_dir:tmpdir "v2vqemunullcotst" ".img" in Unix.truncate tmp 1024; let json = [ @@ -75,8 +75,7 @@ class output_null = * the null-co device w/ a JSON URL. *) let tmpdir = - let base_dir = (open_guestfs ())#get_cachedir () in - let t = Mkdtemp.temp_dir ~base_dir "null." in + let t = Mkdtemp.temp_dir ~base_dir:tmpdir "null." in rmdir_on_exit t; t in object diff --git a/v2v/output_rhv_upload.ml b/v2v/output_rhv_upload.ml index 5c6c26110..4922a2040 100644 --- a/v2v/output_rhv_upload.ml +++ b/v2v/output_rhv_upload.ml @@ -155,8 +155,7 @@ class output_rhv_upload output_alloc output_conn rhv_options = (* Create a temporary directory which will be deleted on exit. *) let tmpdir = - let base_dir = (open_guestfs ())#get_cachedir () in - let t = Mkdtemp.temp_dir ~base_dir "rhvupload." in + let t = Mkdtemp.temp_dir ~base_dir:tmpdir "rhvupload." in rmdir_on_exit t; t in diff --git a/v2v/parse_ova.ml b/v2v/parse_ova.ml index 0b939ac43..1f332a82c 100644 --- a/v2v/parse_ova.ml +++ b/v2v/parse_ova.ml @@ -71,8 +71,7 @@ let rec parse_ova ova = if is_directory ova then ova, Directory else ( let tmpdir = - let base_dir = (open_guestfs ())#get_cachedir () in - let t = Mkdtemp.temp_dir ~base_dir "ova." in + let t = Mkdtemp.temp_dir ~base_dir:tmpdir "ova." in rmdir_on_exit t; t in @@ -222,7 +221,7 @@ and uncompress_head format file = and uncompressed_type format file = let head, headlen = uncompress_head format file in let tmpfile, chan = - Filename.open_temp_file "ova.file." "" in + Filename.open_temp_file ~temp_dir:tmpdir "ova.file." "" in output chan head 0 headlen; close_out chan; let ret = detect_file_type tmpfile in diff --git a/v2v/python_script.ml b/v2v/python_script.ml index 33c5e9a21..2d95f4bd5 100644 --- a/v2v/python_script.ml +++ b/v2v/python_script.ml @@ -35,8 +35,7 @@ let create ?(name = "script.py") ?tmpdir code = let tmpdir = match tmpdir with | None -> - let base_dir = (open_guestfs ())#get_cachedir () in - let t = Mkdtemp.temp_dir ~base_dir "v2v." in + let t = Mkdtemp.temp_dir ~base_dir:Utils.tmpdir "v2v." in rmdir_on_exit t; t | Some dir -> dir in diff --git a/v2v/utils.ml b/v2v/utils.ml index ccbb9d68a..8cc24d6a2 100644 --- a/v2v/utils.ml +++ b/v2v/utils.ml @@ -24,6 +24,10 @@ open Std_utils open Tools_utils open Common_gettext.Gettext +let tmpdir = + try Sys.getenv "VIRT_V2V_TMPDIR" + with Not_found -> (open_guestfs ())#get_cachedir () + (* Is SELinux enabled and enforcing on the host? *) let have_selinux = 0 = Sys.command "getenforce 2>/dev/null | grep -isq Enforcing" @@ -112,7 +116,8 @@ let qemu_img_supports_offset_and_size () = (* We actually attempt to create a qcow2 file with a raw backing * file that has an offset and size. *) - let tmp = Filename.temp_file "v2vqemuimgtst" ".img" in + let tmp = Filename.temp_file ~temp_dir:tmpdir "v2vqemuimgtst" ".img" in + unlink_on_exit tmp; Unix.truncate tmp 1024; let json = [ @@ -132,7 +137,6 @@ let qemu_img_supports_offset_and_size () = (if verbose () then "" else " 2>&1") in debug "%s" cmd; let r = 0 = Sys.command cmd in - Unix.unlink tmp; debug "qemu-img supports \"offset\" and \"size\" in json URLs: %b" r; r diff --git a/v2v/utils.mli b/v2v/utils.mli index 937e2b9b8..5afe2954f 100644 --- a/v2v/utils.mli +++ b/v2v/utils.mli @@ -18,6 +18,10 @@ (** Utilities used in virt-v2v only. *) +val tmpdir : string +(** [VIRT_V2V_TMPDIR] or [/var/tmp]. Create all temporary files + under this directory. *) + val have_selinux : bool (** True if SELinux is enabled and enforcing on the host. *) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index 73edff2c4..6dd5fc693 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -264,8 +264,6 @@ and set_source_networks_and_bridges cmdline source = let nics = List.map (Networks.map cmdline.network_map) source.s_nics in { source with s_nics = nics } -and overlay_dir = (open_guestfs ())#get_cachedir () - (* Conversion can fail or hang if there is insufficient free space in * the temporary directory used to store overlays on the host * (RHBZ#1316479). Although only a few hundred MB is actually @@ -273,12 +271,12 @@ and overlay_dir = (open_guestfs ())#get_cachedir () * guestfs appliance which is also stored here. *) and check_host_free_space () = - let free_space = StatVFS.free_space (StatVFS.statvfs overlay_dir) in - debug "check_host_free_space: overlay_dir=%s free_space=%Ld" - overlay_dir free_space; + let free_space = StatVFS.free_space (StatVFS.statvfs tmpdir) in + debug "check_host_free_space: tmpdir=%s free_space=%Ld" + tmpdir free_space; if free_space < 1_073_741_824L then error (f_"insufficient free space in the conversion server temporary directory %s (%s).\n\nEither free up space in that directory, or set the LIBGUESTFS_CACHEDIR environment variable to point to another directory with more than 1GB of free space.\n\nSee also the virt-v2v(1) manual, section \"Minimum free space check in the host\".") - overlay_dir (human_size free_space) + tmpdir (human_size free_space) (* Create a qcow2 v3 overlay to protect the source image(s). *) and create_overlays source_disks = @@ -286,7 +284,7 @@ and create_overlays source_disks = List.mapi ( fun i ({ s_qemu_uri = qemu_uri; s_format = format } as source) -> let overlay_file = - Filename.temp_file ~temp_dir:overlay_dir "v2vovl" ".qcow2" in + Filename.temp_file ~temp_dir:tmpdir "v2vovl" ".qcow2" in unlink_on_exit overlay_file; (* There is a specific reason to use the newer qcow2 variant: @@ -822,8 +820,7 @@ and actual_target_size target_file disk_stats = and preserve_overlays overlays src_name = List.iter ( fun ov -> - let saved_filename = - sprintf "%s/%s-%s.qcow2" overlay_dir src_name ov.ov_sd in + let saved_filename = sprintf "%s/%s-%s.qcow2" tmpdir src_name ov.ov_sd in rename ov.ov_overlay_file saved_filename; info (f_"Overlay saved as %s [--debug-overlays]") saved_filename ) overlays -- 2.25.0
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</var/tmp> 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
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</var/tmp> 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
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
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
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 :|
On Thu, Apr 02, 2020 at 05:51:32PM +0200, Pino Toscano wrote:
On Thursday, 2 April 2020 17:30:39 CEST Richard W.M. Jones wrote:
On Thu, Apr 02, 2020 at 03:21:14PM +0200, Pino Toscano wrote:
On Thursday, 2 April 2020 14:49:18 CEST Richard W.M. Jones wrote:
Previously we placed large files in g#get_cachedir () (usually /var/tmp). However the problem is this ties the libguestfs appliance and the virt-v2v overlay files to the same location.
When virt-v2v is run in a container, or any other situation where local storage is limited, it's helpful to be able to put the overlay files on an externally mounted PVC, which might be using NFS and shared between containers. But putting the libguestfs appliance on NFS in a shared location is certainly not recommended.
This allows the two locations to be set separately:
VIRT_V2V_TMPDIR - location of large temporary files, can use NFS and may be shared
LIBGUESTFS_CACHEDIR - location of libguestfs appliance
Another motivation for this patch is to allow more reliable cleanup of temporary files by an external process, as described in the updated documentation. ---
I do not understand the motivation behind this, which adds yet another location with temporary files in addition to: - LIBGUESTFS_TMPDIR - $TMPDIR by default (which itself is /tmp by default) - LIBGUESTFS_CACHEDIR - /var/tmp by default (with a .guestfs-$UID subdirectory for the appliance)
Before this patch, almost all the temporary files are stored directly or in subdirectories of $TMPDIR, except big files such as overlays and OVA extracted content that are in CACHEDIR. With the proposed changes, _all_ the temporary files will be in CACHEDIR, so there are the following problems: - this directory will be cluttered with a lot more files than before - if it is shared, then other places where it is mounted will see the same files - if it is shared, then creating temporary files will possibly mean doing network I/O - if virt-v2v exits uncleantly, there will be a lot more files to cleanup than now - even without being shared, /var/tmp is persistent unlike /tmp (which can be tmpfs-backed on some distros/setups), meaning old temporary files will linger way more
How about if we confine the change to just large files (ie. ones which are currently placed in cachedir)?
This is already the case, isn't it?
However the way that virt-v2v works at the moment means you cannot put the large files (especially v2vovl*.qcow2) in a different place from the libguestfs appliance. This means that if you have only "some" space in /var/tmp -- enough for the appliance, but not enough for the potentially much larger space required by v2vovl*.qcow2 with multiple copies of virt-v2v running -- then you cannot separate the overlays to another directory. This isn't just a problem for containers.
/var/tmp is a temporary directory, hence it ought to have enough space for big temporary files. This is nothing special for libguestfs or virt-v2v.
It is not valid to assume that /var/tmp is arbitrarily big enough for any files v2v might want to create. AFAIK, there's no real rule about how large it is expected to be. Linux distros typically don't do fine grained partitioning by default, so /var/tmp is often the same filesystem as /, but it is not unusual for admins to have / relatively small, and have a separate data partition somewhere which has the vast majority of storage space available to the host. In the container world the / is indeed generally of a fairly limited size, as that's an unpacked container image filesystem usually on the local host storage. Apps needing large storage capacities will usually be given an explicit volume they can use for this purpose. NB, the provided volume may be persistent across container boots, or it may be freshly formatted on every boot. Whomever configures the container decides this based on the usage reuqirements of whatever app is to be run in the container.
I do not see what makes v2vovl*.qcow2 files so special in this regard, requiring them to be handled specially than other big temporary files already stored by libguestfs/virt-v2v. Examples: - the appliance - the temporary directory for qemu when using the libvirt backend - the extracted content of an OVA, in case we cannot read it directly - the disks when exporting to glance (-o glance)
Out of those things, I think the appliance / QEMU temp dir are in a different league to the disk images. They're going to be small enough that I it wouldn't tax the storage capacity of /var/tmp. Anything related to VM disk images is liable to be an order of magnitude bigger, easily in the 100's of GB league. With this POV, I would put OVA contents, glance exports, and v2v disk images in the same category, and would expect to be able to explicitly control where they are placed, independantly from anything that goes in /var/tmp. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Monday, 6 April 2020 19:45:44 CEST Daniel P. Berrangé wrote:
On Thu, Apr 02, 2020 at 05:51:32PM +0200, Pino Toscano wrote:
On Thursday, 2 April 2020 17:30:39 CEST Richard W.M. Jones wrote:
On Thu, Apr 02, 2020 at 03:21:14PM +0200, Pino Toscano wrote:
On Thursday, 2 April 2020 14:49:18 CEST Richard W.M. Jones wrote:
Previously we placed large files in g#get_cachedir () (usually /var/tmp). However the problem is this ties the libguestfs appliance and the virt-v2v overlay files to the same location.
When virt-v2v is run in a container, or any other situation where local storage is limited, it's helpful to be able to put the overlay files on an externally mounted PVC, which might be using NFS and shared between containers. But putting the libguestfs appliance on NFS in a shared location is certainly not recommended.
This allows the two locations to be set separately:
VIRT_V2V_TMPDIR - location of large temporary files, can use NFS and may be shared
LIBGUESTFS_CACHEDIR - location of libguestfs appliance
Another motivation for this patch is to allow more reliable cleanup of temporary files by an external process, as described in the updated documentation. ---
I do not understand the motivation behind this, which adds yet another location with temporary files in addition to: - LIBGUESTFS_TMPDIR - $TMPDIR by default (which itself is /tmp by default) - LIBGUESTFS_CACHEDIR - /var/tmp by default (with a .guestfs-$UID subdirectory for the appliance)
Before this patch, almost all the temporary files are stored directly or in subdirectories of $TMPDIR, except big files such as overlays and OVA extracted content that are in CACHEDIR. With the proposed changes, _all_ the temporary files will be in CACHEDIR, so there are the following problems: - this directory will be cluttered with a lot more files than before - if it is shared, then other places where it is mounted will see the same files - if it is shared, then creating temporary files will possibly mean doing network I/O - if virt-v2v exits uncleantly, there will be a lot more files to cleanup than now - even without being shared, /var/tmp is persistent unlike /tmp (which can be tmpfs-backed on some distros/setups), meaning old temporary files will linger way more
How about if we confine the change to just large files (ie. ones which are currently placed in cachedir)?
This is already the case, isn't it?
However the way that virt-v2v works at the moment means you cannot put the large files (especially v2vovl*.qcow2) in a different place from the libguestfs appliance. This means that if you have only "some" space in /var/tmp -- enough for the appliance, but not enough for the potentially much larger space required by v2vovl*.qcow2 with multiple copies of virt-v2v running -- then you cannot separate the overlays to another directory. This isn't just a problem for containers.
/var/tmp is a temporary directory, hence it ought to have enough space for big temporary files. This is nothing special for libguestfs or virt-v2v.
It is not valid to assume that /var/tmp is arbitrarily big enough for any files v2v might want to create. AFAIK, there's no real rule about how large it is expected to be. Linux distros typically don't do fine grained partitioning by default, so /var/tmp is often the same filesystem as /, but it is not unusual for admins to have / relatively small, and have a separate data partition somewhere which has the vast majority of storage space available to the host.
In the container world the / is indeed generally of a fairly limited size, as that's an unpacked container image filesystem usually on the local host storage. Apps needing large storage capacities will usually be given an explicit volume they can use for this purpose. NB, the provided volume may be persistent across container boots, or it may be freshly formatted on every boot. Whomever configures the container decides this based on the usage reuqirements of whatever app is to be run in the container.
Sure, I agree with that. The important thing is still that that you need to have space for the temporary files somewhere: be it /var/tmp, /mnt/scratch, whatever. Because of this, and the fact that usually containers are created fresh, the cache of the supermin appliance starts to make little sense, and then a very simple solution is to point libguestfs to that extra space: $ dir=$(mktemp --tmpdir -d /path/to/big/temporary/space/libguestfs.XXXXXX) $ export LIBGUESTGS_CACHEDIR=$dir $ export TMPDIR=$dir # optionally $ libguestfs-test-tool [etc] $ rm -rf $dir Easy to use, already doable, solves all the issues. This whole problem started from a QE report on leftover files after failed migrations: bz#1820282. What this report doesn't say, however, is that beside the mentioned files that virt-v2v creates, there are also leftover files that libguestfs itself creates. These files are usually downloaded from the guest for the inspection, and generally not that big compared to e.g. the overlays that virt-v2v creates. Nonetheless, an abrupt exit of virt-v2v will leave those in place, and they will still slowly fill up the space on /var/tmp (or whatever is the location of $LIBGUESTFS_CACHEDIR). Sure, creating a special directory for virt-v2v solves /some/ of the issues, and most probably the ones that concern most from a disk space POV. However, since we "uncovered" the issue and started to get our hands on it, I don't think it would be ideal to create an ad-hoc solution that solves just some. -- Pino Toscano
On Tue, Apr 07, 2020 at 01:25:02PM +0200, Pino Toscano wrote:
The important thing is still that that you need to have space for the temporary files somewhere: be it /var/tmp, /mnt/scratch, whatever. Because of this, and the fact that usually containers are created fresh, the cache of the supermin appliance starts to make little sense, and then a very simple solution is to point libguestfs to that extra space:
$ dir=$(mktemp --tmpdir -d /path/to/big/temporary/space/libguestfs.XXXXXX) $ export LIBGUESTGS_CACHEDIR=$dir $ export TMPDIR=$dir # optionally $ libguestfs-test-tool [etc] $ rm -rf $dir
Easy to use, already doable, solves all the issues.
So AIUI there are a few problems with this (although I'm still investigating and trying to make a local reproducer): - The external large space may be on NFS, with the usual problems there like root_squash, no SELinux labels, slowness. This means it's not suitable for the appliance, but might nevertheless be suitable for overlay files. - The external large space may be shared with other containers, and I'm not convinced that our locking in supermin will be safe if multiple parallel instances start up at the same time. We certainly never tested it, and don't currently advise it. One may well say (and I might even agree): don't do that, or change the NFS configuration, or don't use NFS. But we may have to deal with either a Kubernetes configuration as it exists already, or may find that the tenant != Kubernetes admin. Another issue, incidental to this but related, is that we cannot use the fixed appliance because docker/podman have broken support for sparse files. (It's possible to configure libguestfs to use qcow2 for fixed appliances, but we don't do this now, we never really tested it, and it both greatly slows down and adds more dependencies to the supermin build step.)
This whole problem started from a QE report on leftover files after failed migrations: bz#1820282.
(I should note also there are two bugs which I personally think we can solve with the same fix, but they are completely different bugs.)
What this report doesn't say, however, is that beside the mentioned files that virt-v2v creates, there are also leftover files that libguestfs itself creates. These files are usually downloaded from the guest for the inspection, and generally not that big compared to e.g. the overlays that virt-v2v creates. Nonetheless, an abrupt exit of virt-v2v will leave those in place, and they will still slowly fill up the space on /var/tmp (or whatever is the location of $LIBGUESTFS_CACHEDIR).
I guess that small files being left around aren't really a problem. The problem they have is large files being left around, and I can understand why that would be an issue and not the small files.
Sure, creating a special directory for virt-v2v solves /some/ of the issues, and most probably the ones that concern most from a disk space POV. However, since we "uncovered" the issue and started to get our hands on it, I don't think it would be ideal to create an ad-hoc solution that solves just some.
Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
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 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 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 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/c2c11382bbeb4500f3388a31ffd08c... [2] https://github.com/libguestfs/libguestfs/blob/c2c11382bbeb4500f3388a31ffd08c...
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.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].
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/c2c11382bbeb4500f3388a31ffd08c... 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 /var/tmp (just for the qemu files).
Or simply move the location of the fixed appliance, and set LIBGUESTFS_PATH to it. I would do it regardless to avoid the situation described above, i.e. that the fixed appliance is shadowed by the supermin appliance because of the same location of both.
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?
Then we can apply that also for the virt-v2v files in general, running it for any content older than 2/3 days. Even better, with the proposed consolidation of temporary directories [1], tmpreaper can be pointed at those directories only, not touching anything else. [1] https://www.redhat.com/archives/libguestfs/2020-April/msg00050.html -- Pino Toscano
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.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].
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/c2c11382bbeb4500f3388a31ffd08c... 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 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 /var/tmp (just for the qemu files).
Or simply move the location of the fixed appliance, and set LIBGUESTFS_PATH to it. I would do it regardless to avoid the situation described above, i.e. that the fixed appliance is shadowed by the supermin appliance because of the same location of both.
Yup, I have already heeded this advice and posted a patch which keeps the appliance in /usr/lib64/guestfs.fixed and sets LIBGUESTFS_PATH. Tomas
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?
Then we can apply that also for the virt-v2v files in general, running it for any content older than 2/3 days. Even better, with the proposed consolidation of temporary directories [1], tmpreaper can be pointed at those directories only, not touching anything else.
[1] https://www.redhat.com/archives/libguestfs/2020-April/msg00050.html
-- Pino Toscano
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
-- Tomáš Golembiovský <tgolembi@redhat.com>
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 :|
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.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.
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
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 :|
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
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
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
On Wed, 10 Jun 2020 11:31:33 +0100 "Richard W.M. Jones" <rjones@redhat.com> 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 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
-- Tomáš Golembiovský <tgolembi@redhat.com>
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" <rjones@redhat.com> 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/
On Tue, 16 Jun 2020 22:06:58 +0100 "Richard W.M. Jones" <rjones@redhat.com> 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" <rjones@redhat.com> 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ý <tgolembi@redhat.com>
participants (4)
-
Daniel P. Berrangé -
Pino Toscano -
Richard W.M. Jones -
Tomáš Golembiovský