Libguestfs - Jun 2010 - [PATCH] RHEV: Warn instead of die if rmtree dies during cleanup

rmtree can die instead of returning failure under some circumstances. We
don't
want this to stop cleanup.
---
 lib/Sys/VirtV2V/Target/RHEV.pm |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/lib/Sys/VirtV2V/Target/RHEV.pm b/lib/Sys/VirtV2V/Target/RHEV.pm
index 295c19d..c9add0c 100644
--- a/lib/Sys/VirtV2V/Target/RHEV.pm
+++ b/lib/Sys/VirtV2V/Target/RHEV.pm
@@ -390,9 +390,18 @@ sub _cleanup
 
     return unless (defined($tmpdir));
 
-    rmtree($tmpdir) or warn(user_message(__x("Unable to remove temporary
".
-                                            "directory {dir}",
-                                            dir => $tmpdir)));
+    eval {
+        rmtree($tmpdir) or warn(user_message(__x("Unable to remove
temporary ".
+                                                 "directory {dir}",
+                                                 dir => $tmpdir)));
+    };
+
+    if ($@) {
+        warn(user_message(__x("Error removing temporary directory {dir}:
".
+                              "{error}",
+                              dir => $tmpdir, error => $@)));
+    }
+
     $tmpdir = undef;
 }
 
-- 
1.7.0.1

On Thu, Jun 10, 2010 at 04:43:58PM +0100, Matthew Booth
wrote:
> rmtree can die instead of returning failure under some circumstances. We
don't
> want this to stop cleanup.
> ---
>  lib/Sys/VirtV2V/Target/RHEV.pm |   15 ++++++++++++---
>  1 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/Sys/VirtV2V/Target/RHEV.pm
b/lib/Sys/VirtV2V/Target/RHEV.pm
> index 295c19d..c9add0c 100644
> --- a/lib/Sys/VirtV2V/Target/RHEV.pm
> +++ b/lib/Sys/VirtV2V/Target/RHEV.pm
> @@ -390,9 +390,18 @@ sub _cleanup
>  
>      return unless (defined($tmpdir));
>  
> -    rmtree($tmpdir) or warn(user_message(__x("Unable to remove
temporary ".
> -                                            "directory {dir}",
> -                                            dir => $tmpdir)));
> +    eval {
> +        rmtree($tmpdir) or warn(user_message(__x("Unable to remove
temporary ".
> +                                                 "directory
{dir}",
> +                                                 dir => $tmpdir)));
> +    };
> +
> +    if ($@) {
> +        warn(user_message(__x("Error removing temporary directory
{dir}: ".
> +                              "{error}",
> +                              dir => $tmpdir, error => $@)));
> +    }
> +
>      $tmpdir = undef;
>  }
>  
> -- 
> 1.7.0.1
It's a bit wierd having the error message effectively repeated twice,
but I understand why it is necessary.  Can we do something like this?

  eval { rmtree $tmpdir or die "rmtree returned an error" };
  if ($@) { warn ... }

Anyway, the patch is fine with or without this change, so ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v