Libguestfs - Mar 2019 - Re: [PATCH] v2v: linux: canonicalize module path for arch detection (RHBZ#1690574)

Kernel modules can be also symlinks to files available outside the
"canonical" module directory; since `file` by default does not
deference
symlinks, passing directly symlinks to the architecture detection
results in errors, as the "file" API does not recognize "symbolic
link"
as architecture.

To prevent this situation, canonicalize the path of the module picked
for architecture detection: this way, "file-architecture" will run on
a
real file.
---
 v2v/linux_kernels.ml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/v2v/linux_kernels.ml b/v2v/linux_kernels.ml
index 3313aabc7..889ec2f2a 100644
--- a/v2v/linux_kernels.ml
+++ b/v2v/linux_kernels.ml
@@ -189,7 +189,7 @@ let detect_kernels (g : G.guestfs) inspect family bootloader
*)
              let arch                 let any_module = modpath ^ List.hd
modules in
-               g#file_architecture any_module in
+               g#file_architecture (g#realpath any_module) in
 
              (* Just return the module names, without path or extension. *)
              let modules = List.filter_map (
-- 
2.20.1

On Wed, Mar 20, 2019 at 05:00:06PM +0100, Pino Toscano
wrote:
> Kernel modules can be also symlinks to files available outside the
> "canonical" module directory; since `file` by default does not
deference
> symlinks, passing directly symlinks to the architecture detection
> results in errors, as the "file" API does not recognize
"symbolic link"
> as architecture.
> 
> To prevent this situation, canonicalize the path of the module picked
> for architecture detection: this way, "file-architecture" will
run on a
> real file.
> ---
>  v2v/linux_kernels.ml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/v2v/linux_kernels.ml b/v2v/linux_kernels.ml
> index 3313aabc7..889ec2f2a 100644
> --- a/v2v/linux_kernels.ml
> +++ b/v2v/linux_kernels.ml
> @@ -189,7 +189,7 @@ let detect_kernels (g : G.guestfs) inspect family
bootloader >                *)
>               let arch >                 let any_module = modpath ^
List.hd modules in
> -               g#file_architecture any_module in
> +               g#file_architecture (g#realpath any_module) in
I don't mind this, but would it better to fix the actual API?

Anyway either way is fine so ACK.

Rich.
>               (* Just return the module names, without path or extension.
*)
>               let modules = List.filter_map (
> -- 
> 2.20.1
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
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 Wednesday, 20 March 2019 17:12:22 CET Richard W.M. Jones
wrote:
> On Wed, Mar 20, 2019 at 05:00:06PM +0100, Pino Toscano wrote:
> > Kernel modules can be also symlinks to files available outside the
> > "canonical" module directory; since `file` by default does
not deference
> > symlinks, passing directly symlinks to the architecture detection
> > results in errors, as the "file" API does not recognize
"symbolic link"
> > as architecture.
> > 
> > To prevent this situation, canonicalize the path of the module picked
> > for architecture detection: this way, "file-architecture"
will run on a
> > real file.
> > ---
> >  v2v/linux_kernels.ml | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/v2v/linux_kernels.ml b/v2v/linux_kernels.ml
> > index 3313aabc7..889ec2f2a 100644
> > --- a/v2v/linux_kernels.ml
> > +++ b/v2v/linux_kernels.ml
> > @@ -189,7 +189,7 @@ let detect_kernels (g : G.guestfs) inspect family
bootloader > >                *)
> >               let arch > >                 let any_module =
modpath ^ List.hd modules in
> > -               g#file_architecture any_module in
> > +               g#file_architecture (g#realpath any_module) in
> 
> I don't mind this, but would it better to fix the actual API?
The "file" API handles any non-regular file, printing its type (see
also daemon/file.ml).  So I'm not sure there is room for changes, since
this is actually a wanted behaviour (see also its tests).

-- 
Pino Toscano