[Libguestfs] [libguestfs-common PATCH 2/2] detect_kernels: deal with RHEL's kernel-core / kernel-modules-core split

[Laszlo Ersek]

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 at redhat.com>
>> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
>> Tested-by: Vera Wu <vwu at 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

Thank you!

I've posted the 9.2.z port now as well.

> 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