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