[Libguestfs] [PATCH virt-v2v] convert: If listing RPM applications fails, rebuild DB and retry
Laszlo Ersek
lersek at redhat.com
Thu May 26 13:06:49 UTC 2022
On 05/26/22 11:23, Richard W.M. Jones wrote:
> 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 at redhat.com>
>
> Sure thing, thanks!
>
> So ...
>
> libguestfs commit: 488245ed6c0c5db282ec7fed646e8bc00ce0d487
>
> virt-v2v commit with additional commentary referencing libguestfs
> commit: 31bf5db25bcfd8a9f5a48cc0523abae28861de9a
Thank you, that's a very good comment in there.
Laszlo
More information about the Libguestfs
mailing list