On Wed, 2010-03-31 at 14:09 -0400, Darryl L. Pierce wrote: > 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 <[email protected]> > > --- > > 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 > > [email protected] > > https://www.redhat.com/mailman/listinfo/ovirt-devel > > With those couple of nitpicks, ACK. > > _______________________________________________ > Ovirt-devel mailing list > [email protected] > https://www.redhat.com/mailman/listinfo/ovirt-devel
Pushing without the nits as per discussion on IRC. Single quotes cause the variables to be treated as literals. $ touch "test 1" $ d="test 1" $ readlink -f "$d" /tmp/tmp.YkXO7N2oXD/test 1 $ readlink -f '$d' /tmp/tmp.YkXO7N2oXD/$d $ devices="$devices $(readlink -f '$d')" $ devices="$devices $(readlink -f "$d")" $ echo $devices /tmp/tmp.YkXO7N2oXD/$d /tmp/tmp.YkXO7N2oXD/test 1 $ devices= $ devices="$devices $(readlink -f \"$d\")" readlink: extra operand `1"' Try `readlink --help' for more information. _______________________________________________ Ovirt-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/ovirt-devel
