[Ovirt-devel] [PATCH node] Handle space in storage wwid
Mike Burns
mburns at redhat.com
Wed Mar 31 18:36:39 UTC 2010
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 <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.
>
> _______________________________________________
> Ovirt-devel mailing list
> Ovirt-devel at redhat.com
> 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.
More information about the ovirt-devel
mailing list