Libguestfs - Aug 2016 - Re: [PATCH] v2v: do not hide the error, rather report it

The Invalid_argument exception is there to catch unexpected situation
when rpm returns no output. Such situation should be reported rather
then hidden.

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 v2v/linux.ml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/v2v/linux.ml b/v2v/linux.ml
index e57dad6..46cb3ba 100644
--- a/v2v/linux.ml
+++ b/v2v/linux.ml
@@ -109,8 +109,8 @@ let rec file_owner (g : G.guestfs) inspect path            
raise Not_found
          else
            raise exn
-       | Invalid_argument msg ->
-         raise Not_found
+       | Invalid_argument "index out of bounds" ->
+         error (f_"internal error: file_owner: rpm command returned no
output")
       )
 
   | format ->
-- 
2.9.2

On Friday, 5 August 2016 11:32:27 CEST Tomáš Golembiovský
wrote:
> The Invalid_argument exception is there to catch unexpected situation
> when rpm returns no output. Such situation should be reported rather
> then hidden.
> 
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
>  v2v/linux.ml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/v2v/linux.ml b/v2v/linux.ml
> index e57dad6..46cb3ba 100644
> --- a/v2v/linux.ml
> +++ b/v2v/linux.ml
> @@ -109,8 +109,8 @@ let rec file_owner (g : G.guestfs) inspect path >   
raise Not_found
>           else
>             raise exn
> -       | Invalid_argument msg ->
> -         raise Not_found
> +       | Invalid_argument "index out of bounds" ->
> +         error (f_"internal error: file_owner: rpm command returned
no output")
At this point, IMHO it would make more sense to split the different
operations in different try/with blocks, to distinguish better the
errors; something like:

  let pkgs     try
      pkgs = g#command_lines cmd
    with Guestfs.Error msg as exn ->
      if String.find msg "is not owned" >= 0 then
        raise Not_found
      else
        raise exn in
  if Array.length = 0 then
    error (f_"internal error: file_owner: rpm command returned no
output");
  pkgs.(0)

-- 
Pino Toscano

On Fri, Aug 05, 2016 at 11:32:27AM +0200, Tomáš Golembiovský
wrote:
> The Invalid_argument exception is there to catch unexpected situation
> when rpm returns no output. Such situation should be reported rather
> then hidden.
> 
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
>  v2v/linux.ml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/v2v/linux.ml b/v2v/linux.ml
> index e57dad6..46cb3ba 100644
> --- a/v2v/linux.ml
> +++ b/v2v/linux.ml
> @@ -109,8 +109,8 @@ let rec file_owner (g : G.guestfs) inspect path >   
raise Not_found
>           else
>             raise exn
> -       | Invalid_argument msg ->
> -         raise Not_found
> +       | Invalid_argument "index out of bounds" ->
> +         error (f_"internal error: file_owner: rpm command returned
no output")
>        )
Unlike Pino I'm actually OK with this fix.  It's up to you if
you want to try it Pino's way.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

On Friday, 5 August 2016 12:25:03 CEST Richard W.M. Jones
wrote:
> On Fri, Aug 05, 2016 at 11:32:27AM +0200, Tomáš Golembiovský wrote:
> > The Invalid_argument exception is there to catch unexpected situation
> > when rpm returns no output. Such situation should be reported rather
> > then hidden.
> > 
> > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> > ---
> >  v2v/linux.ml | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/v2v/linux.ml b/v2v/linux.ml
> > index e57dad6..46cb3ba 100644
> > --- a/v2v/linux.ml
> > +++ b/v2v/linux.ml
> > @@ -109,8 +109,8 @@ let rec file_owner (g : G.guestfs) inspect path
> >             raise Not_found
> >           else
> >             raise exn
> > -       | Invalid_argument msg ->
> > -         raise Not_found
> > +       | Invalid_argument "index out of bounds" ->
> > +         error (f_"internal error: file_owner: rpm command
returned no output")
> >        )
> 
> Unlike Pino I'm actually OK with this fix.  It's up to you if
> you want to try it Pino's way.
OK, pushed this as is then.

Thanks,
-- 
Pino Toscano