In libguestfs we didn't bother to check the return values from any librpm calls. In some cases where possibly the RPM database is faulty, this caused us to return a zero-length list of installed applications (but no error indication). Libguestfs has subsequently been fixed so now it returns an error if the RPM database is corrupt. This commit changes virt-v2v behaviour so that if either guestfs_inspect_list_applications2 returns a zero-length list (ie. old libguestfs) or it throws an error (new libguestfs) then we attempt to rebuild the RPM database and retry the operation. Rebuilding the database can recover from some but not all RPM DB corruption. See-also: https://bugzilla.redhat.com/show_bug.cgi?id=2089623#c12

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2089623 Reported-by: Xiaodai Wang Reported-by: Ming Xie --- convert/inspect_source.ml | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/convert/inspect_source.ml b/convert/inspect_source.ml index 1736009629..16058de644 100644 --- a/convert/inspect_source.ml +++ b/convert/inspect_source.ml @@ -34,6 +34,7 @@ let rec inspect_source root_choice g = reject_if_not_installed_image g root; let typ = g#inspect_get_type root in + let package_format = g#inspect_get_package_format root in (* Mount up the filesystems. *) let mps = g#inspect_get_mountpoints root in @@ -71,7 +72,7 @@ let rec inspect_source root_choice g = ) mps; (* Get list of applications/packages installed. *) - let apps = g#inspect_list_applications2 root in + let apps = list_applications g root package_format in let apps = Array.to_list apps in (* A map of app2_name -> application2, for easier lookups. Note @@ -106,7 +107,7 @@ let rec inspect_source root_choice g = i_arch = g#inspect_get_arch root; i_major_version = g#inspect_get_major_version root; i_minor_version = g#inspect_get_minor_version root; - i_package_format = g#inspect_get_package_format root; + i_package_format = package_format; i_package_management = g#inspect_get_package_management root; i_product_name = g#inspect_get_product_name root; i_product_variant = g#inspect_get_product_variant root; @@ -186,6 +187,27 @@ and reject_if_not_installed_image g root = if fmt <> "installed" then error (f_"libguestfs thinks this is not an installed operating system (it might be, for example, an installer disk or live CD). If this is wrong, it is probably a bug in libguestfs. root=%s fmt=%s") root fmt +(* Wrapper around g#inspect_list_applications2 which, for RPM + * guests, on failure tries to rebuild the RPM database before + * repeating the operation. + *) +and list_applications g root = function + | "rpm" -> + (* RPM guest. *) + (try

+ let apps = g#inspect_list_applications2 root in + if apps = [||] then raise (G.Error "no applications returned"); + apps + with G.Error msg -> + debug "%s" msg; + debug "rebuilding RPM database and retrying ..."; + ignore (g#sh "rpmdb --rebuilddb"); + g#inspect_list_applications2 root + ) + | _ -> + (* Non-RPM guest, just do it. *) + g#inspect_list_applications2 root + (* See if this guest could use UEFI to boot. It should use GPT and * it should have an EFI System Partition (ESP). *

On Thu, May 26, 2022 at 10:53:59AM +0200, Laszlo Ersek wrote: > On 05/25/22 18:02, Richard W.M. Jones wrote: >> In libguestfs we didn't bother to check the return values from any >> librpm calls. In some cases where possibly the RPM database is >> faulty, this caused us to return a zero-length list of installed >> applications (but no error indication). Libguestfs has subsequently >> been fixed so now it returns an error if the RPM database is corrupt. >> >> This commit changes virt-v2v behaviour so that if either >> guestfs_inspect_list_applications2 returns a zero-length list (ie. old >> libguestfs) or it throws an error (new libguestfs) then we attempt to >> rebuild the RPM database and retry the operation. Rebuilding the >> database can recover from some but not all RPM DB corruption. >> >> See-also: https://bugzilla.redhat.com/show_bug.cgi?id=2089623#c12 > > What an absolutely horrific error mode. Great job debugging it! > >> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2089623 >> Reported-by: Xiaodai Wang >> Reported-by: Ming Xie >> --- >> convert/inspect_source.ml | 26 ++++++++++++++++++++++++-- >> 1 file changed, 24 insertions(+), 2 deletions(-) >> >> diff --git a/convert/inspect_source.ml b/convert/inspect_source.ml >> index 1736009629..16058de644 100644 >> --- a/convert/inspect_source.ml >> +++ b/convert/inspect_source.ml >> @@ -34,6 +34,7 @@ let rec inspect_source root_choice g = >> reject_if_not_installed_image g root; >> >> let typ = g#inspect_get_type root in >> + let package_format = g#inspect_get_package_format root in >> >> (* Mount up the filesystems. *) >> let mps = g#inspect_get_mountpoints root in >> @@ -71,7 +72,7 @@ let rec inspect_source root_choice g = >> ) mps; >> >> (* Get list of applications/packages installed. *) >> - let apps = g#inspect_list_applications2 root in >> + let apps = list_applications g root package_format in >> let apps = Array.to_list apps in >> >> (* A map of app2_name -> application2, for easier lookups. Note >> @@ -106,7 +107,7 @@ let rec inspect_source root_choice g = >> i_arch = g#inspect_get_arch root; >> i_major_version = g#inspect_get_major_version root; >> i_minor_version = g#inspect_get_minor_version root; >> - i_package_format = g#inspect_get_package_format root; >> + i_package_format = package_format; >> i_package_management = g#inspect_get_package_management root; >> i_product_name = g#inspect_get_product_name root; >> i_product_variant = g#inspect_get_product_variant root; >> @@ -186,6 +187,27 @@ and reject_if_not_installed_image g root = >> if fmt <> "installed" then >> error (f_"libguestfs thinks this is not an installed operating system (it might be, for example, an installer disk or live CD). If this is wrong, it is probably a bug in libguestfs. root=%s fmt=%s") root fmt >> >> +(* Wrapper around g#inspect_list_applications2 which, for RPM >> + * guests, on failure tries to rebuild the RPM database before >> + * repeating the operation. >> + *) >> +and list_applications g root = function >> + | "rpm" -> >> + (* RPM guest. *) >> + (try > > [*] > >> + let apps = g#inspect_list_applications2 root in >> + if apps = [||] then raise (G.Error "no applications returned"); >> + apps >> + with G.Error msg -> >> + debug "%s" msg; >> + debug "rebuilding RPM database and retrying ..."; >> + ignore (g#sh "rpmdb --rebuilddb"); >> + g#inspect_list_applications2 root >> + ) >> + | _ -> >> + (* Non-RPM guest, just do it. *) >> + g#inspect_list_applications2 root >> + >> (* See if this guest could use UEFI to boot. It should use GPT and >> * it should have an EFI System Partition (ESP). >> * >> > > The commit message explains well why the "g#inspect_list_applications2" > method call that is at the top of each "match" pattern cannot be > factored out (to a common call just before the "match"). However, > looking at the code, it's not easy to understand. > > Can you please: > > (1) Commit the libguestfs patch, > > (2) insert a comment at [*], saying that either of the two lines just > below may raise an exception, and which one does depends on libguestfs > having the commit <HASH> from point (1)? > > With that: > > Reviewed-by: Laszlo Ersek <lersek(a)redhat.com&gt; Sure thing, thanks! So ... libguestfs commit: 488245ed6c0c5db282ec7fed646e8bc00ce0d487 virt-v2v commit with additional commentary referencing libguestfs commit: 31bf5db25bcfd8a9f5a48cc0523abae28861de9a