[Libguestfs] [PATCH] v2v: rework free space check in guest mountpoints

[Richard W.M. Jones]

On Mon, Nov 06, 2017 at 03:45:14PM +0100, Pino Toscano wrote:
> - move the space needed by mountpoint mapping to an own function,
>   outside of the checking loop
> - remove the minimum limit of 100M on partitions, since existing
>   partitions that we check (e.g. /boot) may be smaller than that
> - in case /boot is not on a separate mountpoint, enforce the space check
>   on the root mountpoint instead, since it is where /boot is in that
>   case
> ---
>  v2v/v2v.ml | 55 ++++++++++++++++++++++++++++---------------------------
>  1 file changed, 28 insertions(+), 27 deletions(-)
> 
> diff --git a/v2v/v2v.ml b/v2v/v2v.ml
> index b4c41e188..24b38458f 100644
> --- a/v2v/v2v.ml
> +++ b/v2v/v2v.ml
> @@ -415,34 +415,35 @@ and print_mpstat chan { mp_dev = dev; mp_path = path;
>   *)
>  and check_guest_free_space mpstats =
>    message (f_"Checking for sufficient free disk space in the guest");
> +
> +  (* Check whether /boot has its own mount point. *)
> +  let has_boot = List.exists (fun { mp_path } -> mp_path = "/boot") mpstats in
> +
> +  let needed_bytes_for_mp = function
> +    | "/boot"
> +    | "/" when not has_boot ->
> +      (* We usually regenerate the initramfs, which has a
> +       * typical size of 20-30MB.  Hence:
> +       *)
> +      50_000_000L
> +    | "/" ->
> +      (* We may install some packages, and they would usually go
> +       * on the root filesystem.
> +       *)
> +      20_000_000L
> +    | _ ->
> +      (* For everything else, just make sure there is some free space. *)
> +      10_000_000L
> +  in
> +
>    List.iter (
> -    fun { mp_path = mp;
> -          mp_statvfs = { G.bfree; blocks; bsize } } ->
> -      (* Ignore small filesystems. *)
> -      let total_size = blocks *^ bsize in
> -      if total_size > 100_000_000L then (
> -        (* bfree = free blocks for root user *)
> -        let free_bytes = bfree *^ bsize in
> -        let needed_bytes =
> -          match mp with
> -          | "/" ->
> -            (* We may install some packages, and they would usually go
> -             * on the root filesystem.
> -             *)
> -            20_000_000L
> -          | "/boot" ->
> -            (* We usually regenerate the initramfs, which has a
> -             * typical size of 20-30MB.  Hence:
> -             *)
> -            50_000_000L
> -          | _ ->
> -            (* For everything else, just make sure there is some free space. *)
> -            10_000_000L in
> -
> -        if free_bytes < needed_bytes then
> -          error (f_"not enough free space for conversion on filesystem ‘%s’.  %Ld bytes free < %Ld bytes needed")
> -            mp free_bytes needed_bytes
> -      )
> +    fun { mp_path; mp_statvfs = { G.bfree; bsize } } ->
> +      (* bfree = free blocks for root user *)
> +      let free_bytes = bfree *^ bsize in
> +      let needed_bytes = needed_bytes_for_mp mp_path in
> +      if free_bytes < needed_bytes then
> +        error (f_"not enough free space for conversion on filesystem ‘%s’.  %Ld bytes free < %Ld bytes needed")
> +          mp_path free_bytes needed_bytes
>    ) mpstats
>  
>  (* Perform the fstrim. *)

Yeah, it's a pretty straightforward refactoring except for
losing the 100 MB min size and the bit about checking root if
/boot is not separately mounted.

Therefore:
ACK

I suspect this change is a bit dangerous for RHEL 7.5 at this
point.  Better to give it a bit more testing in case the
100 MB min size was really needed for some reason.

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