Libguestfs - Mar 2010 - [PATCH] Default to IDE when VirtIO isn't available

Previously we used SCSI when VirtIO wasn't available. KVM's SCSI support
is not
as mature as its IDE support, and SCSI isn't supported at all in RHEV.
---
 lib/Sys/VirtV2V/Converter.pm       |   50 +++++++++++-------
 lib/Sys/VirtV2V/Converter/Linux.pm |   19 ++++---
 lib/Sys/VirtV2V/GuestOS/RedHat.pm  |  100 ++++++++++++++++++++----------------
 3 files changed, 100 insertions(+), 69 deletions(-)

diff --git a/lib/Sys/VirtV2V/Converter.pm b/lib/Sys/VirtV2V/Converter.pm
index 71e94f7..5dc8550 100644
--- a/lib/Sys/VirtV2V/Converter.pm
+++ b/lib/Sys/VirtV2V/Converter.pm
@@ -87,7 +87,7 @@ use constant KVM_XML_NOVIRTIO => "
   </os>
   <devices>
     <disk device='disk'>
-      <target bus='scsi'/>
+      <target bus='ide'/>
     </disk>
     <interface type='network'>
       <model type='e1000'/>
@@ -268,51 +268,65 @@ sub _unconfigure_bootloaders
     }
 }
 
