Ovirt devel - Mar 2010 - [PATCH node] Handle space in storage wwid

Quote everywhere that we reference wwid in o-c-storage, o-c-boot
and ovirt-functions.

Signed-off-by: Mike Burns <mburns at redhat.com>
---
 scripts/ovirt-config-boot    |    6 +-
 scripts/ovirt-config-storage |  178 +++++++++++++++++++++---------------------
 scripts/ovirt-functions      |   38 +++++-----
 3 files changed, 111 insertions(+), 111 deletions(-)

diff --git a/scripts/ovirt-config-boot b/scripts/ovirt-config-boot
index d545878..347fc18 100755
--- a/scripts/ovirt-config-boot
+++ b/scripts/ovirt-config-boot
@@ -88,7 +88,7 @@ ovirt_boot_setup() {
             rc=0
         else
             candidate_dev=$(findfs LABEL=$candidate 2>/dev/null)
-            e2label $candidate_dev RootNew
+            e2label "$candidate_dev" RootNew
             rc=$?
         fi
         if [ $rc -ne 0 ]; then
@@ -97,7 +97,7 @@ ovirt_boot_setup() {
           return $rc
         fi

-        mount $candidate_dev /liveos/
+        mount "$candidate_dev" /liveos/

         rm -rf /liveos/LiveOS
         mkdir -p /liveos/LiveOS
@@ -175,7 +175,7 @@ EOF
     if [ "$OVIRT_ISCSI_ENABLED" != "y" ]; then
         umount /liveos
         # mark new Root ready to go, reboot() in ovirt-function switches it to
active
-        e2label $candidate_dev RootUpdate
+        e2label "$candidate_dev" RootUpdate
     fi

     rm -rf $tmpdir
diff --git a/scripts/ovirt-config-storage b/scripts/ovirt-config-storage
index 8d59a6b..9fee10e 100755
--- a/scripts/ovirt-config-storage
+++ b/scripts/ovirt-config-storage
@@ -36,12 +36,12 @@ swap_min_size=5

 # return sd name for given #:# identifier
 get_sd_name() {
-    local id=$1
-    local device_var=$2
+    local id="$1"
+    local device_var="$2"
     local device_sys=$(grep -H "^$id$" /sys/block/*/dev | cut -d:
-f1)

     if [ -n "$device_sys" ]; then
-        eval $device_var=$(basename $(dirname $device_sys))
+        eval $device_var=$(basename $(dirname "$device_sys"))
         return
     fi
     eval $device_var=1
@@ -49,23 +49,23 @@ get_sd_name() {

 # gets the dependent block devices for multipath devices
 get_multipath_devices() {
-    local mpath_device=mpath-$(basename $1)
-    local deplist_var=$2
+    local mpath_device=mpath-$(basename "$1")
+    local deplist_var="$2"
     local deplist=""

     #get dependencies for multipath device
-    local deps=$(dmsetup deps -u $mpath_device \
+    local deps=$(dmsetup deps -u "$mpath_device" \
     | sed -r 's/\(([0-9]+), ([0-9]+)\)/\1:\2/g' \
     | sed 's/ /\n/g' | grep [0-9]:[0-9] )

     local device=""
-    for dep in $deps
+    for dep in "$deps"
     do
         local device=""
-        get_sd_name $dep device
-        if [[ ! $device = 1 ]]; then
-            if [[ $deplist = "" ]]; then
-                deplist=$device
+        get_sd_name "$dep" device
+        if [[ ! "$device" = 1 ]]; then
+            if [[ "$deplist" = "" ]]; then
+                deplist="$device"
             else
                 deplist="$deplist $device"
             fi
@@ -79,8 +79,8 @@ get_multipath_devices() {
 #return sdX device if no multipath device
 translate_multipath_device() {
     #trim so that only sdX is stored, but support passing /dev/sdX
-    local dev=$1
-    local mpath_var=$2
+    local dev="$1"
+    local mpath_var="$2"

     if [ -z "$dev" ]; then
         if [ -n "$mpath_var" ]; then
@@ -89,36 +89,36 @@ translate_multipath_device() {
         return
     fi
     if [[ "$dev" =~ "/dev/mapper" ]]; then
-        eval $mpath_var=$dev
+        eval $math_var="$dev"
         return
     fi

-    local dm_dev=/dev/$(multipath -ll $dev | egrep dm-[0-9]+ | sed -r
's/^.& (dm-[0-9]+) .*$/\1/')
+    local dm_dev=/dev/$(multipath -ll "$dev" | egrep dm-[0-9]+ | sed
-r 's/^.& (dm-[0-9]+) .*$/\1/')

     local mpath_device-    get_dm_device $dm_dev mpath_device
+    get_dm_device "$dm_dev" mpath_device

     if [ -z "$mpath_device" ]; then
-        mpath_device=$dev
+        mpath_device="$dev"
     fi

-    eval $mpath_var=$mpath_device
+    eval $mpath_var="$mpath_device"
 }


 get_drive_size()
 {
-    local drive=$1
-    local space_var=$2
+    local drive="$1"
+    local space_var="$2"

     local size-    local udi=$(hal-find-by-property --key block.device --string
$drive)
+    local udi=$(hal-find-by-property --key block.device --string
"$drive")
     # if more than one UDI was found then iterate over them to find the base
device
     if [[ "${udi}" =~ \$  ]]; then
-        udi=$(echo $udi | sed 's/\$/ /g')
+        udi=$(echo "$udi" | sed 's/\$/ /g')
         for found in ${udi}; do
-            if [[ "false" == $(hal-get-property --udi $found --key
block.is_volume) ]]; then
-                udi=$found
+            if [[ "false" == $(hal-get-property --udi
"$found" --key block.is_volume) ]]; then
+                udi="$found"
                 break
             fi
         done
@@ -127,7 +127,7 @@ get_drive_size()
         # If hal didn't find the device, it could be a virtio block device
         # or a multipath device
         # In this case, use sfdisk -s to get the size
-        size=$(sfdisk -s $drive 2>/dev/null)
+        size=$(sfdisk -s "$drive" 2>/dev/null)
         if [ -z "$size" ]; then
             size=0
         fi
@@ -147,7 +147,7 @@ get_drive_size()
     echo "$drive ($size MB)"
     test -z "$udi" || echo "Disk Identifier: $(basename
"$udi")"
     if [ -n "$space_var" ]; then
-        eval $space_var=$size
+        eval $space_var="$size"
     fi
 }

@@ -155,7 +155,7 @@ check_partition_sizes()
 {
     local disk_size need_size

-    local min_data_size=$DATA_SIZE
+    local min_data_size="$DATA_SIZE"
     if [ "$DATA_SIZE" = -1 ]; then
         min_data_size=5
     fi
@@ -164,7 +164,7 @@ check_partition_sizes()
     if [ "$OVIRT_ISCSI_ENABLED" == "y" ]; then
         get_drive_size "$BOOTDRIVE" BOOTDRIVESPACE
         drive_list="BOOT"
-        BOOT_NEED_SIZE=$BOOT_SIZE
+        BOOT_NEED_SIZE="$BOOT_SIZE"
     else
         get_drive_size "$ROOTDRIVE" ROOTDRIVESPACE
         get_drive_size "$HOSTVGDRIVE" HOSTVGDRIVESPACE
@@ -172,7 +172,7 @@ check_partition_sizes()
         HOSTVG_NEED_SIZE=$(echo "scale=0;" \
                          "$SWAP_SIZE + $CONFIG_SIZE + $LOGGING_SIZE +
$min_data_size" | bc -l)

-        if [ $ROOTDRIVE == $HOSTVGDRIVE ]; then
+        if [ "$ROOTDRIVE" == "$HOSTVGDRIVE" ]; then
             drive_list="ROOT"
             ROOT_NEED_SIZE=$(echo "scale=0; $ROOT_SIZE * 2 +
$HOSTVG_NEED_SIZE"| bc -l)
         else
@@ -204,7 +204,7 @@ check_partition_sizes()
     # check if an existing HostVG exists on a device other than the target
     if [ "$OVIRT_ISCSI_ENABLED" != "y" ]; then
         devices="$(pvs -o pv_name,vg_name --noheadings | \
-            grep "HostVG"|grep -v $HOSTVGDRIVE|awk '{ print $1
}')"
+            grep "HostVG"|grep -v "$HOSTVGDRIVE"|awk '{
print $1 }')"
         rc=0
         if [ -n "$devices" ]; then
             printf "\n"
@@ -230,7 +230,7 @@ check_partition_sizes()
 manual_input()
 {
     local manual_device
-    local return_var=$1
+    local return_var="$1"
     while true; do
         read -rp "Enter disk device path: " manual_device
         if [ -z "$device" ]; then
@@ -239,7 +239,7 @@ manual_input()
         fi
         translate_multipath_device "$manual_device" manual_device
         if [ -n "$manual_device" ]; then
-            if [ -b "$(readlink -f $device)" ]; then
+            if [ -b "$(readlink -f "$device")" ]; then
                 eval $return_var="$manual_device"
                 return 0
             fi
@@ -273,7 +273,7 @@ get_dev_name()
         test "X$drive_type" = Xdisk || continue
         local block_dev=$(hal-get-property --udi "$d" --key
block.device)
         # Must start with a '/'.
-        case $block_dev in
+        case "$block_dev" in
             *' '*)
                 # we use space as separator
                 warn "block device name '$block_dev' contains
space; skipping";
@@ -288,7 +288,7 @@ get_dev_name()
     done
     d=""
     for d in $byid_list; do
-        devices="$devices `readlink -f $d`";
+        devices="$devices $(readlink -f "$d")";
     done

     # FIXME: workaround for detecting virtio block devices
@@ -296,7 +296,7 @@ get_dev_name()

     # FIXME: workaround for detecting cciss devices
     for dev in $(ls /dev/cciss 2>/dev/null); do
-        if [[ ! $dev =~ p[0-9]+\$ ]]; then
+        if [[ ! "$dev" =~ p[0-9]+\$ ]]; then
             devices="$devices /dev/cciss/dev"
         fi
     done
@@ -306,7 +306,7 @@ get_dev_name()
     for dev in $(dmsetup ls --target=multipath | awk '{print $1}'); do
         devices="$devices /dev/mapper/$dev"
         local sd_devs=""
-        get_multipath_devices $dev sd_devs
+        get_multipath_devices "$dev" sd_devs
         devs_to_remove="${devs_to_remove} ${sd_devs}"
     done

@@ -352,7 +352,7 @@ do_configure()
         printf "\n\nPlease select the disk to use for the Boot
partition.\n\n"
         BOOTDRIVE=$(get_dev_name) || return 0
         get_drive_size "$BOOTDRIVE" BOOTDRIVESPACE
-        echo $BOOTDRIVE
+        echo "$BOOTDRIVE"
     else
         printf "\n\nPlease select the disk to use for the Root.\n\n"
         ROOTDRIVE=$(get_dev_name) || return 0
@@ -361,13 +361,13 @@ do_configure()
         printf "\n\nPlease select the disk to use for the
HostVG.\n\n"
         HOSTVGDRIVE=$(get_dev_name) || return 0
         local skipped=false
-        if check_existing_hostvg $HOSTVGDRIVE devs; then
+        if check_existing_hostvg "$HOSTVGDRIVE" devs; then
             for dev in $devs
             do
                 printf "Removing HostVG on $dev will erase the drive and
cannot be undone\n"
                 if ask_yes_or_no "Do you want to remove HostVG from $dev
(y/n)?"; then
                     start_log
-                    if ! wipe_lvm_on_disk $dev; then
+                    if ! wipe_lvm_on_disk "$dev"; then
                         stop_log
                         return 1
                     fi
@@ -379,7 +379,7 @@ do_configure()
         fi
         $skipped  && printf "Installation cannot proceed with
existing HostVG.\n" && return 0
         get_drive_size "$HOSTVGDRIVE" HOSTVGDRIVESPACE
-        echo $HOSTVGDRIVESPACE
+        echo "$HOSTVGDRIVESPACE"
     fi
     printf "\n\nPlease configure storage partitions.\n\n"
     printf "* Enter partition sizes in MB.\n"
@@ -459,7 +459,7 @@ set /files$OVIRT_DEFAULTS/OVIRT_VOL_LOGGING_SIZE
$LOGGING_SIZE
 set /files$OVIRT_DEFAULTS/OVIRT_VOL_DATA_SIZE $DATA_SIZE
 EOF

-   if [ -n $BOOTDRIVE ]; then
+   if [ -n "$BOOTDRIVE" ]; then
        augtool <<EOF
 set /files$OVIRT_DEFAULTS/OVIRT_BOOT_INIT $BOOTDRIVE
 EOF
@@ -478,7 +478,7 @@ do_review()
         local data_size_display="$DATA_SIZE MB"
         if [ "$DATA_SIZE" = -1 ]; then

-            if [ $ROOTDRIVE == $HOSTVGDRIVE ]; then
+            if [ "$ROOTDRIVE" == "$HOSTVGDRIVE" ]; then
                 local remaining_mb=$(( $ROOTDRIVESPACE - $SWAP_SIZE \
                         - $ROOT_SIZE * 2 - $CONFIG_SIZE - $LOGGING_SIZE ))
                 test $remaining_mb -lt 0 && is_negative=1
@@ -529,7 +529,7 @@ check_existing_hostvg()
             grep "HostVG" | awk '{print $1}' )"
     else
         devices="$(pvs -o pv_name,vg_name --noheadings | \
-            grep -v ${install_dev} | grep "HostVG" | awk '{print
$1}' )"
+            grep -v "${install_dev}" | grep "HostVG" | awk
'{print $1}' )"
     fi
     rc=1
     if [ -n "$devices" ]; then
@@ -537,7 +537,7 @@ check_existing_hostvg()
         printf "There appears to already be an installation on another
device:\n"
         for device in $devices; do
             get_multipath_devices ${device%p[0-9]} sd_dev
-            sd_dev=$(echo $sd_dev | awk '{print $1}')
+            sd_dev=$(echo "$sd_dev" | awk '{print $1}')
             udi=$(hal-find-by-property --key block.device --string
/dev/${sd_dev})
             printf "\t$device ($(basename "$udi"))\n"
         done
@@ -546,7 +546,7 @@ check_existing_hostvg()
         rc=0
     fi

-    test -z $devices_var || eval $devices_var=$devices
+    test -z "$devices_var" || eval
"$devices_var"="$devices"

     return $rc

@@ -563,9 +563,9 @@ wipe_lvm_on_disk()
     if [[ "$dev" =~ "/dev/sd" ]]; then
         part_delim=""
     fi
-    for vg in $(pvs -o vg_name --noheadings $dev $dev${dev_delim}[0-9]*
2>/dev/null|sort -u); do
+    for vg in $(pvs -o vg_name --noheadings "$dev"
"$dev${dev_delim}[0-9]"* 2>/dev/null|sort -u); do
         if pvs -o pv_name,vg_name --noheadings | \
-            grep $vg | egrep -v -q "${dev}${part_delim}[0-9]+|${dev}"
2>/dev/null; then
+            grep "$vg" | egrep -v -q
"${dev}${part_delim}[0-9]+|${dev}" 2>/dev/null; then
             log "The volume group \"$vg\" spans multiple
disks."
             log "This operation cannot complete.  Please manually"
             log "cleanup the storage using standard linux tools."
@@ -583,7 +583,7 @@ reread_partitions()
     if [[ $drive =~ "/dev/mapper" ]]; then
         kpartx -a -p p $drive
     else
-        blockdev --rereadpt $drive
+        blockdev --rereadpt "$drive"
     fi
 }

@@ -617,19 +617,19 @@ perform_partitioning()
         log "Partitioning drive: $BOOTDRIVE"
         log "Wiping old boot sector"
         dd if=/dev/zero of=$BOOTDRIVE bs=1024K count=1
-        reread_partitions $BOOTDRIVE
-        partprobe -s $BOOTDRIVE
+        reread_partitions "$BOOTDRIVE"
+        partprobe -s "$BOOTDRIVE"
         log "Creating boot partition"
-        parted $BOOTDRIVE -s "mklabel ${LABEL_TYPE}"
-        parted $BOOTDRIVE -s "mkpartfs primary ext2 0M
${boot_size_si}M"
-        reread_partitions $BOOTDRIVE
-        partboot=$BOOTDRIVE1
-        if [ ! -e $partboot ]; then
-            partboot=${BOOTDRIVE}p1
+        parted "$BOOTDRIVE" -s "mklabel ${LABEL_TYPE}"
+        parted "$BOOTDRIVE" -s "mkpartfs primary ext2 0M
${boot_size_si}M"
+        reread_partitions "$BOOTDRIVE"
+        partboot="$BOOTDRIVE1"
+        if [ ! -e "$partboot" ]; then
+            partboot="${BOOTDRIVE}p1"
         fi
         # sleep to ensure filesystems are created before continuing
         sleep 10
-        mke2fs ${partboot} -L Boot
+        mke2fs "${partboot}" -L Boot
         tune2fs -c 0 -i 0 ${partboot}
         log "Completed!"
         return
@@ -639,58 +639,58 @@ perform_partitioning()
     log "Partitioning drive: $ROOTDRIVE"
  # FIXME: save a backup copy, just in case?
     log "Wiping old boot sector"
-    dd if=/dev/zero of=$ROOTDRIVE bs=1024K count=1
-    reread_partitions $ROOTDRIVE
-    partprobe -s $ROOTDRIVE
+    dd if=/dev/zero of="$ROOTDRIVE" bs=1024K count=1
+    reread_partitions "$ROOTDRIVE"
+    partprobe -s "$ROOTDRIVE"
     log "Labeling Drive: $ROOTDRIVE"
-    parted $ROOTDRIVE -s "mklabel ${LABEL_TYPE}"
+    parted "$ROOTDRIVE" -s "mklabel ${LABEL_TYPE}"

-    if [ $ROOTDRIVE != $HOSTVGDRIVE ]; then
+    if [ "$ROOTDRIVE" != "$HOSTVGDRIVE" ]; then
         log "Labeling Drive: $HOSTVGDRIVE"
-        parted $HOSTVGDRIVE -s "mklabel ${LABEL_TYPE}"
+        parted "$HOSTVGDRIVE" -s "mklabel ${LABEL_TYPE}"
     fi
     log "Creating Root and RootBackup Partitions"
     let RootBackup_end=${ROOT_SIZE}*2
     parted $ROOTDRIVE -s "mkpart primary ext2 0M ${ROOT_SIZE}M"
     parted $ROOTDRIVE -s "mkpart primary ext2 ${ROOT_SIZE}M
${RootBackup_end}M"
     reread_partitions $ROOTDRIVE
-    partroot=${ROOTDRIVE}1
-    partrootbackup=${ROOTDRIVE}2
-    if [ ! -e $partroot ]; then
-        partroot=${ROOTDRIVE}p1
-        partrootbackup=${ROOTDRIVE}p2
+    partroot="${ROOTDRIVE}1"
+    partrootbackup="${ROOTDRIVE}2"
+    if [ ! -e "$partroot" ]; then
+        partroot="${ROOTDRIVE}p1"
+        partrootbackup="${ROOTDRIVE}p2"
     fi
     # sleep to ensure filesystems are created before continuing
     sleep 10
-    mke2fs ${partroot} -L Root
-    mke2fs ${partrootbackup} -L RootBackup
-    tune2fs -c 0 -i 0 ${partroot}
-    tune2fs -c 0 -i 0 ${partrootbackup}
+    mke2fs "${partroot}" -L Root
+    mke2fs "${partrootbackup}" -L RootBackup
+    tune2fs -c 0 -i 0 "${partroot}"
+    tune2fs -c 0 -i 0 "${partrootbackup}"
     log "Creating LVM partition"

-    if [ $ROOTDRIVE == $HOSTVGDRIVE ]; then
+    if [ "$ROOTDRIVE" == "$HOSTVGDRIVE" ]; then
         parted $HOSTVGDRIVE -s "mkpart primary ext2 ${RootBackup_end}M
-1"
         hostvgpart="3"
     else
-        parted $HOSTVGDRIVE -s "mkpart primary ext2 0M -1"
+        parted "$HOSTVGDRIVE" -s "mkpart primary ext2 0M
-1"
         hostvgpart="1"
     fi
     log "Toggling LVM on"
-    parted $HOSTVGDRIVE -s "set $hostvgpart lvm on"
-    parted $ROOTDRIVE -s "print"
+    parted "$HOSTVGDRIVE" -s "set $hostvgpart lvm on"
+    parted "$ROOTDRIVE" -s "print"
     udevadm settle 2> /dev/null || udevsettle
-    reread_partitions $HOSTVGDRIVE
+    reread_partitions "$HOSTVGDRIVE"

     # sync GPT to the legacy MBR partitions
     if [ "gpt" == "$LABEL_TYPE" ]; then
         log "Running gptsync to create legacy mbr"
-        gptsync $ROOTDRIVE
+        gptsync "$ROOTDRIVE"
     fi

-    partpv=${HOSTVGDRIVE}${hostvgpart}
+    partpv="${HOSTVGDRIVE}${hostvgpart}"
     if [ ! -e "$partpv" ]; then
         # e.g. /dev/cciss/c0d0p2
-        partpv=${HOSTVGDRIVE}p${hostvgpart}
+        partpv="${HOSTVGDRIVE}p${hostvgpart}"
     fi
     log "Creating physical volume"
     if [ ! -e "$partpv" ]; then
@@ -806,18 +806,18 @@ while true; do
     for OPTION in "$@"; do
         while true; do
             read -ep "Enter $OPTION: "
-            if [[ $REPLY == "q" || $REPLY == "Q" ]]; then
+            if [[ "$REPLY" == "q" || "$REPLY" ==
"Q" ]]; then
                 return
             fi

             if [ "$OPTION" == "Target IP" ]; then
-                OVIRT_ISCSI_TARGET_IP=$REPLY
+                OVIRT_ISCSI_TARGET_IP="$REPLY"
                 if [ -n "$REPLY" ]; then
                     break;
                 fi

             elif [ "$OPTION" == "Target Port" ]; then
-                OVIRT_ISCSI_TARGET_PORT=$REPLY
+                OVIRT_ISCSI_TARGET_PORT="$REPLY"
                 if [ -z "$REPLY" ]; then
                     OVIRT_ISCSI_TARGET_PORT="3260"
                     break;
@@ -826,11 +826,11 @@ while true; do
                 fi

             elif [ "$OPTION" == "CHAP Username" ]; then
-                OVIRT_ISCSI_CHAP_USERNAME=$REPLY
+                OVIRT_ISCSI_CHAP_USERNAME="$REPLY"
                 break

             elif [ "$OPTION" == "CHAP Password" ]; then
-                OVIRT_ISCSI_CHAP_PASSWORD=$REPLY
+                OVIRT_ISCSI_CHAP_PASSWORD="$REPLY"
                 break;
             fi
         done
@@ -868,7 +868,7 @@ set /files/etc/default/ovirt/OVIRT_ISCSI_CHAP_PASSWORD
$OVIRT_ISCSI_CHAP_PASSWOR
 EOF
     fi

-    iscsiadm -p $OVIRT_ISCSI_TARGET_IP:$OVIRT_ISCSI_TARGET_PORT -m discovery -t
sendtargets
+    iscsiadm -p "$OVIRT_ISCSI_TARGET_IP:$OVIRT_ISCSI_TARGET_PORT" -m
discovery -t sendtargets
     log "Restarting iscsi service"
     service iscsi restart

@@ -927,8 +927,8 @@ if [ -n "$OVIRT_INIT" ]; then
     # if present, use the drive selected with 'ovirt_init' boot
parameter
     # setting these the same until kernel cmdline argument implemented
     translate_multipath_device "$OVIRT_INIT" DRIVE
-    ROOTDRIVE=$DRIVE
-    HOSTVGDRIVE=$DRIVE
+    ROOTDRIVE="$DRIVE"
+    HOSTVGDRIVE="$DRIVE"
     get_drive_size "$ROOTDRIVE" ROOTDRIVESPACE
 fi

@@ -943,7 +943,7 @@ if [ "$1" == "AUTO" ]; then
     log "Beginning automatic disk partitioning.\n"
     if [ -n "$OVIRT_INIT" ]; then
         # do not format if HostVG exists on selected disk...
-        check_existing_hostvg $HOSTVGDRIVE
+        check_existing_hostvg "$HOSTVGDRIVE"
         existingHostVG=$?
         # ... unless overridden by ovirt_firstboot parameter
         if is_firstboot || [ $existingHostVG -ne 0 ]; then
diff --git a/scripts/ovirt-functions b/scripts/ovirt-functions
index d8aa008..edf6267 100644
--- a/scripts/ovirt-functions
+++ b/scripts/ovirt-functions
@@ -148,7 +148,7 @@ EOF
 #
 wipe_volume_group()
 {
-    vg=$1
+    vg="$1"

     for d in $(grep $vg /proc/mounts|awk '{print $2}'); do
         log "Unmounting $d"
@@ -156,10 +156,10 @@ wipe_volume_group()
     done
     for d in $(grep $vg /proc/swaps|awk '{print $1}'); do
         log "Turning off $d"
-        swapoff $d
+        swapoff "$d"
     done
     log "Removing $vg"
-    vgremove -f $vg
+    vgremove -f "$vg"
 }

 # find_srv SERVICE PROTO
@@ -719,10 +719,10 @@ reboot() {
     if [ "$OVIRT_ISCSI_ENABLED" = "yes" ]; then
         # setup new Root if update is prepared
         if findfs LABEL=RootUpdate 2>&1 >/dev/null; then
-            root_update_dev=$(findfs LABEL=RootUpdate 2>/dev/null)
-            root_dev=$(findfs LABEL=Root 2>/dev/null)
-            e2label $root_dev RootBackup
-            e2label $root_update_dev Root
+            root_update_dev="$(findfs LABEL=RootUpdate
2>/dev/null)"
+            root_dev="$(findfs LABEL=Root 2>/dev/null)"
+            e2label "$root_dev" RootBackup
+            e2label "$root_update_dev" Root
         fi
     fi
     # run post-install hooks
@@ -812,12 +812,12 @@ network_up () {

 # Cleans partition tables
 wipe_partitions() {
-    local drive=$1
+    local drive="$1"
     log "Wiping old boot sector"
-    dd if=/dev/zero of=$drive bs=1024K count=1
+    dd if=/dev/zero of="$drive" bs=1024K count=1
     # zero out the GPT secondary header
     log "Wiping secondary gpt header"
-    local disk_kb_count=$(sfdisk -s $drive 2>/dev/null)
+    local disk_kb_count=$(sfdisk -s "$drive" 2>/dev/null)
     dd if=/dev/zero of=$drive bs=1024 seek=$(($disk_kb_count - 1)) count=1
 }

@@ -840,35 +840,35 @@ test_ntp_configuration () {

 get_dm_device ()
 {
-    local device=$1
+    local device="$1"
     local return_var=$2
-    major=$(stat -c '%t' $(readlink -f $device))
-    minor=$(stat -c '%T' $(readlink -f $device))
+    major=$(stat -c '%t' $(readlink -f "$device"))
+    minor=$(stat -c '%T' $(readlink -f "$device"))
     local dm_device     local rc=1
     for dm in /dev/mapper/*; do
-        if [ $major = $(stat -c '%t' $dm) -a \
-            $minor = $(stat -c '%T' $dm) ]; then
-            local dm_device=$dm
+        if [ $major = $(stat -c '%t' "$dm") -a \
+            $minor = $(stat -c '%T' "$dm") ]; then
+            local dm_device="$dm"
             rc=0
             break
         fi
     done

-    eval $return_var=$dm_device
+    eval $return_var="$dm_device"

     return $rc
 }

 #Function to determine partition and device names
 get_part_info() {
-    local drive_in=$1
+    local drive_in="$1"
     local dev_var=$2
     local part_var=$3
     local devname_1 devname2 part_number
     local rc=0

-    eval $(readlink -f $drive_in|awk {'
+    eval $(readlink -f "$drive_in" |awk {'
         print "devname_1=" substr($1,1,length($1)-1);
         print "devname_2=" substr($1,1,length($1)-2);
         part_number=substr($1,length($1),1);
-- 
1.6.6.1

On Wed, Mar 31, 2010 at 11:48:17AM -0400, Mike Burns wrote:

Comments inline.
> Quote everywhere that we reference wwid in o-c-storage, o-c-boot
> and ovirt-functions.
> 
> Signed-off-by: Mike Burns <mburns at redhat.com>
> ---
>  scripts/ovirt-config-boot    |    6 +-
>  scripts/ovirt-config-storage |  178
+++++++++++++++++++++---------------------
>  scripts/ovirt-functions      |   38 +++++-----
>  3 files changed, 111 insertions(+), 111 deletions(-)
> 
> diff --git a/scripts/ovirt-config-boot b/scripts/ovirt-config-boot
> index d545878..347fc18 100755
> --- a/scripts/ovirt-config-boot
> +++ b/scripts/ovirt-config-boot
> @@ -88,7 +88,7 @@ ovirt_boot_setup() {
>              rc=0
>          else
>              candidate_dev=$(findfs LABEL=$candidate 2>/dev/null)
> -            e2label $candidate_dev RootNew
> +            e2label "$candidate_dev" RootNew
>              rc=$?
>          fi
>          if [ $rc -ne 0 ]; then
> @@ -97,7 +97,7 @@ ovirt_boot_setup() {
>            return $rc
>          fi
> 
> -        mount $candidate_dev /liveos/
> +        mount "$candidate_dev" /liveos/
> 
>          rm -rf /liveos/LiveOS
>          mkdir -p /liveos/LiveOS
> @@ -175,7 +175,7 @@ EOF
>      if [ "$OVIRT_ISCSI_ENABLED" != "y" ]; then
>          umount /liveos
>          # mark new Root ready to go, reboot() in ovirt-function switches
it to active
> -        e2label $candidate_dev RootUpdate
> +        e2label "$candidate_dev" RootUpdate
>      fi
> 
>      rm -rf $tmpdir
> diff --git a/scripts/ovirt-config-storage b/scripts/ovirt-config-storage
> index 8d59a6b..9fee10e 100755
> --- a/scripts/ovirt-config-storage
> +++ b/scripts/ovirt-config-storage
> @@ -36,12 +36,12 @@ swap_min_size=5
> 
>  # return sd name for given #:# identifier
>  get_sd_name() {
> -    local id=$1
> -    local device_var=$2
> +    local id="$1"
> +    local device_var="$2"
>      local device_sys=$(grep -H "^$id$" /sys/block/*/dev | cut
-d: -f1)
> 
>      if [ -n "$device_sys" ]; then
> -        eval $device_var=$(basename $(dirname $device_sys))
> +        eval $device_var=$(basename $(dirname "$device_sys"))
>          return
>      fi
>      eval $device_var=1
> @@ -49,23 +49,23 @@ get_sd_name() {
> 
>  # gets the dependent block devices for multipath devices
>  get_multipath_devices() {
> -    local mpath_device=mpath-$(basename $1)
> -    local deplist_var=$2
> +    local mpath_device=mpath-$(basename "$1")
> +    local deplist_var="$2"
>      local deplist=""
> 
>      #get dependencies for multipath device
> -    local deps=$(dmsetup deps -u $mpath_device \
> +    local deps=$(dmsetup deps -u "$mpath_device" \
>      | sed -r 's/\(([0-9]+), ([0-9]+)\)/\1:\2/g' \
>      | sed 's/ /\n/g' | grep [0-9]:[0-9] )
> 
>      local device=""
> -    for dep in $deps
> +    for dep in "$deps"
>      do
>          local device=""
> -        get_sd_name $dep device
> -        if [[ ! $device = 1 ]]; then
> -            if [[ $deplist = "" ]]; then
> -                deplist=$device
> +        get_sd_name "$dep" device
> +        if [[ ! "$device" = 1 ]]; then
> +            if [[ "$deplist" = "" ]]; then
> +                deplist="$device"
>              else
>                  deplist="$deplist $device"
>              fi
> @@ -79,8 +79,8 @@ get_multipath_devices() {
>  #return sdX device if no multipath device
>  translate_multipath_device() {
>      #trim so that only sdX is stored, but support passing /dev/sdX
> -    local dev=$1
> -    local mpath_var=$2
> +    local dev="$1"
> +    local mpath_var="$2"
> 
>      if [ -z "$dev" ]; then
>          if [ -n "$mpath_var" ]; then
> @@ -89,36 +89,36 @@ translate_multipath_device() {
>          return
>      fi
>      if [[ "$dev" =~ "/dev/mapper" ]]; then
> -        eval $mpath_var=$dev
> +        eval $math_var="$dev"
>          return
>      fi
> 
> -    local dm_dev=/dev/$(multipath -ll $dev | egrep dm-[0-9]+ | sed -r
's/^.& (dm-[0-9]+) .*$/\1/')
> +    local dm_dev=/dev/$(multipath -ll "$dev" | egrep dm-[0-9]+ |
sed -r 's/^.& (dm-[0-9]+) .*$/\1/')
> 
>      local mpath_device> -    get_dm_device $dm_dev mpath_device
> +    get_dm_device "$dm_dev" mpath_device
> 
>      if [ -z "$mpath_device" ]; then
> -        mpath_device=$dev
> +        mpath_device="$dev"
>      fi
> 
> -    eval $mpath_var=$mpath_device
> +    eval $mpath_var="$mpath_device"
>  }
> 
> 
>  get_drive_size()
>  {
> -    local drive=$1
> -    local space_var=$2
> +    local drive="$1"
> +    local space_var="$2"
> 
>      local size> -    local udi=$(hal-find-by-property --key
block.device --string $drive)
> +    local udi=$(hal-find-by-property --key block.device --string
"$drive")
>      # if more than one UDI was found then iterate over them to find the
base device
>      if [[ "${udi}" =~ \$  ]]; then
> -        udi=$(echo $udi | sed 's/\$/ /g')
> +        udi=$(echo "$udi" | sed 's/\$/ /g')
>          for found in ${udi}; do
> -            if [[ "false" == $(hal-get-property --udi $found
--key block.is_volume) ]]; then
> -                udi=$found
> +            if [[ "false" == $(hal-get-property --udi
"$found" --key block.is_volume) ]]; then
> +                udi="$found"
>                  break
>              fi
>          done
> @@ -127,7 +127,7 @@ get_drive_size()
>          # If hal didn't find the device, it could be a virtio block
device
>          # or a multipath device
>          # In this case, use sfdisk -s to get the size
> -        size=$(sfdisk -s $drive 2>/dev/null)
> +        size=$(sfdisk -s "$drive" 2>/dev/null)
>          if [ -z "$size" ]; then
>              size=0
>          fi
> @@ -147,7 +147,7 @@ get_drive_size()
>      echo "$drive ($size MB)"
>      test -z "$udi" || echo "Disk Identifier: $(basename
"$udi")"
>      if [ -n "$space_var" ]; then
> -        eval $space_var=$size
> +        eval $space_var="$size"
>      fi
>  }
> 
> @@ -155,7 +155,7 @@ check_partition_sizes()
>  {
>      local disk_size need_size
> 
> -    local min_data_size=$DATA_SIZE
> +    local min_data_size="$DATA_SIZE"
>      if [ "$DATA_SIZE" = -1 ]; then
>          min_data_size=5
>      fi
> @@ -164,7 +164,7 @@ check_partition_sizes()
>      if [ "$OVIRT_ISCSI_ENABLED" == "y" ]; then
>          get_drive_size "$BOOTDRIVE" BOOTDRIVESPACE
>          drive_list="BOOT"
> -        BOOT_NEED_SIZE=$BOOT_SIZE
> +        BOOT_NEED_SIZE="$BOOT_SIZE"
>      else
>          get_drive_size "$ROOTDRIVE" ROOTDRIVESPACE
>          get_drive_size "$HOSTVGDRIVE" HOSTVGDRIVESPACE
> @@ -172,7 +172,7 @@ check_partition_sizes()
>          HOSTVG_NEED_SIZE=$(echo "scale=0;" \
>                           "$SWAP_SIZE + $CONFIG_SIZE + $LOGGING_SIZE +
$min_data_size" | bc -l)
> 
> -        if [ $ROOTDRIVE == $HOSTVGDRIVE ]; then
> +        if [ "$ROOTDRIVE" == "$HOSTVGDRIVE" ]; then
>              drive_list="ROOT"
>              ROOT_NEED_SIZE=$(echo "scale=0; $ROOT_SIZE * 2 +
$HOSTVG_NEED_SIZE"| bc -l)
>          else
> @@ -204,7 +204,7 @@ check_partition_sizes()
>      # check if an existing HostVG exists on a device other than the target
>      if [ "$OVIRT_ISCSI_ENABLED" != "y" ]; then
>          devices="$(pvs -o pv_name,vg_name --noheadings | \
> -            grep "HostVG"|grep -v $HOSTVGDRIVE|awk '{ print
$1 }')"
> +            grep "HostVG"|grep -v "$HOSTVGDRIVE"|awk
'{ print $1 }')"
>          rc=0
>          if [ -n "$devices" ]; then
>              printf "\n"
> @@ -230,7 +230,7 @@ check_partition_sizes()
>  manual_input()
>  {
>      local manual_device
> -    local return_var=$1
> +    local return_var="$1"
>      while true; do
>          read -rp "Enter disk device path: " manual_device
>          if [ -z "$device" ]; then
> @@ -239,7 +239,7 @@ manual_input()
>          fi
>          translate_multipath_device "$manual_device"
manual_device
>          if [ -n "$manual_device" ]; then
> -            if [ -b "$(readlink -f $device)" ]; then
> +            if [ -b "$(readlink -f "$device")" ]; then
To avoid parsing errors, I'd change the double quotes to single here:
+            if [ -b "$(readline -f '$device')" ]; then
>                  eval $return_var="$manual_device"
>                  return 0
>              fi
> @@ -273,7 +273,7 @@ get_dev_name()
>          test "X$drive_type" = Xdisk || continue
>          local block_dev=$(hal-get-property --udi "$d" --key
block.device)
>          # Must start with a '/'.
> -        case $block_dev in
> +        case "$block_dev" in
>              *' '*)
>                  # we use space as separator
>                  warn "block device name '$block_dev' contains
space; skipping";
> @@ -288,7 +288,7 @@ get_dev_name()
>      done
>      d=""
>      for d in $byid_list; do
> -        devices="$devices `readlink -f $d`";
> +        devices="$devices $(readlink -f "$d")";
Same here regarding the using multiple double quotes in a line:
+        devices="$devices $(readlink -f '$d')";
>      done
> 
>      # FIXME: workaround for detecting virtio block devices
> @@ -296,7 +296,7 @@ get_dev_name()
> 
>      # FIXME: workaround for detecting cciss devices
>      for dev in $(ls /dev/cciss 2>/dev/null); do
> -        if [[ ! $dev =~ p[0-9]+\$ ]]; then
> +        if [[ ! "$dev" =~ p[0-9]+\$ ]]; then
>              devices="$devices /dev/cciss/dev"
>          fi
>      done
> @@ -306,7 +306,7 @@ get_dev_name()
>      for dev in $(dmsetup ls --target=multipath | awk '{print
$1}'); do
>          devices="$devices /dev/mapper/$dev"
>          local sd_devs=""
> -        get_multipath_devices $dev sd_devs
> +        get_multipath_devices "$dev" sd_devs
>          devs_to_remove="${devs_to_remove} ${sd_devs}"
>      done
> 
> @@ -352,7 +352,7 @@ do_configure()
>          printf "\n\nPlease select the disk to use for the Boot
partition.\n\n"
>          BOOTDRIVE=$(get_dev_name) || return 0
>          get_drive_size "$BOOTDRIVE" BOOTDRIVESPACE
> -        echo $BOOTDRIVE
> +        echo "$BOOTDRIVE"
>      else
>          printf "\n\nPlease select the disk to use for the
Root.\n\n"
>          ROOTDRIVE=$(get_dev_name) || return 0
> @@ -361,13 +361,13 @@ do_configure()
>          printf "\n\nPlease select the disk to use for the
HostVG.\n\n"
>          HOSTVGDRIVE=$(get_dev_name) || return 0
>          local skipped=false
> -        if check_existing_hostvg $HOSTVGDRIVE devs; then
> +        if check_existing_hostvg "$HOSTVGDRIVE" devs; then
>              for dev in $devs
>              do
>                  printf "Removing HostVG on $dev will erase the drive
and cannot be undone\n"
>                  if ask_yes_or_no "Do you want to remove HostVG from
$dev (y/n)?"; then
>                      start_log
> -                    if ! wipe_lvm_on_disk $dev; then
> +                    if ! wipe_lvm_on_disk "$dev"; then
>                          stop_log
>                          return 1
>                      fi
> @@ -379,7 +379,7 @@ do_configure()
>          fi
>          $skipped  && printf "Installation cannot proceed with
existing HostVG.\n" && return 0
>          get_drive_size "$HOSTVGDRIVE" HOSTVGDRIVESPACE
> -        echo $HOSTVGDRIVESPACE
> +        echo "$HOSTVGDRIVESPACE"
>      fi
>      printf "\n\nPlease configure storage partitions.\n\n"
>      printf "* Enter partition sizes in MB.\n"
> @@ -459,7 +459,7 @@ set /files$OVIRT_DEFAULTS/OVIRT_VOL_LOGGING_SIZE
$LOGGING_SIZE
>  set /files$OVIRT_DEFAULTS/OVIRT_VOL_DATA_SIZE $DATA_SIZE
>  EOF
> 
> -   if [ -n $BOOTDRIVE ]; then
> +   if [ -n "$BOOTDRIVE" ]; then
>         augtool <<EOF
>  set /files$OVIRT_DEFAULTS/OVIRT_BOOT_INIT $BOOTDRIVE
>  EOF
> @@ -478,7 +478,7 @@ do_review()
>          local data_size_display="$DATA_SIZE MB"
>          if [ "$DATA_SIZE" = -1 ]; then
> 
> -            if [ $ROOTDRIVE == $HOSTVGDRIVE ]; then
> +            if [ "$ROOTDRIVE" == "$HOSTVGDRIVE" ];
then
>                  local remaining_mb=$(( $ROOTDRIVESPACE - $SWAP_SIZE \
>                          - $ROOT_SIZE * 2 - $CONFIG_SIZE - $LOGGING_SIZE ))
>                  test $remaining_mb -lt 0 && is_negative=1
> @@ -529,7 +529,7 @@ check_existing_hostvg()
>              grep "HostVG" | awk '{print $1}' )"
>      else
>          devices="$(pvs -o pv_name,vg_name --noheadings | \
> -            grep -v ${install_dev} | grep "HostVG" | awk
'{print $1}' )"
> +            grep -v "${install_dev}" | grep "HostVG" |
awk '{print $1}' )"
>      fi
>      rc=1
>      if [ -n "$devices" ]; then
> @@ -537,7 +537,7 @@ check_existing_hostvg()
>          printf "There appears to already be an installation on
another device:\n"
>          for device in $devices; do
>              get_multipath_devices ${device%p[0-9]} sd_dev
> -            sd_dev=$(echo $sd_dev | awk '{print $1}')
> +            sd_dev=$(echo "$sd_dev" | awk '{print $1}')
>              udi=$(hal-find-by-property --key block.device --string
/dev/${sd_dev})
>              printf "\t$device ($(basename "$udi"))\n"
>          done
> @@ -546,7 +546,7 @@ check_existing_hostvg()
>          rc=0
>      fi
> 
> -    test -z $devices_var || eval $devices_var=$devices
> +    test -z "$devices_var" || eval
"$devices_var"="$devices"
> 
>      return $rc
> 
> @@ -563,9 +563,9 @@ wipe_lvm_on_disk()
>      if [[ "$dev" =~ "/dev/sd" ]]; then
>          part_delim=""
>      fi
> -    for vg in $(pvs -o vg_name --noheadings $dev $dev${dev_delim}[0-9]*
2>/dev/null|sort -u); do
> +    for vg in $(pvs -o vg_name --noheadings "$dev"
"$dev${dev_delim}[0-9]"* 2>/dev/null|sort -u); do
>          if pvs -o pv_name,vg_name --noheadings | \
> -            grep $vg | egrep -v -q
"${dev}${part_delim}[0-9]+|${dev}" 2>/dev/null; then
> +            grep "$vg" | egrep -v -q
"${dev}${part_delim}[0-9]+|${dev}" 2>/dev/null; then
>              log "The volume group \"$vg\" spans multiple
disks."
>              log "This operation cannot complete.  Please
manually"
>              log "cleanup the storage using standard linux
tools."
> @@ -583,7 +583,7 @@ reread_partitions()
>      if [[ $drive =~ "/dev/mapper" ]]; then
>          kpartx -a -p p $drive
>      else
> -        blockdev --rereadpt $drive
> +        blockdev --rereadpt "$drive"
>      fi
>  }
> 
> @@ -617,19 +617,19 @@ perform_partitioning()
>          log "Partitioning drive: $BOOTDRIVE"
>          log "Wiping old boot sector"
>          dd if=/dev/zero of=$BOOTDRIVE bs=1024K count=1
> -        reread_partitions $BOOTDRIVE
> -        partprobe -s $BOOTDRIVE
> +        reread_partitions "$BOOTDRIVE"
> +        partprobe -s "$BOOTDRIVE"
>          log "Creating boot partition"
> -        parted $BOOTDRIVE -s "mklabel ${LABEL_TYPE}"
> -        parted $BOOTDRIVE -s "mkpartfs primary ext2 0M
${boot_size_si}M"
> -        reread_partitions $BOOTDRIVE
> -        partboot=$BOOTDRIVE1
> -        if [ ! -e $partboot ]; then
> -            partboot=${BOOTDRIVE}p1
> +        parted "$BOOTDRIVE" -s "mklabel ${LABEL_TYPE}"
> +        parted "$BOOTDRIVE" -s "mkpartfs primary ext2 0M
${boot_size_si}M"
> +        reread_partitions "$BOOTDRIVE"
> +        partboot="$BOOTDRIVE1"
> +        if [ ! -e "$partboot" ]; then
> +            partboot="${BOOTDRIVE}p1"
>          fi
>          # sleep to ensure filesystems are created before continuing
>          sleep 10
> -        mke2fs ${partboot} -L Boot
> +        mke2fs "${partboot}" -L Boot
>          tune2fs -c 0 -i 0 ${partboot}
>          log "Completed!"
>          return
> @@ -639,58 +639,58 @@ perform_partitioning()
>      log "Partitioning drive: $ROOTDRIVE"
>   # FIXME: save a backup copy, just in case?
>      log "Wiping old boot sector"
> -    dd if=/dev/zero of=$ROOTDRIVE bs=1024K count=1
> -    reread_partitions $ROOTDRIVE
> -    partprobe -s $ROOTDRIVE
> +    dd if=/dev/zero of="$ROOTDRIVE" bs=1024K count=1
> +    reread_partitions "$ROOTDRIVE"
> +    partprobe -s "$ROOTDRIVE"
>      log "Labeling Drive: $ROOTDRIVE"
> -    parted $ROOTDRIVE -s "mklabel ${LABEL_TYPE}"
> +    parted "$ROOTDRIVE" -s "mklabel ${LABEL_TYPE}"
> 
> -    if [ $ROOTDRIVE != $HOSTVGDRIVE ]; then
> +    if [ "$ROOTDRIVE" != "$HOSTVGDRIVE" ]; then
>          log "Labeling Drive: $HOSTVGDRIVE"
> -        parted $HOSTVGDRIVE -s "mklabel ${LABEL_TYPE}"
> +        parted "$HOSTVGDRIVE" -s "mklabel
${LABEL_TYPE}"
>      fi
>      log "Creating Root and RootBackup Partitions"
>      let RootBackup_end=${ROOT_SIZE}*2
>      parted $ROOTDRIVE -s "mkpart primary ext2 0M ${ROOT_SIZE}M"
>      parted $ROOTDRIVE -s "mkpart primary ext2 ${ROOT_SIZE}M
${RootBackup_end}M"
>      reread_partitions $ROOTDRIVE
> -    partroot=${ROOTDRIVE}1
> -    partrootbackup=${ROOTDRIVE}2
> -    if [ ! -e $partroot ]; then
> -        partroot=${ROOTDRIVE}p1
> -        partrootbackup=${ROOTDRIVE}p2
> +    partroot="${ROOTDRIVE}1"
> +    partrootbackup="${ROOTDRIVE}2"
> +    if [ ! -e "$partroot" ]; then
> +        partroot="${ROOTDRIVE}p1"
> +        partrootbackup="${ROOTDRIVE}p2"
>      fi
>      # sleep to ensure filesystems are created before continuing
>      sleep 10
> -    mke2fs ${partroot} -L Root
> -    mke2fs ${partrootbackup} -L RootBackup
> -    tune2fs -c 0 -i 0 ${partroot}
> -    tune2fs -c 0 -i 0 ${partrootbackup}
> +    mke2fs "${partroot}" -L Root
> +    mke2fs "${partrootbackup}" -L RootBackup
> +    tune2fs -c 0 -i 0 "${partroot}"
> +    tune2fs -c 0 -i 0 "${partrootbackup}"
>      log "Creating LVM partition"
> 
> -    if [ $ROOTDRIVE == $HOSTVGDRIVE ]; then
> +    if [ "$ROOTDRIVE" == "$HOSTVGDRIVE" ]; then
>          parted $HOSTVGDRIVE -s "mkpart primary ext2
${RootBackup_end}M -1"
>          hostvgpart="3"
>      else
> -        parted $HOSTVGDRIVE -s "mkpart primary ext2 0M -1"
> +        parted "$HOSTVGDRIVE" -s "mkpart primary ext2 0M
-1"
>          hostvgpart="1"
>      fi
>      log "Toggling LVM on"
> -    parted $HOSTVGDRIVE -s "set $hostvgpart lvm on"
> -    parted $ROOTDRIVE -s "print"
> +    parted "$HOSTVGDRIVE" -s "set $hostvgpart lvm on"
> +    parted "$ROOTDRIVE" -s "print"
>      udevadm settle 2> /dev/null || udevsettle
> -    reread_partitions $HOSTVGDRIVE
> +    reread_partitions "$HOSTVGDRIVE"
> 
>      # sync GPT to the legacy MBR partitions
>      if [ "gpt" == "$LABEL_TYPE" ]; then
>          log "Running gptsync to create legacy mbr"
> -        gptsync $ROOTDRIVE
> +        gptsync "$ROOTDRIVE"
>      fi
> 
> -    partpv=${HOSTVGDRIVE}${hostvgpart}
> +    partpv="${HOSTVGDRIVE}${hostvgpart}"
>      if [ ! -e "$partpv" ]; then
>          # e.g. /dev/cciss/c0d0p2
> -        partpv=${HOSTVGDRIVE}p${hostvgpart}
> +        partpv="${HOSTVGDRIVE}p${hostvgpart}"
>      fi
>      log "Creating physical volume"
>      if [ ! -e "$partpv" ]; then
> @@ -806,18 +806,18 @@ while true; do
>      for OPTION in "$@"; do
>          while true; do
>              read -ep "Enter $OPTION: "
> -            if [[ $REPLY == "q" || $REPLY == "Q" ]];
then
> +            if [[ "$REPLY" == "q" ||
"$REPLY" == "Q" ]]; then
>                  return
>              fi
> 
>              if [ "$OPTION" == "Target IP" ]; then
> -                OVIRT_ISCSI_TARGET_IP=$REPLY
> +                OVIRT_ISCSI_TARGET_IP="$REPLY"
>                  if [ -n "$REPLY" ]; then
>                      break;
>                  fi
> 
>              elif [ "$OPTION" == "Target Port" ]; then
> -                OVIRT_ISCSI_TARGET_PORT=$REPLY
> +                OVIRT_ISCSI_TARGET_PORT="$REPLY"
>                  if [ -z "$REPLY" ]; then
>                      OVIRT_ISCSI_TARGET_PORT="3260"
>                      break;
> @@ -826,11 +826,11 @@ while true; do
>                  fi
> 
>              elif [ "$OPTION" == "CHAP Username" ];
then
> -                OVIRT_ISCSI_CHAP_USERNAME=$REPLY
> +                OVIRT_ISCSI_CHAP_USERNAME="$REPLY"
>                  break
> 
>              elif [ "$OPTION" == "CHAP Password" ];
then
> -                OVIRT_ISCSI_CHAP_PASSWORD=$REPLY
> +                OVIRT_ISCSI_CHAP_PASSWORD="$REPLY"
>                  break;
>              fi
>          done
> @@ -868,7 +868,7 @@ set /files/etc/default/ovirt/OVIRT_ISCSI_CHAP_PASSWORD
$OVIRT_ISCSI_CHAP_PASSWOR
>  EOF
>      fi
> 
> -    iscsiadm -p $OVIRT_ISCSI_TARGET_IP:$OVIRT_ISCSI_TARGET_PORT -m
discovery -t sendtargets
> +    iscsiadm -p
"$OVIRT_ISCSI_TARGET_IP:$OVIRT_ISCSI_TARGET_PORT" -m discovery -t
sendtargets
>      log "Restarting iscsi service"
>      service iscsi restart
> 
> @@ -927,8 +927,8 @@ if [ -n "$OVIRT_INIT" ]; then
>      # if present, use the drive selected with 'ovirt_init' boot
parameter
>      # setting these the same until kernel cmdline argument implemented
>      translate_multipath_device "$OVIRT_INIT" DRIVE
> -    ROOTDRIVE=$DRIVE
> -    HOSTVGDRIVE=$DRIVE
> +    ROOTDRIVE="$DRIVE"
> +    HOSTVGDRIVE="$DRIVE"
>      get_drive_size "$ROOTDRIVE" ROOTDRIVESPACE
>  fi
> 
> @@ -943,7 +943,7 @@ if [ "$1" == "AUTO" ]; then
>      log "Beginning automatic disk partitioning.\n"
>      if [ -n "$OVIRT_INIT" ]; then
>          # do not format if HostVG exists on selected disk...
> -        check_existing_hostvg $HOSTVGDRIVE
> +        check_existing_hostvg "$HOSTVGDRIVE"
>          existingHostVG=$?
>          # ... unless overridden by ovirt_firstboot parameter
>          if is_firstboot || [ $existingHostVG -ne 0 ]; then
> diff --git a/scripts/ovirt-functions b/scripts/ovirt-functions
> index d8aa008..edf6267 100644
> --- a/scripts/ovirt-functions
> +++ b/scripts/ovirt-functions
> @@ -148,7 +148,7 @@ EOF
>  #
>  wipe_volume_group()
>  {
> -    vg=$1
> +    vg="$1"
> 
>      for d in $(grep $vg /proc/mounts|awk '{print $2}'); do
>          log "Unmounting $d"
> @@ -156,10 +156,10 @@ wipe_volume_group()
>      done
>      for d in $(grep $vg /proc/swaps|awk '{print $1}'); do
>          log "Turning off $d"
> -        swapoff $d
> +        swapoff "$d"
>      done
>      log "Removing $vg"
> -    vgremove -f $vg
> +    vgremove -f "$vg"
>  }
> 
>  # find_srv SERVICE PROTO
> @@ -719,10 +719,10 @@ reboot() {
>      if [ "$OVIRT_ISCSI_ENABLED" = "yes" ]; then
>          # setup new Root if update is prepared
>          if findfs LABEL=RootUpdate 2>&1 >/dev/null; then
> -            root_update_dev=$(findfs LABEL=RootUpdate 2>/dev/null)
> -            root_dev=$(findfs LABEL=Root 2>/dev/null)
> -            e2label $root_dev RootBackup
> -            e2label $root_update_dev Root
> +            root_update_dev="$(findfs LABEL=RootUpdate
2>/dev/null)"
> +            root_dev="$(findfs LABEL=Root 2>/dev/null)"
> +            e2label "$root_dev" RootBackup
> +            e2label "$root_update_dev" Root
>          fi
>      fi
>      # run post-install hooks
> @@ -812,12 +812,12 @@ network_up () {
> 
>  # Cleans partition tables
>  wipe_partitions() {
> -    local drive=$1
> +    local drive="$1"
>      log "Wiping old boot sector"
> -    dd if=/dev/zero of=$drive bs=1024K count=1
> +    dd if=/dev/zero of="$drive" bs=1024K count=1
>      # zero out the GPT secondary header
>      log "Wiping secondary gpt header"
> -    local disk_kb_count=$(sfdisk -s $drive 2>/dev/null)
> +    local disk_kb_count=$(sfdisk -s "$drive" 2>/dev/null)
>      dd if=/dev/zero of=$drive bs=1024 seek=$(($disk_kb_count - 1)) count=1
>  }
> 
> @@ -840,35 +840,35 @@ test_ntp_configuration () {
> 
>  get_dm_device ()
>  {
> -    local device=$1
> +    local device="$1"
>      local return_var=$2
> -    major=$(stat -c '%t' $(readlink -f $device))
> -    minor=$(stat -c '%T' $(readlink -f $device))
> +    major=$(stat -c '%t' $(readlink -f "$device"))
> +    minor=$(stat -c '%T' $(readlink -f "$device"))
>      local dm_device>      local rc=1
>      for dm in /dev/mapper/*; do
> -        if [ $major = $(stat -c '%t' $dm) -a \
> -            $minor = $(stat -c '%T' $dm) ]; then
> -            local dm_device=$dm
> +        if [ $major = $(stat -c '%t' "$dm") -a \
> +            $minor = $(stat -c '%T' "$dm") ]; then
> +            local dm_device="$dm"
>              rc=0
>              break
>          fi
>      done
> 
> -    eval $return_var=$dm_device
> +    eval $return_var="$dm_device"
> 
>      return $rc
>  }
> 
>  #Function to determine partition and device names
>  get_part_info() {
> -    local drive_in=$1
> +    local drive_in="$1"
>      local dev_var=$2
>      local part_var=$3
>      local devname_1 devname2 part_number
>      local rc=0
> 
> -    eval $(readlink -f $drive_in|awk {'
> +    eval $(readlink -f "$drive_in" |awk {'
>          print "devname_1=" substr($1,1,length($1)-1);
>          print "devname_2=" substr($1,1,length($1)-2);
>          part_number=substr($1,length($1),1);
> -- 
> 1.6.6.1
> 
> _______________________________________________
> Ovirt-devel mailing list
> Ovirt-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/ovirt-devel
With those couple of nitpicks, ACK.

-- 
Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL:
<http://listman.redhat.com/archives/ovirt-devel/attachments/20100331/04ab4887/attachment.sig>