Libguestfs - Aug 2009 - [PATCH] Export inspect_linux_kernel in Lib.pm

---
 perl/lib/Sys/Guestfs/Lib.pm |   38 +++++++++++++++++++++++++-------------
 1 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/perl/lib/Sys/Guestfs/Lib.pm b/perl/lib/Sys/Guestfs/Lib.pm
index 1f84bc6..dfa79af 100644
--- a/perl/lib/Sys/Guestfs/Lib.pm
+++ b/perl/lib/Sys/Guestfs/Lib.pm
@@ -66,7 +66,8 @@ use vars qw(@EXPORT_OK @ISA);
 @ISA = qw(Exporter);
 @EXPORT_OK = qw(open_guest get_partitions resolve_windows_path
   inspect_all_partitions inspect_partition
-  inspect_operating_systems mount_operating_system inspect_in_detail);
+  inspect_operating_systems mount_operating_system inspect_in_detail
+  inspect_linux_kernel);
 
 =head2 open_guest
 
@@ -1551,10 +1552,19 @@ sub _check_for_kernels
                 }
                 $config{cmdline} = join(' ', @args) if(scalar(@args)
> 0);
 
-                my $kernel = _inspect_linux_kernel($g, $os, "$path");
+                my $kernel +                    inspect_linux_kernel($g, $path,
$os->{package_format});
 
                 # Check the kernel was recognised
                 if(defined($kernel)) {
+                    # Put this kernel on the top level kernel list
+                    my $kernels = $os->{kernels};
+                    if(!defined($kernels)) {
+                        $kernels = [];
+                        $os->{kernels} = $kernels;
+                    }
+                    push(@$kernels, $kernel);
+
                     $config{kernel} = $kernel;
 
                     # Look for an initrd entry
@@ -1599,9 +1609,19 @@ sub _check_for_kernels
     }
 }
 
-sub _inspect_linux_kernel
+=head2 inspect_linux_kernel
+
+ my $kernel_hash = inspect_linux_kernel($g, $vmlinuz_path, $package_format);
+
+inspect_linux_kernel returns a hash describing the target linux kernel. For the
+contents of the hash, see the I<kernels> structure described under
+L</inspect_in_detail>.
+
+=cut
+
+sub inspect_linux_kernel
 {
-    my ($g, $os, $path) = @_;
+    my ($g, $path, $package_format) = @_;
 
     my %kernel = ();
 
@@ -1610,7 +1630,7 @@ sub _inspect_linux_kernel
     # If this is a packaged kernel, try to work out the name of the package
     # which installed it. This lets us know what to install to replace it with,
     # e.g. kernel, kernel-smp, kernel-hugemem, kernel-PAE
-    if($os->{package_format} eq "rpm") {
+    if($package_format eq "rpm") {
         my $package;
         eval { $package = $g->command(['rpm', '-qf',
'--qf',
                                        '%{NAME}', $path]); };
@@ -1667,14 +1687,6 @@ sub _inspect_linux_kernel
     # of any kernel module.
     $kernel{arch} = file_architecture ($g, $any_module);
 
-    # Put this kernel on the top level kernel list
-    my $kernels = $os->{kernels};
-    if(!defined($kernels)) {
-        $kernels = [];
-        $os->{kernels} = $kernels;
-    }
-    push(@$kernels, \%kernel);
-
     return \%kernel;
 }
 
-- 
1.6.2.5

On Wed, Aug 19, 2009 at 12:54:41PM +0100, Matthew Booth
wrote:
> ---
>  perl/lib/Sys/Guestfs/Lib.pm |   38 +++++++++++++++++++++++++-------------
>  1 files changed, 25 insertions(+), 13 deletions(-)
ACK.

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top

Matthew Booth wrote:
> diff --git a/perl/lib/Sys/Guestfs/Lib.pm b/perl/lib/Sys/Guestfs/Lib.pm
...
> @@ -1551,10 +1552,19 @@ sub _check_for_kernels
...
>                  # Check the kernel was recognised
>                  if(defined($kernel)) {
> +                    # Put this kernel on the top level kernel list
> +                    my $kernels = $os->{kernels};
> +                    if(!defined($kernels)) {
> +                        $kernels = [];
> +                        $os->{kernels} = $kernels;
> +                    }
> +                    push(@$kernels, $kernel);
> +
Hi Matt,

It took me too long to see what was being done above,
so I rewrote it in a way that I found easier to read:

How about this?

diff --git a/perl/lib/Sys/Guestfs/Lib.pm b/perl/lib/Sys/Guestfs/Lib.pm
index dfa79af..2acdec6 100644
--- a/perl/lib/Sys/Guestfs/Lib.pm
+++ b/perl/lib/Sys/Guestfs/Lib.pm
@@ -1558,12 +1558,8 @@ sub _check_for_kernels
                 # Check the kernel was recognised
                 if(defined($kernel)) {
                     # Put this kernel on the top level kernel list
-                    my $kernels = $os->{kernels};
-                    if(!defined($kernels)) {
-                        $kernels = [];
-                        $os->{kernels} = $kernels;
-                    }
-                    push(@$kernels, $kernel);
+                    $os->{kernels} ||= [];
+                    push(@{$os->{kernels}}, $kernel);

                     $config{kernel} = $kernel;


The ||= operator may look odd, but it's another of these idiom things.
Once you see it a few times, then "get it", and start using it,
you'll never go back.

Which would you prefer to read/maintain?  This:

    # Ensure the array-ref is initialized.
    if (!defined $some_long_name->[$some_complicated_expression]) {
        $some_long_name->[$some_complicated_expression] = [];
    }

    # Append to it.
    ...

or this:

    # Ensure the array-ref is initialized.
    $some_long_name->[$some_complicated_expression] ||= [];

    # Append to it.
    ...