+sub _suffixcmp
+{
+    my ($a, $b) = @_;
+
+    return 1 if (length($a) > length($b));
+    return -1 if (length($a) < length($b));
+
+    return 1 if ($a gt $b);
+    return -1 if ($a lt $b);
+    return 0;
+}
+
 sub _configure_storage
 {
     my ($dom, $devices, $virtio) = @_;
 
-    my $prefix = $virtio ? 'vd' : 'sd';
+    my $prefix = $virtio ? 'vd' : 'hd';
+
+    my @removed = ();
 
     my $suffix = 'a';
     foreach my $device (@$devices) {
         my ($target) =
$dom->findnodes("/domain/devices/disk[\@device='disk']/".
                                       
"target[\@dev='$device']");
 
-        die(user_message(__x("Previously detected drive {drive} is no
longer ".
-                             "present in domain XML: {xml}",
-                             drive => $device,
-                             xml => $dom->toString())))
+        die("Previously detected drive $device is no longer present in
domain ".
+            "XML: ".$dom->toString())
             unless (defined($target));
 
-        $target->setAttribute('bus', $virtio ? 'virtio' :
'scsi');
-        $target->setAttribute('dev', $prefix.$suffix);
-        $suffix++; # Perl magic means 'z'++ == 'aa'
+        # Don't add more than 4 IDE disks
+        if (!$virtio && _suffixcmp($suffix, 'd') > 0) {
+            push(@removed, "$device(disk)");
+        } else {
+            $target->setAttribute('bus', $virtio ? 'virtio'
: 'ide');
+            $target->setAttribute('dev', $prefix.$suffix);
+            $suffix++; # Perl magic means 'z'++ == 'aa'
+        }
     }
 
-    # Convert the first 4 CDROM drives to IDE, and remove the rest
-    $suffix = 'a';
-    my $i = 0;
-    my @removed = ();
+    # Convert CD-ROM devices to IDE.
+    $suffix = 'a' if ($virtio);
     foreach my $target
        
($dom->findnodes("/domain/devices/disk[\@device='cdrom']/target"))
     {
-        if ($i < 4) {
+        if (_suffixcmp($suffix, 'd') <= 0) {
             $target->setAttribute('bus', 'ide');
             $target->setAttribute('dev', "hd$suffix");
             $suffix++;
         } else {
-            push(@removed, $target->getAttribute('dev'));
+            push(@removed,
$target->getAttribute('dev')."(cdrom)");
 
             my $disk = $target->getParentNode();
             $disk->getParentNode()->removeChild($disk);
         }
-        $i++;
     }
 
     if (@removed > 0) {
-        print user_message(__x("WARNING: Only 4 CDROM drives are
supported. ".
-                               "The following CDROM drives have been
removed: ".
+        print user_message(__x("WARNING: Only 4 IDE devices are supported.
".
+                               "The following drives have been removed:
".
                                "{list}",
                                list => join(' ', @removed)));
     }
diff --git a/lib/Sys/VirtV2V/Converter/Linux.pm
b/lib/Sys/VirtV2V/Converter/Linux.pm
index 6f49351..3e76762 100644
--- a/lib/Sys/VirtV2V/Converter/Linux.pm
+++ b/lib/Sys/VirtV2V/Converter/Linux.pm
@@ -142,7 +142,7 @@ sub _configure_kernel_modules
 
     # Make a note of whether we've added scsi_hostadapter
     # We need this on RHEL 4/virtio because mkinitrd can't detect root on
-    # virtio. For simplicity we always ensure this is set.
+    # virtio. For simplicity we always ensure this is set for virtio disks.
     my $scsi_hostadapter = 0;
 
     foreach my $module (keys(%$modules)) {
@@ -164,17 +164,22 @@ sub _configure_kernel_modules
                 $hvs_modules{$module} = 1;
             }
 
-            $guestos->update_kernel_module($module,
-                                          $virtio ? "virtio_blk" :
"sym53c8xx");
+            if ($virtio) {
+                $guestos->update_kernel_module($module,
'virtio_blk');
+                $scsi_hostadapter = 1;
+            }
 
-            $scsi_hostadapter = 1;
+            # IDE doesn't need scsi_hostadapter
+            else {
+                $guestos->disable_kernel_module($module);
+            }
         }
     }
 
     # Add an explicit scsi_hostadapter if it wasn't there before
-    $guestos->enable_kernel_module('scsi_hostadapter',
-                                $virtio ? "virtio_blk" :
"sym53c8xx")
-        unless($scsi_hostadapter);
+    if ($virtio && !$scsi_hostadapter) {
+        $guestos->enable_kernel_module('scsi_hostadapter',
'virtio_blk');
+    }
 
     # Warn if any old-HV specific kernel modules weren't updated
     foreach my $module (keys(%hvs_modules)) {
diff --git a/lib/Sys/VirtV2V/GuestOS/RedHat.pm
b/lib/Sys/VirtV2V/GuestOS/RedHat.pm
index 3f5d90a..8664c2d 100644
--- a/lib/Sys/VirtV2V/GuestOS/RedHat.pm
+++ b/lib/Sys/VirtV2V/GuestOS/RedHat.pm
@@ -964,54 +964,52 @@ sub remap_block_devices
     # same names as used by the guest. However, if the guest is using libata,
     # IDE drives could be renamed.
 
-    # Look for IDE and SCSI devices in fstab for the guest
-    my %guestif;
-    foreach my $spec ($g->aug_match('/files/etc/fstab/*/spec')) {
-        my $device = $g->aug_get($spec);
-
-        next unless($device =~ m{^/dev/(sd|hd)([a-z]+)});
-        $guestif{$1} ||= {};
-        $guestif{$1}->{$1.$2} = 1;
+    # Modern distros use libata, and IDE devices are presented as sdX
+    my $libata = 1;
+
+    # RHEL 2, 3 and 4 didn't use libata
+    # RHEL 5 does use libata, but udev rules call IDE devices hdX anyway
+    if ($desc->{distro} eq 'rhel') {
+        if ($desc->{major_version} eq '2' ||
+            $desc->{major_version} eq '3' ||
+            $desc->{major_version} eq '4' ||
+            $desc->{major_version} eq '5')
+        {
+            $libata = 0;
+        }
     }
+    # Fedora has used libata since FC7, which is long out of support. We assume
+    # that all Fedora distributions in use use libata.
+
+    if ($libata) {
+        # Look for IDE and SCSI devices in fstab for the guest
+        my %guestif;
+        foreach my $spec ($g->aug_match('/files/etc/fstab/*/spec'))
{
+            my $device = $g->aug_get($spec);
+
+            next unless($device =~ m{^/dev/(sd|hd)([a-z]+)});
+            $guestif{$1} ||= {};
+            $guestif{$1}->{$1.$2} = 1;
+        }
 
-    # If fstab contains references to sdX, these could refer to IDE or SCSI
-    # devices. Need to look at the domain config for clues.
-    if (exists($guestif{sd})) {
-        # Look for IDE and SCSI devices from the domain definition
-        my %domainif;
-        foreach my $device (@$devices) {
-            foreach my $type ('hd', 'sd') {
-                if ($device =~ m{^$type([a-z]+)}) {
-                    $domainif{$type} ||= {};
-                    $domainif{$type}->{$device} = 1;
+        # If fstab contains references to sdX, these could refer to IDE or SCSI
+        # devices. We may need to update them.
+        if (exists($guestif{sd})) {
+            # Look for IDE and SCSI devices from the domain definition
+            my %domainif;
+            foreach my $device (@$devices) {
+                foreach my $type ('hd', 'sd') {
+                    if ($device =~ m{^$type([a-z]+)}) {
+                        $domainif{$type} ||= {};
+                        $domainif{$type}->{$device} = 1;
+                    }
                 }
             }
-        }
-
-        # If domain defines both IDE and SCSI drives, and fstab contains
-        # references on sdX, but not hdX, we don't know if the guest is
using
-        # libata or not.  This means that we don't know if sdX in fstab
refers
-        # to hdX or sdX in the domain. Warn and assume that libata device
-        # renaming is not in use.
-        if (exists($domainif{hd}) && exists($domainif{sd}) &&
-            !exists($guestif{hd}))
-        {
-            print STDERR user_message(__"WARNING: Unable to determine
whether ".
-                                        "sdX devices in /etc/fstab refer
to ".
-                                        "IDE or SCSI devices. Assuming
they ".
-                                        "refer to SCSI devices. /etc/fstab
".
-                                        "may be incorrect after conversion
if ".
-                                        "guest uses libata.");
-        }
 
-        # If we've got only IDE devices in the domain, and only sdX devices
in
-        # fstab, the guest is renaming them.
-        elsif (exists($domainif{hd}) && !exists($domainif{sd})
&&
-               !exists($guestif{hd}))
-        {
             my %map;
 
             my $letter = 'a';
+            # IDE drives are presented first
             foreach my $old (sort { _drivecmp('hd', $a, $b) }
                                   keys(%{$domainif{hd}}))
             {
@@ -1019,10 +1017,16 @@ sub remap_block_devices
                 $letter++;
             }
 
+            # Followed by SCSI drives
+            foreach my $old (sort { _drivecmp('sd', $a, $b) }
+                                  keys(%{$domainif{sd}}))
+            {
+                $map{$old} = "sd$letter";
+                $letter++;
+            }
+
             map { $_ = $map{$_} } @$devices;
         }
-
-        # Otherwise we leave it alone
     }
 
     # We now assume that $devices contains an ordered list of device names, as
@@ -1030,8 +1034,16 @@ sub remap_block_devices
     # device names.
     my %map;
 
-    # Everything will be converted to either vdX or sdX
-    my $prefix = $virtio ? 'vd' : 'sd';
+    # Everything will be converted to either vdX, sdX or hdX
+    my $prefix;
+    if ($virtio) {
+        $prefix = 'vd';
+    } elsif ($libata) {
+        $prefix = 'sd';
+    } else {
+        $prefix = 'hd'
+    }
+
     my $letter = 'a';
     foreach my $device (@$devices) {
         $map{$device} = $prefix.$letter;
-- 
1.6.6.1

On Wed, Mar 31, 2010 at 02:45:30PM +0100, Matthew Booth
wrote:
> Previously we used SCSI when VirtIO wasn't available. KVM's SCSI
> support is not as mature as its IDE support, and SCSI isn't
> supported at all in RHEV.
Ugh, but ACK.

It might have been better for it to be configurable, but in general we
want to encourage virtio rather than either of the legacy devices (IDE
or SCSI).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora