Libguestfs - Dec 2021 - [PATCH] convert_linux: translate the first CD-ROM's references in boot conf files

On Wed, Dec 15, 2021 at 11:17:51AM +0100, Laszlo Ersek
wrote:
> I *really* dislike the one aspect of the current multi-line messages
> that they are nailed to column #0. It disrupts the code flow for me. For
> example, consider the following snippet from this very file
> [convert/convert_linux.ml]:
> 
>                 g#cp grub_config uefi_grub_conf;
>                 let fix_script = sprintf
> "#!/bin/bash
> efibootmgr -c -L \"CentOS 6\"
> rm -rf %s" uefi_fallback_path in
>                 Firstboot.add_firstboot_script
>                   g inspect.i_root "fix uefi boot" fix_script)
>
> I find it less than ideal.
> 
> I'd really like to indent such strings logically. And that's
exactly
> what the backslash is for (I had checked the ocaml documentation): after
> an escaped newline, leading spaces and tabs are dropped. I think that's
> a fantastic feature of OCaml; I used it intentionally.
Right, this is indeed a reason to use the \, as it drops the following
whitespace.
> > You can just match to arbitrary depth in single match statements, so
> > the whole match would become something like:
> > 
> >   let map = map @
> >     match cdroms with
> >     | { Some Source_IDE, Some slot, `RHEL_family v } :: _ when v <=
5 ->
> >            [("hd" ^ drive_name slot, "cdrom")]
> >     | _ -> [] in
> 
> Does this make the code easier to read to you? (Honest question.) To me,
> this does not separate the steps "take first" and
"investigate first",
> and so it's harder to read. But if that's only because I don't
have
> enough OCaml experience yet, I'm happy to change it. (Because, in
> comparison, you might find the two "match" expressions to be the
> distraction.)
For me it's less code => better, within reason. The above (after
fixing the bugs - see later emails) is more natural.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

On 12/15/21 11:57, Richard W.M. Jones wrote:
> On Wed, Dec 15, 2021 at 11:17:51AM +0100, Laszlo Ersek wrote:
>> I *really* dislike the one aspect of the current multi-line messages
>> that they are nailed to column #0. It disrupts the code flow for me.
For
>> example, consider the following snippet from this very file
>> [convert/convert_linux.ml]:
>>
>>                 g#cp grub_config uefi_grub_conf;
>>                 let fix_script = sprintf
>> "#!/bin/bash
>> efibootmgr -c -L \"CentOS 6\"
>> rm -rf %s" uefi_fallback_path in
>>                 Firstboot.add_firstboot_script
>>                   g inspect.i_root "fix uefi boot"
fix_script)
>>
>> I find it less than ideal.
>>
>> I'd really like to indent such strings logically. And that's
exactly
>> what the backslash is for (I had checked the ocaml documentation):
after
>> an escaped newline, leading spaces and tabs are dropped. I think
that's
>> a fantastic feature of OCaml; I used it intentionally.
> 
> Right, this is indeed a reason to use the \, as it drops the following
> whitespace.
> 
>>> You can just match to arbitrary depth in single match statements,
so
>>> the whole match would become something like:
>>>
>>>   let map = map @
>>>     match cdroms with
>>>     | { Some Source_IDE, Some slot, `RHEL_family v } :: _ when v
<= 5 ->
>>>            [("hd" ^ drive_name slot, "cdrom")]
>>>     | _ -> [] in
>>
>> Does this make the code easier to read to you? (Honest question.) To
me,
>> this does not separate the steps "take first" and
"investigate first",
>> and so it's harder to read. But if that's only because I
don't have
>> enough OCaml experience yet, I'm happy to change it. (Because, in
>> comparison, you might find the two "match" expressions to be
the
>> distraction.)
> 
> For me it's less code => better, within reason. The above (after
> fixing the bugs - see later emails) is more natural.
Before posting v2, I intended to test it :) , so I installed a new RHEL5 guest,
and tried to run virt-v2v (with the v2 patch) on it:

virt-v2v -i libvirt -o local -os ~/output rhel5.11

Unfortunately, things break before my code gets a chance to run; here:

  (* Fail early if i_apps is empty.  Certain steps such as kernel
   * detection won't work without this.  If the list is empty it
   * likely indicates that libguestfs inspection is broken for
   * this guest.  See for example RHBZ#1965147.
   *)
  if inspect.i_apps = [] then
    error (f_"inspection of the package database failed for ...

This seems to come from an empty apps list, from
"convert/inspect_source.ml", which in turn invokes
"g#inspect_list_applications2".

I turned to guestfish, and ran the following manually:

inspect-os
inspect-get-mountpoints /dev/VolGroup00/LogVol00
inspect-list-applications2 /dev/VolGroup00/LogVol00

Indeed the last command does not return anything. I turned on
"verbose" and "trace" then, and then I see a failure in the
daemon:

libguestfs: trace: inspect_list_applications2
"/dev/VolGroup00/LogVol00"
libguestfs: trace: inspect_get_type "/dev/VolGroup00/LogVol00"
libguestfs: trace: inspect_get_type = "linux"
libguestfs: trace: inspect_get_package_format
"/dev/VolGroup00/LogVol00"
libguestfs: trace: inspect_get_package_format = "rpm"
libguestfs: trace: internal_list_rpm_applications
\x1b[31;1merror: \x1b[0m\x1b[1mUnable to open /usr/lib/rpm/rpmrc for reading: No
such file or directory.
\x1b[0m\x1b[31;1merror: \x1b[0m\x1b[1mno dbpath has been set
\x1b[0m\x1b[31;1merror: \x1b[0m\x1b[1mcannot open Packages database in 
\x1b[0mlibrpm returned 0 installed packages
libguestfs: trace: internal_list_rpm_applications = <struct
guestfs_application2_list(0)>
libguestfs: trace: inspect_list_applications2 = <struct
guestfs_application2_list(0)>

The file "/usr/lib/rpm/rpmrc" absolutely exists in the guest (I booted
it up separately, and checked it).

It seems that "internal_list_rpm_applications" [daemon/rpm.ml] calls
chroot, and uses a child project -- can that be related to the issue?

Anyway, I think I'll post v2, just saying that I could not actually test it.

Thanks,
Laszlo