On Mon, Feb 03, 2025 at 02:35:20PM -0500, Cole Robinson wrote:

Current package removal implementation uses `rpm -e`, which will fail if anything depends on the package we are trying to remove. Like when `spausedd` is dependent on `open-vm-tools`.

Reuse the package uninstall logic from virt-customize, which will handle this no problem.

Fixes: https://issues.redhat.com/browse/RHEL-71522

Signed-off-by: Cole Robinson <crobinso@redhat.com> ---

First time touching ocaml, be gentle :)

This fixes my minimal test case, but there's test suite regressions.

+ How to run a single test against in tree virt-v2v?

automake can do this for you, eg: $ make check -C test-data # one time, creates test data $ make check -C tests TESTS=" test-cdrom.sh " (you can list multiple tests). For running other programs using local virt-v2v, you can use the ./run script in the top build directory, eg: ~/virt-v2v/run ./some_script_that_uses_virt_v2v The ./run script will set the PATH and other environment variables correctly so that the locally build virt-v2v runs. This is in fact exactly how the 'make check' command above works, see this line: https://github.com/libguestfs/virt-v2v/blob/9fc97d621a2e0bc1d7f7b369f8252ea7...

+ and/or get `make check` to spit out more info?

Look at tests/<test-name>.log. If there's still not enough information, you'll have to modify the test to add 'virt-v2v -v -x' flags.

more questions:

+ uninstall failure is still fatal. example how to make this non-fatal?

(Comment below)

+ `virt-v2v -v` logs don't show command stdout. expected? do I need an extra debug option?

g#sh is implemented here: https://github.com/libguestfs/libguestfs/blob/master/daemon/sh.c (do_sh) You can see that stdout is captured and returned as string which is why it doesn't appear in any debugging output. stderr is also captured and returned in error messages. I don't think in either case you'll see the output in 'virt-v2v -v -x ...' unless you yourself print it.

+ better way to share code with customize_run.ml ? guest_pkgs_command() is lifted from there

In this case the code is different, see comment below. If this was a very useful & self-contained piece of code then we could export it from the Customize_run module, but that doesn't seem to be the case here. In general it's fine to copy small bits of random code that aren't very reusable.

+ should I instead change Linux.remove impl, or should I aim to remove that code entirely?

If Linux.remove is now dead (unused anywhere else) then it should be removed. Note you'll need to check all projects which use the common/ git submodule, namely libguestfs & guestfs-tools & virt-v2v.

convert/convert_linux.ml | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml index 3d0e2b88..2e0cf61b 100644 --- a/convert/convert_linux.ml +++ b/convert/convert_linux.ml @@ -131,6 +131,18 @@ let convert (g : G.guestfs) source inspect i_firmware _ keep_serial_console _ = (*----------------------------------------------------------------------*) (* Conversion step. *)

+ let guest_pkgs_command f = + try f (g#inspect_get_package_management inspect.i_root) with

Using g#inspect_get_package_management will call into libguestfs to re-get the package_management field from this struct inside the libguestfs appliance / daemon: https://github.com/libguestfs/libguestfs/blob/0dee44bd369c59da2bd0aa531d98d9... which is fine, but also a lot of work. It turns out we've already cached this field in a local struct, 'inspect.i_package_management'. The i_root parameter is not needed as we've already decided here which root filesystem we're going to convert. That's controlled by the virt-v2v --root command line flag. Virt-v2v can only convert one root (eg for dual-boot guests) so once we know what that root is, we fetch all the inspection data for that root into a local struct 'inspect'.

+ | Guest_packages.Unknown_package_manager msg + | Guest_packages.Unimplemented_package_manager msg -> + error "%s" msg

These exceptions can be thrown by Guest_packages.uninstall_command below, so you'll need to catch them. But I think this function (guest_pkgs_command) effectively disappears so the try/with clause moves into the next function:

+ in + + let uninstall_packages pkgs = + let cmd = guest_pkgs_command(Guest_packages.uninstall_command pkgs) in

guest_pkgs_command goes away, but instead you need to catch the two exceptions that Guest_packages.uninstall_command can throw here.

+ ignore (g#sh cmd);

To be clear what's happening with this line of code: 'ignore' is an internal OCaml function that throws away the normal result of the command. In this case it throws away stdout (a string) which was captured when running the command. If the command fails (ie. returns a non-zero exit code) then g#sh throws an exception 'Guestfs.Error' (with a string parameter which is the error message, stderr of the command in this case). Therefore to ignore the error case you have to do something like: (try ignore (g#sh cmd) with Guestfs.Error msg -> warning (f_"could not uninstall packages ‘%s’: %s (ignored)") list_of_packages msg ) There are lots of examples of this if you search for 'Guestfs.Error' in the code.

+ in + let rec do_convert () = augeas_grub_configuration ();

@@ -237,7 +249,7 @@ let convert (g : G.guestfs) source inspect i_firmware _ keep_serial_console _ = else None ) inspect.i_apps in - Linux.remove g inspect.i_root xenmods; + uninstall_packages xenmods;

(* Undo related nastiness if kmod-xenpv was installed. *) if xenmods <> [] then ( @@ -310,7 +322,7 @@ let convert (g : G.guestfs) source inspect i_firmware _ keep_serial_console _ = fun { G.app2_name = name } -> name = package_name ) inspect.i_apps in if has_guest_additions then - Linux.remove g inspect.i_root [package_name]; + uninstall_packages [package_name];

(* Guest Additions might have been installed from a tarball. The * above code won't detect this case. Look for the uninstall tool @@ -455,8 +467,7 @@ let convert (g : G.guestfs) source inspect i_firmware _ keep_serial_console _ = ) );

- let remove = !remove in - Linux.remove g inspect.i_root remove; + uninstall_packages !remove;

(* VMware Tools may have been installed from a tarball, so the * above code won't remove it. Look for the uninstall tool and run @@ -503,7 +514,7 @@ let convert (g : G.guestfs) source inspect i_firmware _ keep_serial_console _ = let pkgs = List.map (fun { G.app2_name = name } -> name) pkgs in

if pkgs <> [] then ( - Linux.remove g inspect.i_root pkgs; + uninstall_packages pkgs;

(* Installing these guest utilities automatically unconfigures * ttys in /etc/inittab if the system uses it. We need to put -- 2.48.1

Rest looks fine. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html