[Ovirt-devel] [PATCH]: Rewrite the ovirt-identify-node in bash

[Jim Meyering]

Chris Lalancette <clalance at redhat.com> wrote:
>      Attached is a patch to rewrite the ovirt-identify-node script in bash.
> Note that this is a drop-in replacement for the current python script; in
> particular, it doesn't do any restructuring like Darryl's new code is going to
> do.  However, given that we need to get rid of python because of my last patch,
> I think this is a fine interim step.  Note that this patch must be applied on
> top of the previous "Shrink Ovirt Node" patch to apply cleanly.

Nice!
only a few nit-picky suggestions...

> +# gather our information
> +uuid=`hostname -f`
> +arch=`uname -i`
> +memsize=$(( `getconf _PHYS_PAGES` * `getconf PAGESIZE` / 1024 / 1024 ))
> +numcpus=`getconf _NPROCESSORS_ONLN`
> +speed=`grep "cpu MHz" /proc/cpuinfo | uniq | cut -d':' -f2 | sed -e 's/^[[:space:]]*\(.*\)$/\1/' -e 's/^\(.*\)[[:space:]]*$/\1/'`

speed=$(sed -n "/cpu MHz/{s/.*://p;q;}" /proc/cpuinfo|tr -dc 0-9.)

That saves a few pipes, but also avoids the potential for problems
if you have CPUs with differing speeds -- it just takes the first one.

These days (esp with bash, where portability isn't an issue at all),
I prefer to use $(...) over `...`, because you can nest them -- plus
a little more readable, imho.

You have to resort to using `...` only if you're targeting the old
style /bin/sh that are still around, but that no one uses if they can
avoid it.  Even in coreutils tests, which try to be ultra portable,
I've been able to switch, since it uses a cool m4 macro from autoconf
to find a POSIX-conforming (and thus supports $(...) syntax, functions,
etc.) bourne shell that works just about everywhere.

Connecting them all with && you can detect if any fail:

all_ok=0
uuid=$(hostname -f) &&
  arch=$(uname -i) &&
  memsize=$(( $(getconf _PHYS_PAGES) * $(getconf PAGESIZE) / 1024 / 1024 )) &&
  numcpus=$(getconf _NPROCESSORS_ONLN) &&
  speed=$(sed -n "/cpu MHz/{s/.*://p;q;}" /proc/cpuinfo|tr -dc 0-9.) &&
    all_ok=1
test $all_ok = 1 || die "something failed; see above"