Libguestfs - Jan 2011 - [PATCH] Fix architecture detection of Linux guests with invalid grub.conf

If a Linux guest's grub.conf contained a default kernel which referred to
non-existent kernel, architecture detection would default to i686. For an x86_64
guest, the result would not boot.

This change uses the first grub.conf entry if the default is invalid or not
present, and falls back to the architecture detected by inspection otherwise. If
neither of these yield an architecture, it defaults to x86_64 instead of i686 as
a more conservative default.

Fixes RHBZ#623579
---
 lib/Sys/VirtV2V/Converter/RedHat.pm |   21 ++++++++++++---------
 1 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/lib/Sys/VirtV2V/Converter/RedHat.pm
b/lib/Sys/VirtV2V/Converter/RedHat.pm
index 78a454a..a8a7051 100644
--- a/lib/Sys/VirtV2V/Converter/RedHat.pm
+++ b/lib/Sys/VirtV2V/Converter/RedHat.pm
@@ -610,17 +610,20 @@ sub _get_os_arch
     my $boot = $desc->{boot};
     my $default_boot = $boot->{default} if(defined($boot));
 
-    my $arch;
-    if(defined($default_boot)) {
-        my $config = $boot->{configs}->[$default_boot];
+    # Pick the default config if one is defined
+    my $config = $boot->{configs}->[$default_boot] if
defined($default_boot);
 
-        if(defined($config->{kernel})) {
-            $arch = $config->{kernel}->{arch};
-        }
-    }
+    # Pick the first defined config if there is no default, or it is invalid
+    $config = $boot->{configs}[0] unless defined($config);
+
+    my $arch = $config->{kernel}->{arch}
+        if defined($config) && defined($config->{kernel});
+
+    # Use the libguestfs-detected arch if the above failed
+    $arch = $desc->{arch} unless defined($arch);
 
-    # Default to i686 if we didn't find an architecture
-    return 'i686' if(!defined($arch));
+    # Default to x86_64 if we still didn't find an architecture
+    return 'x86_64' unless defined($arch);
 
     # We want an i686 guest for i[345]86
     return 'i686' if($arch =~ /^i[345]86$/);
-- 
1.7.3.4

On Wed, Jan 19, 2011 at 04:35:04PM +0000, Matthew Booth
wrote:
> If a Linux guest's grub.conf contained a default kernel which referred
to
> non-existent kernel, architecture detection would default to i686. For an
x86_64
> guest, the result would not boot.
> 
> This change uses the first grub.conf entry if the default is invalid or not
> present, and falls back to the architecture detected by inspection
otherwise. If
> neither of these yield an architecture, it defaults to x86_64 instead of
i686 as
> a more conservative default.
> 
> Fixes RHBZ#623579
> ---
>  lib/Sys/VirtV2V/Converter/RedHat.pm |   21 ++++++++++++---------
>  1 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/Sys/VirtV2V/Converter/RedHat.pm
b/lib/Sys/VirtV2V/Converter/RedHat.pm
> index 78a454a..a8a7051 100644
> --- a/lib/Sys/VirtV2V/Converter/RedHat.pm
> +++ b/lib/Sys/VirtV2V/Converter/RedHat.pm
> @@ -610,17 +610,20 @@ sub _get_os_arch
>      my $boot = $desc->{boot};
>      my $default_boot = $boot->{default} if(defined($boot));
>  
> -    my $arch;
> -    if(defined($default_boot)) {
> -        my $config = $boot->{configs}->[$default_boot];
> +    # Pick the default config if one is defined
> +    my $config = $boot->{configs}->[$default_boot] if
defined($default_boot);
>  
> -        if(defined($config->{kernel})) {
> -            $arch = $config->{kernel}->{arch};
> -        }
> -    }
> +    # Pick the first defined config if there is no default, or it is
invalid
> +    $config = $boot->{configs}[0] unless defined($config);
> +
> +    my $arch = $config->{kernel}->{arch}
> +        if defined($config) && defined($config->{kernel});
> +
> +    # Use the libguestfs-detected arch if the above failed
> +    $arch = $desc->{arch} unless defined($arch);
>  
> -    # Default to i686 if we didn't find an architecture
> -    return 'i686' if(!defined($arch));
> +    # Default to x86_64 if we still didn't find an architecture
> +    return 'x86_64' unless defined($arch);
>  
>      # We want an i686 guest for i[345]86
>      return 'i686' if($arch =~ /^i[345]86$/);
Code looks good to me, and the reasoning is sound, ACK.

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