[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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-,,')


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]