Libguestfs - May 2010 - [PATCH] Fix error in Converter::Windows when there is no transfer iso

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            |   51 ++++++++++++++++++++++++
 lib/Sys/VirtV2V/Converter/Windows.pm |   40 ++++---------------
 lib/Sys/VirtV2V/GuestOS/RedHat.pm    |   72 ++++++---------------------------
 3 files changed, 72 insertions(+), 91 deletions(-)

diff --git a/lib/Sys/VirtV2V/Config.pm b/lib/Sys/VirtV2V/Config.pm
index 334adf9..10ef8be 100644
--- a/lib/Sys/VirtV2V/Config.pm
+++ b/lib/Sys/VirtV2V/Config.pm
@@ -203,6 +203,44 @@ 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
+        $self->{transfer_mount} =
$g->mkdtemp("/tmp/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 +496,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-windows-vm/
@@ -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

On 26/05/10 13:12, Matthew Booth wrote:
> 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
Ignore this, it's broken.

Matt
-- 
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

M:       +44 (0)7977 267231
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490