Tomáš Golembiovský
2016-Aug-05 09:32 UTC
[Libguestfs] [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
Pino Toscano
2016-Aug-05 09:53 UTC
Re: [Libguestfs] [PATCH] v2v: do not hide the error, rather report it
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
Richard W.M. Jones
2016-Aug-05 11:25 UTC
Re: [Libguestfs] [PATCH] v2v: do not hide the error, rather report it
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
Pino Toscano
2016-Aug-08 09:03 UTC
Re: [Libguestfs] [PATCH] v2v: do not hide the error, rather report it
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
Possibly Parallel Threads
- [PATCH] v2v: fixed file_owner function
- Re: [PATCH] v2v: fixed file_owner function
- Re: [PATCH] v2v: do not hide the error, rather report it
- [PATCH] v2v: Fix RPM file owned test (RHBZ#1503958).
- Re: [PATCH 2/8] v2v: add basic support for the "deb" package manager