On 4/5/23 13:55, Richard W.M. Jones wrote:
On Mon, Mar 20, 2023 at 12:53:00PM +0100, Laszlo Ersek wrote:
> In "kernel-5.14.0-269.el9", the "kernel-modules-core" subpackage
got split
> from the "kernel-core" subpackage. Therefore, a single binary RPM
> containing *both* the "/boot/vmlinuz-5.14.0-269.el9.x86_64" file *and* the
> "/lib/modules/5.14.0-269.el9.x86_64" directory no longer exists. The file
> now belongs to "kernel-core", and the directory to
"kernel-modules-core".
>
> As a result, when we investigate the file list of "kernel-core" (based on
> "kernel-core" providing "/boot/vmlinuz-5.14.0-269.el9.x86_64"),
the first
> match against "/lib/modules/" is not the actual module root directory
> "/lib/modules/5.14.0-269.el9.x86_64", but the nonsensical
> "/lib/modules/5.14.0-269.el9.x86_64/.vmlinuz.hmac" regular file. This
> latter file is never a directory, therefore we rule out "kernel-core" as a
> kernel package.
>
> We also rule out "kernel-modules-core" (even earlier) because it does not
> contain "/boot/vmlinuz-5.14.0-269.el9.x86_64".
>
> Now, the code already deals with the case if the prospective kernel
> package *does not provide a match* for the "/lib/modules/" prefix: in that
> case, we construct the modpath manually, from said prefix, and the version
> number found in "/boot/vmlinuz-<version>". This fallback is good, but
it's
> unreachable if *there is* a candidate, it's just wrong (i.e., not a
> directory).
>
> Perform the "is_dir" check on the candidate modpath earlier, so that we
> can fall back to the manual modpath construction if the modpath candidate
> exists, but is wrong.
>
> With this, the original "is_dir" check becomes superfluous (duplicated)
> *except* when the "Not_found" branch is taken. Therefore, hoist the
> original "is_dir" check into that branch.
>
> Bugzilla:
https://bugzilla.redhat.com/show_bug.cgi?id=2175703
> Reported-by: Vera Wu <vwu(a)redhat.com>
> Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
> Tested-by: Vera Wu <vwu(a)redhat.com>
> ---
>
> Notes:
> context:-U5
>
> mldrivers/linux_kernels.ml | 24 +++++++++++++++-----
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/mldrivers/linux_kernels.ml b/mldrivers/linux_kernels.ml
> index 5b8bdee9923d..23ff76a5504c 100644
> --- a/mldrivers/linux_kernels.ml
> +++ b/mldrivers/linux_kernels.ml
> @@ -125,31 +125,43 @@ let detect_kernels (g : G.guestfs) root bootloader apps =
> *)
> let modpath, version =
> let prefix = "/lib/modules/" in
> let prefix_len = String.length prefix in
> try
> - List.find_map (
> + let modpath, version = List.find_map (
> fun filename ->
> let filename_len = String.length filename in
> if filename_len > prefix_len &&
> String.is_prefix filename prefix then (
> let version = String.sub filename prefix_len
> (filename_len - prefix_len) in
> Some (filename, version)
> ) else
> None
> - ) files
> + ) files in
> + (* Fall back to the version in the vmlinuz file name not only if
> + * a candidate pathname couldn't be found under /lib/modules/,
> + * but also in case the candidate pathname doesn't reference a
> + * directory. See RHBZ#2175703.
> + *
> + * Note that this "is_dir" check is deliberately kept
outside of
> + * the "find_map"'s mapper function above: we want the
first
> + * candidate *to be* a directory, and not the first candidate
> + * *that is* a directory.
> + *)
> + if not (g#is_dir ~followsymlinks:true modpath) then
> + raise Not_found;
> + modpath, version
> with Not_found ->
> let version =
> String.sub vmlinuz 14 (String.length vmlinuz - 14) in
> let modpath = prefix ^ version in
> + (* Check that the modpath exists. *)
> + if not (g#is_dir ~followsymlinks:true modpath) then
> + raise Not_found;
> modpath, version in
>
> - (* Check that the modpath exists. *)
> - if not (g#is_dir ~followsymlinks:true modpath) then
> - raise Not_found;
> -
> (* Find the initramfs which corresponds to the kernel.
> * Since the initramfs is built at runtime, and doesn't have
> * to be covered by the RPM file list, this is basically
> * guesswork.
> *)
I see it's upstream, but anyway a late:
ACK
That detect_kernels function is horribly complicated and hard to
follow. At some point we need to attack it and split it up into
manageable chunks.
Right, it was a struggle for me to find the right location to tweak,
based on just the -v -x log and the source code.
Laszlo