The code for mounting the transfer iso in Converter::Windows didn't do the same level of error checking as the same code in GuestOS::RedHat. This change moves the GuestOS code into Config, and updates both GuestOS::RedHat and Converter::Windows to use Config. Fixes RHBZ#596091 --- lib/Sys/VirtV2V/Config.pm | 53 +++++++++++++++++++++++++ lib/Sys/VirtV2V/Converter/Windows.pm | 40 ++++--------------- lib/Sys/VirtV2V/GuestOS/RedHat.pm | 72 ++++++--------------------------- 3 files changed, 74 insertions(+), 91 deletions(-) diff --git a/lib/Sys/VirtV2V/Config.pm b/lib/Sys/VirtV2V/Config.pm index 334adf9..3f689e0 100644 --- a/lib/Sys/VirtV2V/Config.pm +++ b/lib/Sys/VirtV2V/Config.pm @@ -203,6 +203,46 @@ sub get_transfer_iso return $iso_path; } +=item get_transfer_path(path) + +Return the path to I<path> as accessible by the libguestfs appliance. This +function will also ensure that the transfer iso is mounted. + +=cut + +sub get_transfer_path +{ + my $self = shift; + my ($g, $path) = @_; + + # Check that the transfer iso is mounted + if (!exists($self->{transfer_mount})) { + # Existing code expects the mount to exist, but handles the case where + # files in it don't exist. Therefore we always create the mount point, + # but only mount anything on it if there's actually a transfer iso. + + # Create the transfer mount point + # We create this under / because it's guaranteed to exist in the + # appliance, regardless of the guest OS. + $self->{transfer_mount} = $g->mkdtemp("/transferXXXXXX"); + + # Only mount the transfer iso if there is one + if (defined($self->get_transfer_iso())) { + # Find the transfer device + my @devices = $g->list_devices(); + 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; + } + } + + return File::Spec->catfile($self->{transfer_mount}, $path); +} + sub _get_search { my ($desc, $name, $arch) = @_; @@ -458,6 +498,19 @@ 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/Converter/Windows.pm b/lib/Sys/VirtV2V/Converter/Windows.pm index 4ea2caa..8de2bcd 100644 --- a/lib/Sys/VirtV2V/Converter/Windows.pm +++ b/lib/Sys/VirtV2V/Converter/Windows.pm @@ -181,21 +181,10 @@ sub _preconvert eval { $g->mkdir ("/temp"); }; eval { $g->mkdir ("/temp/v2v"); }; - # Find the transfer device - my @devices = $g->list_devices(); - my $transfer = $devices[$#devices]; - - my $transfer_mount = $g->mkdtemp ("/temp/transferXXXXXX"); - $g->mount_ro ($transfer, $transfer_mount); - - _upload_viostor ($g, $tmpdir, $desc, $devices, $config, - $transfer_mount); - _add_viostor_to_registry ($g, $tmpdir, $desc, $devices, $config, - $transfer_mount); - _upload_service ($g, $tmpdir, $desc, $devices, $config, - $transfer_mount); - _add_service_to_registry ($g, $tmpdir, $desc, $devices, $config, - $transfer_mount); + _upload_viostor ($g, $tmpdir, $desc, $devices, $config); + _add_viostor_to_registry ($g, $tmpdir, $desc, $devices, $config); + _upload_service ($g, $tmpdir, $desc, $devices, $config); + _add_service_to_registry ($g, $tmpdir, $desc, $devices, $config); } # See http://rwmj.wordpress.com/2010/04/30/tip-install-a-device-driver-in-a-win... @@ -206,7 +195,6 @@ sub _add_viostor_to_registry my $desc = shift; my $devices = shift; my $config = shift; - my $transfer_mount = shift; # Locate and download the system registry. my $system_filename; @@ -305,7 +293,6 @@ sub _add_service_to_registry my $desc = shift; my $devices = shift; my $config = shift; - my $transfer_mount = shift; # Locate and download the system registry. my $system_filename; @@ -363,13 +350,12 @@ sub _upload_viostor my $desc = shift; my $devices = shift; my $config = shift; - my $transfer_mount = shift; my $driverpath = "/windows/system32/drivers"; $driverpath = $g->case_sensitive_path ($driverpath); my ($app, $depnames) = $config->match_app ($desc, "viostor", $desc->{arch}); - $app = _transfer_path ($transfer_mount, $app); + $app = $config->get_transfer_path ($g, $app); $g->cp ($app, $driverpath); } @@ -380,36 +366,26 @@ sub _upload_service my $desc = shift; my $devices = shift; my $config = shift; - my $transfer_mount = shift; my $path = "/temp/v2v"; $path = $g->case_sensitive_path ($path); my ($app, $depnames) = $config->match_app ($desc, "firstboot", $desc->{arch}); - $app = _transfer_path ($transfer_mount, $app); + $app = $config->get_transfer_path ($g, $app); $g->cp ($app, $path); ($app, $depnames) = $config->match_app ($desc, "firstbootapp", $desc->{arch}); - $app = _transfer_path ($transfer_mount, $app); + $app = $config->get_transfer_path ($g, $app); $g->cp ($app, $path); ($app, $depnames) = $config->match_app ($desc, "rhsrvany", $desc->{arch}); - $app = _transfer_path ($transfer_mount, $app); + $app = $config->get_transfer_path ($g, $app); $g->cp ($app, $path); } -# Get full, local path of a file on the transfer mount -sub _transfer_path -{ - my $transfer_mount = shift; - my $path = shift; - - return File::Spec->catfile ($transfer_mount, $path); -} - =back =head1 COPYRIGHT diff --git a/lib/Sys/VirtV2V/GuestOS/RedHat.pm b/lib/Sys/VirtV2V/GuestOS/RedHat.pm index c46cbbe..1709733 100644 --- a/lib/Sys/VirtV2V/GuestOS/RedHat.pm +++ b/lib/Sys/VirtV2V/GuestOS/RedHat.pm @@ -889,7 +889,9 @@ sub _install_config } my @missing; - if (defined($kernel) && !$g->exists($self->_transfer_path($kernel))) { + if (defined($kernel) && + !$g->exists($self->{config}->get_transfer_path($g, $kernel))) + { push(@missing, $kernel); } @@ -1167,7 +1169,7 @@ sub _get_nevra my $g = $self->{g}; - $rpm = $self->_transfer_path($rpm); + $rpm = $self->{config}->get_transfer_path($g, $rpm); # Get NEVRA for the rpm to be installed my $nevra = $g->command(['rpm', '-qp', '--qf', @@ -1292,7 +1294,8 @@ sub _get_deppaths foreach my $app (@apps) { my ($path, $deps) = $config->match_app($desc, $app, $arch); - my $exists = $self->{g}->exists($self->_transfer_path($path)); + my $g = $self->{g}; + my $exists = $g->exists($self->{config}->get_transfer_path($g, $path)); if (!$exists) { push(@$missing, $path); @@ -1319,7 +1322,10 @@ sub _get_deppaths }; if (defined($path)) { - if (!$self->{g}->exists($self->_transfer_path($path))) { + my $g = $self->{g}; + + if (!$g->exists($self->{config}->get_transfer_path($g, $path))) + { push(@$missing, $path); foreach my $deppath ($self->_get_deppaths($missing, @@ -1482,10 +1488,11 @@ sub _install_rpms # Nothing to do if we got an empty set return if(scalar(@rpms) == 0); + my $g = $self->{g}; + # All paths are relative to the transfer mount. Need to make them absolute. - @rpms = map { $_ = $self->_transfer_path($_) } @rpms; + @rpms = map { $_ = $self->{config}->get_transfer_path($g, $_) } @rpms; - my $g = $self->{g}; $g->command(['rpm', $upgrade == 1 ? '-U' : '-i', @rpms]); # Reload augeas in case the rpm installation changed anything @@ -1496,46 +1503,6 @@ sub _install_rpms $self->_augeas_error($@) if($@); } -# Get full, local path of a file on the transfer mount -sub _transfer_path -{ - my $self = shift; - - my ($path) = @_; - - $self->_ensure_transfer_mounted(); - - return File::Spec->catfile($self->{transfer_mount}, $path); -} - -# Ensure that the transfer device is mounted. If not, mount it. -sub _ensure_transfer_mounted -{ - my $self = shift; - - # Return immediately if it's already mounted - return if(exists($self->{transfer_mount})); - - my $g = $self->{g}; - - # Code in this file expects the mount to exist, but handles the case where - # files in it don't exist. Therefore we always create the mount point, but - # only mount anything on it if there's actually a transfer iso. - - # Create the transfer mount point - $self->{transfer_mount} = $g->mkdtemp("/tmp/transferXXXXXX"); - - # Only mount the transfer iso if there is one - if (defined($self->{config}->get_transfer_iso())) { - # Find the transfer device - my @devices = $g->list_devices(); - my $transfer = $devices[$#devices]; - - $g->mount_ro($transfer, $self->{transfer_mount}); - $self->{transfer_mounted} = 1; - } -} - =item remap_block_devices(devices, virtio) See BACKEND INTERFACE in L<Sys::VirtV2V::GuestOS> for details. @@ -1830,19 +1797,6 @@ sub supports_virtio return 1; } -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 -- 1.7.0.1