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(a)redhat.com&gt; --- 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?

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

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

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

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

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

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

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

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

+ ignore (g#sh cmd);

+ 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

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(a)redhat.com&gt; > --- > > 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/9fc97d621a2e0bc1d7f7b369f8252... > + 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/0dee44bd369c59da2bd0aa531d9... 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.