[Libguestfs] [PATCH] v2v: linux: use NEVR for querying RPM packages (RHBZ#1669395)

[Pino Toscano]

On Wednesday, 30 January 2019 12:02:11 CET Richard W.M. Jones wrote:
> On Tue, Jan 29, 2019 at 03:29:44PM +0100, Pino Toscano wrote:
> > Use NEVR when querying RPM for the list of files of a package, instead
> > of ENVR.  Also, use the epoch only when non-zero, and version of RPM
> > supports it.
> > 
> > The approach is basically copied from what supermin does in its RPM
> > package handler.
> > ---
> >  v2v/linux.ml | 52 +++++++++++++++++++++++++++++++---------------------
> >  1 file changed, 31 insertions(+), 21 deletions(-)
> > 
> > diff --git a/v2v/linux.ml b/v2v/linux.ml
> > index 43449157b..4cf498d29 100644
> > --- a/v2v/linux.ml
> > +++ b/v2v/linux.ml
> > @@ -80,29 +80,39 @@ let file_list_of_package (g : Guestfs.guestfs) inspect app =
> >  
> >    | "rpm" ->
> >      (* Since RPM allows multiple packages installed with the same
> > -     * name, always check the full ENVR here (RHBZ#1161250).
> > +     * name, always check the full NEVR here (RHBZ#1161250).
> > +     *
> > +     * In RPM < 4.11 query commands that use the epoch number in the
> > +     * package name did not work.
> > +     *
> > +     * For example:
> > +     * RHEL 6 (rpm 4.8.0):
> > +     *   $ rpm -q tar-2:1.23-11.el6.x86_64
> > +     *   package tar-2:1.23-11.el6.x86_64 is not installed
> > +     * Fedora 20 (rpm 4.11.2):
> > +     *   $ rpm -q tar-2:1.26-30.fc20.x86_64
> > +     *   tar-1.26-30.fc20.x86_64
> >       *)
> > +    let rpm_major, rpm_minor =
> > +      let ver = List.find_map (
> > +        function
> > +        | { G.app2_name = name; G.app2_version = version }
> > +            when name = "rpm" -> Some version
> > +        | _ -> None
> > +      ) inspect.i_apps in
> 
> This has the nasty side effect of Not_found exception escaping if for
> some reason we can't find the rpm version.  I think you should catch
> that case and assume old RPM.

More than "can't find the rpm version", the loop looks for rpm in the
list of installed application.  I can add an error handling, although
I don't think it will happen that an RPM-based distro (detected as such
by the inspection code) will not have the "rpm" package installed...

> > +      match String.nsplit "." ver with
> > +      | [major] -> int_of_string major, 0
> > +      | major :: minor :: _ -> int_of_string major, int_of_string minor
> > +      | _ -> error (f_"unrecognized RPM version: %s") ver in
> 
> Because this comes from the guest the version field could contain any
> old stuff, making this a bit error prone.  I think it would also be
> nice if we didn't error out in the last case.  Something like this
> seems better to me:
> 
> let re_rpm_version = PCRE.compile "(\\d+)\\.(\\d+)"
> ...
> 
>   let ver =
>     if PCRE.matches re_rpm_version ver then
>       (int_of_string (PCRE.sub 1), int_of_string (PCRE.sub 2))
>     else
>       (0, 0) in
>   let is_rpm_lt_4_11 = ver < (4, 11) in
> 
> (We already have this sort of code in daemon/inspect_utils.ml, but we
> can't reuse it).

I wanted to avoid running a regex every time RPM is queries for the
list of files in a package.  Nevertheless, I took the approach, making
it used only when the epoch is not zero.

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190130/967823c4/attachment.sig>