Libguestfs - Nov 2009 - [PATCH] supermin: Die with an error if no kernels found (RHBZ#539746).

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://et.redhat.com/~rjones/libguestfs/
See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html
-------------- next part --------------
>From cbe4c60e44db880b1cc4a217e0815db4b89e2041 Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones at redhat.com>
Date: Tue, 24 Nov 2009 18:17:07 +0000
Subject: [PATCH] supermin: Die with an error if no kernels found (RHBZ#539746).

---
 appliance/libguestfs-supermin-helper.in |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/appliance/libguestfs-supermin-helper.in
b/appliance/libguestfs-supermin-helper.in
index 69f22e4..3e4a260 100755
--- a/appliance/libguestfs-supermin-helper.in
+++ b/appliance/libguestfs-supermin-helper.in
@@ -39,6 +39,12 @@ initrd="$3"
 
 arch=$(echo "@host_cpu@" | sed 's/^i.86$/i?86/')
 kernels=$(ls -1vr /boot/vmlinuz-*.$arch* 2>/dev/null | grep -v xen; ls -1vr
/boot/vmlinuz-* 2>/dev/null | grep -v xen)
+
+if [ -z "$kernels" ]; then
+    echo "$0: failed to find a suitable kernel" >&2
+    exit 1
+fi
+
 for f in $kernels; do
     b=$(basename "$f")
     b=$(echo "$b" | sed 's,vmlinuz-,,')
-- 
1.6.5.2

Richard W.M. Jones wrote:
> Subject: [PATCH] supermin: Die with an error if no kernels found
(RHBZ#539746).
>
> ---
>  appliance/libguestfs-supermin-helper.in |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/appliance/libguestfs-supermin-helper.in
b/appliance/libguestfs-supermin-helper.in
> index 69f22e4..3e4a260 100755
> --- a/appliance/libguestfs-supermin-helper.in
> +++ b/appliance/libguestfs-supermin-helper.in
> @@ -39,6 +39,12 @@ initrd="$3"
>
>  arch=$(echo "@host_cpu@" | sed 's/^i.86$/i?86/')
>  kernels=$(ls -1vr /boot/vmlinuz-*.$arch* 2>/dev/null | grep -v xen; ls
-1vr /boot/vmlinuz-* 2>/dev/null | grep -v xen)
> +
> +if [ -z "$kernels" ]; then
> +    echo "$0: failed to find a suitable kernel" >&2
> +    exit 1
> +fi
> +
>  for f in $kernels; do
>      b=$(basename "$f")
>      b=$(echo "$b" | sed 's,vmlinuz-,,')
Hi Rich,

That change looks fine.
However, the context can be improved slightly.
First, on this kernels-assigning line,

    kernels=$(ls -1vr /boot/vmlinuz-*.$arch* 2>/dev/null | grep -v xen; ls
-1vr /boot/vmlinuz-* 2>/dev/null | grep -v xen)

if you add ls' -d option, that will prevent a false-positive match
if /boot/vmlinuz-* ever matches a directory containing something whose
name includes "xen".

Also, you can avoid duplicating the "2>/dev/null | grep -v xen"
part
by using a single invocatin of ls, and then I noticed that the expansion
of the second glob, /boot/vmlinuz-*, would include anything matched by
the first one, so this is adequate:

    kernels=$(ls -1dvr /boot/vmlinuz-* 2>/dev/null | grep -v xen)

diff --git a/appliance/libguestfs-supermin-helper.in
b/appliance/libguestfs-supermin-helper.in
index 69f22e4..a24d905 100755
--- a/appliance/libguestfs-supermin-helper.in
+++ b/appliance/libguestfs-supermin-helper.in
@@ -38,7 +38,8 @@ initrd="$3"
 # without arch second.

 arch=$(echo "@host_cpu@" | sed 's/^i.86$/i?86/')
-kernels=$(ls -1vr /boot/vmlinuz-*.$arch* 2>/dev/null | grep -v xen; ls -1vr
/boot/vmlinuz-* 2>/dev/null | grep -v xen)
+kernels=$(ls -1dvr /boot/vmlinuz-* 2>/dev/null | grep -v xen)
+
 for f in $kernels; do
     b=$(basename "$f")
     b=$(echo "$b" | sed 's,vmlinuz-,,')