[Libguestfs] [PATCH] Improve cleanup of libguestfs handle with Sys::VirtV2V::GuestfsHandle
Matthew Booth
mbooth at redhat.com
Thu Jun 17 16:05:09 UTC 2010
This change replaces all direct usage of a Sys::Guestfs object in virt-v2v with
a Sys::VirtV2V::GuestfsHandle proxy object. The proxy does 3 things:
* Holds handle initialisation code
* Adds the ability to explicitly close the handle
* Adds the ability to register a callback to be executed before close
The cleanup code in Sys::VirtV2V::Config is updated to use the new callback
mechanism.
Additionally, the initialisation code improves handling of failure while
virt-v2v is running with uid:gid == 36:36. If cleanup handlers are called in
this context they can fail due to insufficient permissions. If this code dies,
permissions will be restored before the die is propagated. If this code receives
a user signal it will be postponed until after the code has completed.
---
MANIFEST | 1 +
lib/Sys/VirtV2V/Config.pm | 22 +---
lib/Sys/VirtV2V/GuestfsHandle.pm | 208 ++++++++++++++++++++++++++++++++++++++
v2v/virt-v2v.pl | 72 ++------------
4 files changed, 223 insertions(+), 80 deletions(-)
create mode 100644 lib/Sys/VirtV2V/GuestfsHandle.pm
diff --git a/MANIFEST b/MANIFEST
index 06df2aa..fb6a2fc 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -4,6 +4,7 @@ COPYING
COPYING.LIB
lib/Sys/VirtV2V.pm
lib/Sys/VirtV2V/ExecHelper.pm
+lib/Sys/VirtV2V/GuestfsHandle.pm
lib/Sys/VirtV2V/GuestOS.pm
lib/Sys/VirtV2V/GuestOS/RedHat.pm
lib/Sys/VirtV2V/Config.pm
diff --git a/lib/Sys/VirtV2V/Config.pm b/lib/Sys/VirtV2V/Config.pm
index 3f689e0..761bee5 100644
--- a/lib/Sys/VirtV2V/Config.pm
+++ b/lib/Sys/VirtV2V/Config.pm
@@ -233,10 +233,13 @@ sub get_transfer_path
my $transfer = $devices[$#devices];
$g->mount_ro($transfer, $self->{transfer_mount});
- $self->{transfer_mounted} = 1;
- # We'll need this to unmount in DESTROY
- $self->{g} = $g;
+ # Umount and remove the transfer mount point before the guestfs
+ # handle is closed
+ $g->add_on_close(sub {
+ $g->umount($self->{transfer_mount});
+ $g->rmdir($self->{transfer_mount});
+ });
}
}
@@ -498,19 +501,6 @@ sub set_default_net_mapping
$self->{default_net_mapping} = [ $name, $type ];
}
-sub DESTROY
-{
- my $self = shift;
-
- my $g = $self->{g};
-
- # Remove the transfer mount point if it was used
- $g->umount($self->{transfer_mount})
- if(defined($self->{transfer_mounted}));
- $g->rmdir($self->{transfer_mount})
- if(defined($self->{transfer_mount}));
-}
-
=back
=head1 COPYRIGHT
diff --git a/lib/Sys/VirtV2V/GuestfsHandle.pm b/lib/Sys/VirtV2V/GuestfsHandle.pm
new file mode 100644
index 0000000..8720744
--- /dev/null
+++ b/lib/Sys/VirtV2V/GuestfsHandle.pm
@@ -0,0 +1,208 @@
+# Sys::VirtV2V::GuestfsHandle
+# Copyright (C) 2010 Red Hat Inc.
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+package Sys::VirtV2V::GuestfsHandle;
+
+use strict;
+use warnings;
+
+use Carp;
+
+use Sys::Guestfs::Lib qw(open_guest);
+use Sys::VirtV2V::UserMessage qw(user_message);
+
+use Locale::TextDomain 'virt-v2v';
+
+=pod
+
+=head1 NAME
+
+Sys::VirtV2V::GuestfsHandle - Proxy Sys::Guestfs with custom close behaviour
+
+=head1 SYNOPSIS
+
+ use Sys::VirtV2V::GuestfsHandle;
+
+ my $g = new Sys::VirtV2V::GuestfsHandle($storage, $transferiso);
+
+ # GuestfsHandle proxies all Sys::Guestfs methods
+ print join("\n", $g->list_devices());
+
+ # GuestfsHandle adds 2 new methods
+ $g->add_on_close(sub { print "Bye!\n"; });
+ $g->close();
+
+=head1 DESCRIPTION
+
+Sys::VirtV2V::GuestfsHandle is a proxy to Sys::Guestfs which adds a custom
+close() method, and the ability to register pre-close callbacks.
+
+=head1 METHODS
+
+=over
+
+=item new(storage, transferiso, isrhev)
+
+Create a new object. Open a new Sys::Guestfs handle to proxy, using the disks
+defined in the array I<storage>. Add I<transferiso> as a read-only drive if it
+is given. If I<isrhev> is true, the handle will use user and group 36:36.
+
+=cut
+
+sub new
+{
+ my $class = shift;
+ my ($storage, $transfer, $isrhev) = @_;
+
+ my $self = {};
+
+ my $interface = "ide";
+
+ # Don't respond to signals while we're running setuid. Cleanup operations
+ # can fail if they run as the wrong user.
+ my $sigint = $SIG{'INT'};
+ my $sigquit = $SIG{'QUIT'};
+ my $sig_received;
+
+ $SIG{'INT'} = $SIG{'QUIT'} = sub {
+ $sig_received = shift;
+ };
+
+ if ($isrhev) {
+ $) = "36 36";
+ $> = "36";
+ }
+
+ # Open a guest handle
+ my $g;
+
+ eval {
+ $g = open_guest($storage, rw => 1, interface => $interface);
+
+ # Add the transfer iso if there is one
+ $g->add_drive_ro_with_if($transfer, $interface) if (defined($transfer));
+
+ $g->launch();
+ };
+ my $err = $@;
+
+ if ($isrhev) {
+ $) = "0 0";
+ $> = "0";
+ }
+
+ die($err) if ($err);
+
+ if (defined($sig_received)) {
+ &$sigint($sig_received) if ($sig_received eq 'INT');
+ &$sigquit($sig_received) if ($sig_received eq 'QUIT');
+ }
+ $SIG{'INT'} = $sigint;
+ $SIG{'QUIT'} = $sigquit;
+
+ # Enable autosync to defend against data corruption on unclean shutdown
+ $g->set_autosync(1);
+
+ $self->{g} = $g;
+
+ $self->{onclose} = [];
+
+ bless($self, $class);
+ return $self;
+}
+
+=item add_on_close
+
+Register a callback to be called before closing the underlying Sys::Guestfs
+handle.
+
+=cut
+
+sub add_on_close
+{
+ my $self = shift;
+
+ push(@{$self->{onclose}}, shift);
+}
+
+=item close
+
+Call all registered close callbacks, then close the Sys::Guestfs handle.
+
+=cut
+
+sub close
+{
+ my $self = shift;
+
+ my $g = $self->{g};
+
+ # Nothing to do if handle is already closed
+ return unless (defined($g));
+
+ foreach my $onclose (@{$self->{onclose}}) {
+ &$onclose();
+ }
+
+ my $retval = $?;
+ $? = 0;
+
+ # This will close the underlying libguestfs handle, which may affect $?
+ $self->{g} = undef;
+
+ warn(user_message(__x("libguestfs did not shut down cleanly")))
+ if ($? != 0);
+
+ $? = $retval;
+}
+
+our $AUTOLOAD;
+sub AUTOLOAD
+{
+ (my $methodname) = $AUTOLOAD =~ m/.*::(\w+)$/;
+
+ # We don't want to call DESTROY explicitly
+ return if ($methodname eq "DESTROY");
+
+ my $self = shift;
+ my $g = $self->{g};
+
+ croak("$methodname called on guestfs handle after handle was closed")
+ unless (defined($g));
+
+ return $g->$methodname(@_);
+}
+
+
+=back
+
+=head1 COPYRIGHT
+
+Copyright (C) 2010 Red Hat Inc.
+
+=head1 LICENSE
+
+Please see the file COPYING.LIB for the full license.
+
+=head1 SEE ALSO
+
+L<virt-v2v(1)>,
+L<http://libguestfs.org/>.
+
+=cut
+
+1;
diff --git a/v2v/virt-v2v.pl b/v2v/virt-v2v.pl
index 9008ad6..adaa9a9 100755
--- a/v2v/virt-v2v.pl
+++ b/v2v/virt-v2v.pl
@@ -25,7 +25,7 @@ use Getopt::Long;
use Locale::TextDomain 'virt-v2v';
use Sys::Guestfs;
-use Sys::Guestfs::Lib qw(open_guest get_partitions inspect_all_partitions
+use Sys::Guestfs::Lib qw(get_partitions inspect_all_partitions
inspect_operating_systems mount_operating_system
inspect_in_detail);
@@ -37,6 +37,7 @@ use Sys::VirtV2V::Connection::LibVirtXML;
use Sys::VirtV2V::Target::LibVirt;
use Sys::VirtV2V::Target::RHEV;
use Sys::VirtV2V::ExecHelper;
+use Sys::VirtV2V::GuestfsHandle;
use Sys::VirtV2V::GuestOS;
use Sys::VirtV2V::UserMessage qw(user_message);
@@ -357,18 +358,9 @@ my $storage = $conn->get_storage_paths();
my $transferiso;
$transferiso = $config->get_transfer_iso();
-if ($output_method eq 'rhev') {
- $) = "36 36";
- $> = "36";
-}
-
# Open a libguestfs handle on the guest's storage devices
-my $g = get_guestfs_handle($storage, $transferiso);
-
-if ($output_method eq 'rhev') {
- $) = "0";
- $> = "0";
-}
+my $g = new Sys::VirtV2V::GuestfsHandle($storage, $transferiso,
+ $output_method eq 'rhev');
my $os;
my $guestcaps;
@@ -390,11 +382,11 @@ eval {
# can result in failure to umount RHEV export's temporary mount point.
if ($@) {
my $err = $@;
- close_guest_handle();
+ $g->close();
die($err);
}
-close_guest_handle();
+$g->close();
$target->create_guest($os, $dom, $guestcaps);
@@ -418,58 +410,10 @@ END {
###############################################################################
## Helper functions
-sub close_guest_handle
-{
- # Config may cache the guestfs handle, preventing cleanup below
- $config = undef;
-
- if (defined($g)) {
- 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.
- $g = undef;
-
- # The above undef causes libguestfs's qemu process to be killed. This
- # may update $?, so we preserve it here.
- $? ||= $retval;
- }
-}
-
sub signal_exit
{
- close_guest_handle();
- exit(1);
-}
-
-sub get_guestfs_handle
-{
- my ($storage, $transferiso) = @_;
- my $interface = "ide";
-
- my $g = open_guest($storage, rw => 1, interface => $interface);
-
- # Add the transfer iso if there is one
- $g->add_drive_ro_with_if($transferiso, $interface)
- if(defined($transferiso));
-
- # Enable autosync to defend against data corruption on unclean shutdown
- $g->set_autosync(1);
-
- $g->launch();
-
- return $g;
+ $g->close() if (defined($g));
+ die(user_message(__x("Received signal {sig}. Exiting.", sig => shift)));
}
# Inspect the guest's storage. Returns an OS hashref as returned by
--
1.7.1
More information about the Libguestfs
mailing list