Libguestfs - Apr 2022 - [supermin PATCH v2 1/1] Fix for missing /lib/modules.

Even when supplied with $SUPERMIN_MODULES, supermin tries to access
/lib/modules. This directory does not exist by default in all the
environments.
---
 src/format_ext2.ml | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/format_ext2.ml b/src/format_ext2.ml
index e311ea6..2888ed8 100644
--- a/src/format_ext2.ml
+++ b/src/format_ext2.ml
@@ -95,8 +95,18 @@ let build_ext2 debug basedir files modpath kernel_version
appliance size
     printf "supermin: ext2: copying kernel modules\n%!";
 
   (* Import the kernel modules. *)
-  ext2fs_copy_file_from_host fs "/lib" "/lib";
-  ext2fs_copy_file_from_host fs "/lib/modules"
"/lib/modules";
+  (try
+    ext2fs_copy_file_from_host fs "/lib" "/lib";
+  with Unix_error _ ->
+    ext2fs_copy_file_from_host fs "/" "/lib";
+  );
+
+  (try
+    ext2fs_copy_file_from_host fs "/lib/modules"
"/lib/modules";
+  with Unix_error _ ->
+    ext2fs_copy_file_from_host fs "/" "/lib/modules";
+  );
+
   ext2fs_copy_dir_recursively_from_host fs
     modpath ("/lib/modules/" ^ kernel_version);
 
-- 
2.35.1

On 04/06/22 23:15, George Prekas wrote:
> Even when supplied with $SUPERMIN_MODULES, supermin tries to access
> /lib/modules. This directory does not exist by default in all the
> environments.
> ---
>  src/format_ext2.ml | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/src/format_ext2.ml b/src/format_ext2.ml
> index e311ea6..2888ed8 100644
> --- a/src/format_ext2.ml
> +++ b/src/format_ext2.ml
> @@ -95,8 +95,18 @@ let build_ext2 debug basedir files modpath
kernel_version appliance size
>      printf "supermin: ext2: copying kernel modules\n%!";
>
>    (* Import the kernel modules. *)
> -  ext2fs_copy_file_from_host fs "/lib" "/lib";
> -  ext2fs_copy_file_from_host fs "/lib/modules"
"/lib/modules";
> +  (try
> +    ext2fs_copy_file_from_host fs "/lib" "/lib";
> +  with Unix_error _ ->
> +    ext2fs_copy_file_from_host fs "/" "/lib";
> +  );
> +
> +  (try
> +    ext2fs_copy_file_from_host fs "/lib/modules"
"/lib/modules";
> +  with Unix_error _ ->
> +    ext2fs_copy_file_from_host fs "/" "/lib/modules";
> +  );
> +
>    ext2fs_copy_dir_recursively_from_host fs
>      modpath ("/lib/modules/" ^ kernel_version);
>
>
Two things I'm uncertain about:

- Those semicolons *inside* the parens. I don't think we need them. (In
  fact I'm surprised the code compiles at all -- it surely does; I've
  checked.)

- Formatting / indentation. Minimally, the "with" should be aligned
with
  the "try".

So my suggestion would be:

  (try ext2fs_copy_file_from_host fs "/lib" "/lib"
   with Unix_error _ -> ext2fs_copy_file_from_host fs "/"
"/lib");
  (try ext2fs_copy_file_from_host fs "/lib/modules"
"/lib/modules"
   with Unix_error _ -> ext2fs_copy_file_from_host fs "/"
"/lib/modules");

But arguably this is uncomfortable to read and we have a bit of code
duplication here. How about this (only build-tested):
> diff --git a/src/format_ext2.ml b/src/format_ext2.ml
> index e311ea633deb..83eac3ddad57 100644
> --- a/src/format_ext2.ml
> +++ b/src/format_ext2.ml
> @@ -95,8 +95,13 @@ let build_ext2 debug basedir files modpath
kernel_version appliance size
>      printf "supermin: ext2: copying kernel modules\n%!";
>
>    (* Import the kernel modules. *)
> -  ext2fs_copy_file_from_host fs "/lib" "/lib";
> -  ext2fs_copy_file_from_host fs "/lib/modules"
"/lib/modules";
> +  let copy_file_or_root_from_host f > +    try
> +      ext2fs_copy_file_from_host fs f f
> +    with Unix_error _ ->
> +      ext2fs_copy_file_from_host fs "/" f in
> +  copy_file_or_root_from_host "/lib";
> +  copy_file_or_root_from_host "/lib/modules";
>    ext2fs_copy_dir_recursively_from_host fs
>      modpath ("/lib/modules/" ^ kernel_version);
I certainly defer to Rich on all this of course.

Thanks
Laszlo

On Wed, Apr 06, 2022 at 04:15:15PM -0500, George Prekas
wrote:
> Even when supplied with $SUPERMIN_MODULES, supermin tries to access
> /lib/modules. This directory does not exist by default in all the
> environments.
> ---
>  src/format_ext2.ml | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/format_ext2.ml b/src/format_ext2.ml
> index e311ea6..2888ed8 100644
> --- a/src/format_ext2.ml
> +++ b/src/format_ext2.ml
> @@ -95,8 +95,18 @@ let build_ext2 debug basedir files modpath
kernel_version appliance size
>      printf "supermin: ext2: copying kernel modules\n%!";
>  
>    (* Import the kernel modules. *)
> -  ext2fs_copy_file_from_host fs "/lib" "/lib";
> -  ext2fs_copy_file_from_host fs "/lib/modules"
"/lib/modules";
> +  (try
> +    ext2fs_copy_file_from_host fs "/lib" "/lib";
> +  with Unix_error _ ->
> +    ext2fs_copy_file_from_host fs "/" "/lib";
> +  );
> +
> +  (try
> +    ext2fs_copy_file_from_host fs "/lib/modules"
"/lib/modules";
> +  with Unix_error _ ->
> +    ext2fs_copy_file_from_host fs "/" "/lib/modules";
> +  );
> +
>    ext2fs_copy_dir_recursively_from_host fs
>      modpath ("/lib/modules/" ^ kernel_version);
Thanks.  I pushed this with some formatting changes as suggested in
Laszlo's review, and added comments:

https://github.com/libguestfs/supermin/commit/8effa5a686037ba3c2f8c97753f398b73cc54881

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