[Ovirt-devel] [PATCH] create-wui-appliance.sh: minor fixes

[Jim Meyering]

"Perry N. Myers" <pmyers at redhat.com> wrote:
> Changes look fine so ACK.  One question though, why did you change
> kickstart to lowercase and not IMGDIR?

Hi Perry,

Good point.
I didn't notice that there was another non-const variable
that was upper case.  I noticed/changed KICKSTART because I was
already fixing a problem with it.
Even so, I had qualms about mixing syntax-only changes
with bug-fixing ones.  But this is nothing serious or deep,
so it's no big deal to obscure the fixes slightly.

Also spotted an unused "ISO".

If no one objects, I'll push the following instead:

Thanks for the review.

Jim

>From 66f57c239b5b0afe6acf7ff8a5b4fb73d3f297a5 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Mon, 4 Aug 2008 11:36:22 +0200
Subject: [PATCH] create-wui-appliance.sh: minor fixes

* wui-appliance/create-wui-appliance.sh: Initialize, so that a
stray KICKSTART=garbage definition in the environment can't
make this script malfunction.  Change var names to lower case:
s/KICKSTART/kickstart/;s/IMGDIR/imgdir/.
(ISO): Remove unused variable.
Use printf instead of less-portable "echo -n".
Factor out a common "echo done".
---
 wui-appliance/create-wui-appliance.sh |   33 ++++++++++++++++-----------------
 1 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/wui-appliance/create-wui-appliance.sh b/wui-appliance/create-wui-appliance.sh
index 6365ddb..384f9f6 100755
--- a/wui-appliance/create-wui-appliance.sh
+++ b/wui-appliance/create-wui-appliance.sh
@@ -8,13 +8,12 @@ die() { warn "$@"; try_h; exit 1; }
 RAM=768
 IMGSIZE=6000M

-ISO=
 IMGDIR_DEFAULT=/var/lib/libvirt/images
 NET_SCRIPTS=/etc/sysconfig/network-scripts
 NAME=ovirt-appliance
 BRIDGENAME=ovirtbr

-IMGDIR=$IMGDIR_DEFAULT
+imgdir=$IMGDIR_DEFAULT

 usage() {
     case $# in 1) warn "$1"; try_h; exit 1;; esac
@@ -31,11 +30,12 @@ EOF
 err=0 help=0
 compress=0
 bridge=
+kickstart=
 while getopts :d:k:he: c; do
     case $c in
-        d) IMGDIR=$OPTARG;;
+        d) imgdir=$OPTARG;;
         c) compress=1;;
-        k) KICKSTART=$OPTARG;;
+        k) kickstart=$OPTARG;;
         e) bridge=$OPTARG;;
         h) help=1;;
         '?') err=1; warn "invalid option: \`-$OPTARG'";;
@@ -267,35 +267,34 @@ fi
 } > /dev/null 2>&1

 IMGNAME=$NAME.img
-mkdir -p $IMGDIR
+mkdir -p $imgdir
 virsh destroy $NAME > /dev/null 2>&1
 virsh undefine $NAME > /dev/null 2>&1

-if [ -n "$KICKSTART" ]; then
+if [ -n "$kickstart" ]; then
     mkdir -p tmp
     set -e
-    appliance-creator --config $KICKSTART --name $NAME --tmpdir $(pwd)/tmp
+    appliance-creator --config $kickstart --name $NAME --tmpdir $(pwd)/tmp
     # FIXME add --compress option to appliance-creator
     if [ $compress -ne 0 ]; then
-        echo -n "Compressing the image..."
-        qemu-img convert -c $NAME-sda.raw -O qcow2 "$IMGDIR/$IMGNAME"
+        printf "Compressing the image..."
+        qemu-img convert -c $NAME-sda.raw -O qcow2 "$imgdir/$IMGNAME"
         rm ovirt-appliance-sda.raw
-        echo "done"
     else
-        echo -n "Moving the image..."
-        mv ovirt-appliance-sda.raw "$IMGDIR/$IMGNAME"
-        restorecon -v "$IMGDIR/$IMGNAME"
-        echo "done"
+        printf "Moving the image..."
+        mv ovirt-appliance-sda.raw "$imgdir/$IMGNAME"
+        restorecon -v "$imgdir/$IMGNAME"
     fi
+    echo done
     set +e
 fi

-test ! -r $IMGDIR/$IMGNAME && die "Disk image not found at $IMGDIR/$IMGNAME"
+test ! -r $imgdir/$IMGNAME && die "Disk image not found at $imgdir/$IMGNAME"

 TMPXML=$(mktemp) || exit 1
 # FIXME virt-image to define the appliance instance
-gen_app $IMGDIR/$IMGNAME $RAM > $TMPXML
+gen_app $imgdir/$IMGNAME $RAM > $TMPXML
 virsh define $TMPXML
 rm $TMPXML
-echo "Application defined using disk located at $IMGDIR/$IMGNAME."
+echo "Application defined using disk located at $imgdir/$IMGNAME."
 echo "Run virsh start $NAME to start the appliance"
--
1.6.0.rc1.36.g5ff70