[Libguestfs] [PATCH] v2v: Fix RPM file owned test (RHBZ#1503958).

[Pino Toscano]

On Thursday, 19 October 2017 13:08:38 CEST Richard W.M. Jones wrote:
> Linux.file_owner is not used by any other function, so remove it.
> 
> Linux.is_file_owned is only used when removing kmod-xenpv on old RHEL
> releases, and so is only required to work for RPM.
> 
> The old file_owner/is_file_owned functions were completely broken.
> This replaces them with a simpler, working implementation that only
> covers the narrow use case of removing kmod-xenpv non-owned
> directories.

To be precise: only the RPM implementation did not work on unowned
files.  The dpkg implementation, added as part of the work to support
Debian/Ubuntu guests in v2v, works in both the cases.

> diff --git a/v2v/linux.ml b/v2v/linux.ml
> index bc4af1ad2..d759bf7e6 100644
> --- a/v2v/linux.ml
> +++ b/v2v/linux.ml
> @@ -99,58 +99,26 @@ let file_list_of_package (g : Guestfs.guestfs) inspect app =
>      error (f_"don’t know how to get list of files from package using %s")
>        format
>  
> -let rec file_owner (g : G.guestfs) { i_package_format = package_format } path =
> +let is_file_owned (g : G.guestfs) { i_package_format = package_format } path =
>    match package_format with
> -  | "deb" ->
> -      (* With dpkg usually the directories are owned by all the packages
> -       * that install anything in them.  Also with multiarch the same
> -       * package is allowed (although with different architectures).
> -       * This function returns only one package in all the cases.
> -       *)
> -      let cmd = [| "dpkg"; "-S"; path |] in
> -      debug "%s" (String.concat " " (Array.to_list cmd));
> -      let lines =
> -        try g#command_lines cmd
> -        with Guestfs.Error msg as exn ->
> -          if String.find msg "no path found matching pattern" >= 0 then
> -            raise Not_found
> -          else
> -            raise exn in
> -      if Array.length lines = 0 then
> -        error (f_"internal error: file_owner: dpkg command returned no output");
> -      let line = lines.(0) in
> -      let line =
> -        try String.sub line 0 (String.rindex line ':')
> -        with Invalid_argument _ ->
> -          error (f_"internal error: file_owner: invalid dpkg output: ‘%s’")
> -                line in
> -      fst (String.split "," line)
> -

I'd still leave the dpkg implementation, since code in v2v is not
specific to RPM-based systems.  It can be slightly simplified:

  let cmd = [| "dpkg"; "-S"; path |] in
  debug "%s" (String.concat " " (Array.to_list cmd));
  let lines =
    try g#command_lines cmd
    with Guestfs.Error msg as exn ->
      if String.find msg "no path found matching pattern" >= 0 then
        false
      else
        raise exn in
  if Array.length lines = 0 then
    error (f_"internal error: file_owner: dpkg command returned no output")
  else
    true

>    | "rpm" ->
> -      (* Although it is possible in RPM for multiple packages to own
> -       * a file, this deliberately only returns one package.
> -       *)
> -      let cmd = [| "rpm"; "-qf"; "--qf"; "%{NAME}\\n"; path |] in
> -      debug "%s" (String.concat " " (Array.to_list cmd));
> -      (try
> -         let pkgs = g#command_lines cmd in
> -         pkgs.(0)
> -       with Guestfs.Error msg as exn ->
> -         if String.find msg "is not owned" >= 0 then
> -           raise Not_found
> -         else
> -           raise exn
> -       | Invalid_argument _ (* pkgs.(0) raises index out of bounds *) ->
> -         error (f_"internal error: file_owner: rpm command returned no output")
> -      )
> +     (* Run rpm -qf and print a magic string if the file is owned.
> +      * If not owned, rpm will print "... is not owned by any package"
> +      * and exit with an error.  Unfortunately the string is sent to
> +      * stdout, so here we ignore the exit status of rpm and just
> +      * look for one of the two strings.
> +      *)
> +     let magic = "FILE_OWNED_TEST" in
> +     let cmd = sprintf "rpm -qf --qf %s %s 2>&1 ||:"
> +                       (quote (magic ^ "\n")) (quote path) in
> +     let r = g#sh cmd in
> +     if String.find r magic >= 0 then true
> +     else if String.find r "is not owned" >= 0 then false
> +     else failwithf "RPM file owned test failed: %s" r

Another option could be redirecting stdout to stderr, and assuming
that a successful run means the file is owned.  Something like the
following untested:

  let cmd = sprintf "rpm -qf --qf FOUND %s >&2" (quote path) in
  debug "%s" cmd;
  (try
     ignore (g#sh cmd);
     true
   with Guestfs.Error msg as exn ->
     if String.find msg "is not owned" >= 0 then
       false
     else
       raise exn
  )

This way, we do not need to ignore the RPM return value in all the
cases, and just let it raise the exception.

-- 
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/20171019/95a4502e/attachment.sig>