[PATCH] RHEV: Warn instead of die if rmtree dies during cleanup
by Matthew Booth
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
14 years, 6 months
Re: [Libguestfs] [Bug 602599] New: [RFE] A new API for custom cleanup prior to close in perl bindings
by Richard W.M. Jones
On Thu, Jun 10, 2010 at 05:34:23AM -0400, bugzilla(a)redhat.com wrote:
>
> Summary: [RFE] A new API for custom cleanup prior to close in perl bindings
>
> https://bugzilla.redhat.com/show_bug.cgi?id=602599
>
> Summary: [RFE] A new API for custom cleanup prior to close in
> perl bindings
> Product: Virtualization Tools
>
> If the perl bindings introduce an explicit close API, it will no
> longer be safe to rely on object destruction to call cleanup
> code. For example, if a program creates a temporary directory which
> must persist for the duration of the program but must be cleaned up
> prior to exit, the program cannot rely on a destructor to do the
> cleanup as the handle may have been closed before the destructor is
> executed.
>
> A solution to this would be to allow callbacks to be registered and
> executed immediately prior to close(). For example:
>
> $g->pre_close(sub {
> $g->rmdir($self->{foo});
> });
Unfortunately although this looks temptingly simple, it doesn't work
for a variety of reasons.
(1) $g is captured by the closure and so prevents implicit closure.
Of course you can explicitly close the handle now, but it's still very
counter-intuitive.
(2) Even if you explicitly close the handle, the closure will still
capture $g and prevent the handle from being freed (the reasons are
this are quite subtle -- see perlcall(3) discussion of call_sv for the
full details).
(3) Whatever you do, you end up with a static array in the XS code
storing SVs which has all sorts of horrible thread safety issues.
Again, see perlcall(3) for the details.
(4) guestfs_close can be called from exit (via an atexit handler) and
it's not at all clear that the Perl interpreter won't have been shut
down by the time we try to invoke the closure, resulting in hilarity.
Since it's now possible to store user data in the Perl handle, I
suggest pursuing a Perl-based solution to all this.
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
14 years, 6 months
[PATCH] Fix cleanup of guest handle when installing with local files
by Matthew Booth
Change 13412f7629133b25fda32c35fdb0276e22de3445 caused Config to keep a
reference to the guestfs handle in certain circumstances. This caused a
regression in the cleanup code because the handle was no longer closed in
close_guest_handle.
This change explicitly drops the reference to $config before closing the guest
handle. It also localises $guestos, which also keeps a reference to the guestfs
handle in an eval block.
---
v2v/virt-v2v.pl | 38 ++++++++++++++++++++++++--------------
1 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/v2v/virt-v2v.pl b/v2v/virt-v2v.pl
index 726cd50..9008ad6 100755
--- a/v2v/virt-v2v.pl
+++ b/v2v/virt-v2v.pl
@@ -370,16 +370,29 @@ if ($output_method eq 'rhev') {
$> = "0";
}
-# Inspect the guest
-my $os = inspect_guest($g);
+my $os;
+my $guestcaps;
+eval {
+ # Inspect the guest
+ $os = inspect_guest($g);
+
+ # Instantiate a GuestOS instance to manipulate the guest
+ my $guestos = Sys::VirtV2V::GuestOS->new($g, $os, $dom, $config);
-# Instantiate a GuestOS instance to manipulate the guest
-my $guestos = Sys::VirtV2V::GuestOS->new($g, $os, $dom, $config);
+ # Modify the guest and its metadata
+ $guestcaps = Sys::VirtV2V::Converter->convert($g, $guestos,
+ $config, $dom, $os,
+ $conn->get_storage_devices());
+};
-# Modify the guest and its metadata
-my $guestcaps = Sys::VirtV2V::Converter->convert($g, $guestos,
- $config, $dom, $os,
- $conn->get_storage_devices());
+# If any of the above commands result in failure, we need to ensure that the
+# guestfs qemu process is cleaned up before further cleanup. Failure to do this
+# can result in failure to umount RHEV export's temporary mount point.
+if ($@) {
+ my $err = $@;
+ close_guest_handle();
+ die($err);
+}
close_guest_handle();
@@ -397,11 +410,8 @@ if($guestcaps->{virtio}) {
exit(0);
-# We should always attempt to shut down the guest gracefully
+# die() sets $? to 255, which is untidy.
END {
- close_guest_handle();
-
- # die() sets $? to 255, which is untidy.
$? = $? == 255 ? 1 : $?;
}
@@ -410,8 +420,8 @@ END {
sub close_guest_handle
{
- # Perform GuestOS cleanup before closing the handle
- $guestos = undef;
+ # Config may cache the guestfs handle, preventing cleanup below
+ $config = undef;
if (defined($g)) {
my $retval = $?;
--
1.7.0.1
14 years, 6 months
[PATCH] RHEV: Fix generation of OVF file
by Matthew Booth
From: Qixiang Wan <qwan(a)redhat.com>
Commit 0973765674abd773ad04a99ddfdff8e81693e1ff introduced a regression which
caused the OVF file not to be generated when outputting to RHEV.
Fixes RHBZ#602067
---
lib/Sys/VirtV2V/Target/RHEV.pm | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lib/Sys/VirtV2V/Target/RHEV.pm b/lib/Sys/VirtV2V/Target/RHEV.pm
index 0fec988..eac15a9 100644
--- a/lib/Sys/VirtV2V/Target/RHEV.pm
+++ b/lib/Sys/VirtV2V/Target/RHEV.pm
@@ -726,7 +726,7 @@ EOF
dir => $dir,
error => $!)));
- return Sys::VirtV2V::Target::RHEV::Vol->_move_vols();
+ Sys::VirtV2V::Target::RHEV::Vol->_move_vols();
my $vm;
my $ovfpath = $dir.'/'.$vmuuid.'.ovf';
--
1.7.0.1
14 years, 6 months
[PATCH] Fix cleanup if Ctrl-C kills guestfs qemu process
by Matthew Booth
If a user kills v2v with Ctrl-C during the conversion stage, there is a high
likelihood that the qemu process will die before v2v cleanup completes. When
this happens, the unmount_all and sync calls will fail. This causes all further
cleanup to be aborted, which is undesirable.
Additional fix to RHBZ#596015
---
v2v/virt-v2v.pl | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/v2v/virt-v2v.pl b/v2v/virt-v2v.pl
index 1ab8fdc..726cd50 100755
--- a/v2v/virt-v2v.pl
+++ b/v2v/virt-v2v.pl
@@ -414,11 +414,18 @@ sub close_guest_handle
$guestos = undef;
if (defined($g)) {
- $g->umount_all();
- $g->sync();
-
my $retval = $?;
+ eval {
+ $g->umount_all();
+ $g->sync();
+ };
+ if ($@) {
+ warn(user_message(__x("Error cleaning up guest handle: {error}",
+ error => $@)));
+ $retval ||= 1;
+ }
+
# Note that this undef is what actually causes the underlying handle to
# be closed. This is required to allow the RHEV target's temporary mount
# directory to be unmounted and deleted prior to exit.
--
1.7.0.1
14 years, 6 months