[Libguestfs] [PATCH] v2v: fixed file_owner function
Richard W.M. Jones
rjones at redhat.com
Wed Aug 3 11:07:10 UTC 2016
On Wed, Aug 03, 2016 at 12:51:24PM +0200, Tomáš Golembiovský wrote:
> On Wed, 3 Aug 2016 11:14:26 +0100
> "Richard W.M. Jones" <rjones at redhat.com> wrote:
>
> > On Mon, Aug 01, 2016 at 02:46:14PM +0200, Tomáš Golembiovský wrote:
> > > What was happening in file_owner function did not match the description
> > > in the comment. When a path is owned by multiple packages the returned
> > > string was in fact a concatenation of the names of all packages that own
> > > it. E.g. for `Linux.is_file_owned g inspect "/etc"` the returned value
> > > was "filesystemyum" (i.e. "filesystem" + "yum").
> > >
> > > Signed-off-by: Tomáš Golembiovský <tgolembi at redhat.com>
> > > ---
> > > v2v/linux.ml | 8 ++++++--
> > > 1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/v2v/linux.ml b/v2v/linux.ml
> > > index d20194b..aeff5c5 100644
> > > --- a/v2v/linux.ml
> > > +++ b/v2v/linux.ml
> > > @@ -99,14 +99,18 @@ let rec file_owner g inspect path =
> > > (* 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}"; path |] in
> > > + let cmd = [| "rpm"; "-qf"; "--qf"; "%{NAME}\\n"; path |] in
> > > debug "%s" (String.concat " " (Array.to_list cmd));
> > > - (try g#command 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 msg ->
> > > + raise Not_found
> >
> > My concern/confusion about this patch is what raises Invalid_argument?
> >
> > I'm assuming it's the array index, which could raise `Invalid_argument
> > "index out of bounds"' if the g#command_lines returns a zero-length
> > list, but then my question would be why is the rpm command not
> > returning any lines?
> >
> > Even if this can happen, the exception shouldn't be converted to
> > Not_found. It should be an internal error or the exception should be
> > left unchanged.
>
> Yes, it is to catch the case when rpm reports nothing. Don't know
> if/when this can happen. Do you prefer an error in the exception handler
> (instead of raise Not_found), or should I check the array length (and
> report error) before accessing it?
Since this should never happen, something like this:
| Invalid_argument "index out of bounds" ->
error (f_"internal error: file_owner: rpm command returned no output")
which will immediately exit with a clear error if this case happens.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
More information about the Libguestfs
mailing list