[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Ovirt-devel] [PATCH-node 2/2] A rather large re-write of the ovirt-node.spec file. The reason



Chris Lalancette <clalance redhat com> wrote:
> diff --git a/ovirt-listen-awake/ovirt-convert-host b/ovirt-listen-awake/ovirt-convert-host
> new file mode 100644
> index 0000000..584291a
> --- /dev/null
> +++ b/ovirt-listen-awake/ovirt-convert-host
> @@ -0,0 +1,58 @@
> +#!/bin/bash
> +
> +. /etc/init.d/ovirt-functions
> +
> +PATH=$PATH:/sbin:/usr/sbin
> +
> +if [ $# -ne 0 ]; then
> +    echo "Usage: ovirt-convert-host"
> +    exit 1
> +fi
> +
> +backup_file() {
> +    dir=$(dirname "$1")
> +    case $dir in /*);; *) die "unexpected non-absolute dir: $dir";; esac
> +    mkdir -p "$OVIRT_BACKUP_DIR/${dir:1}"
> +    cp -pf "$1" "$OVIRT_BACKUP_DIR/${dir:1}"

Now that you ensure $dir starts with "/", you can replace each
of the /${dir:1} with $dir.
Not only is that more readable (I don't have to look up ${VAR:1} syntax
in the bash manual ;-), but you avoid the bash-ism.

> +}
> +
> +add_if_not_exist() {
> +    string="$1"
> +    file="$2"
> +
> +    grep -qE "^[[:space:]]*$string($|#|[[:space:]])" "$file" \
> +	|| echo "$string" >> "$file"
> +}

Good idea to make this into a function!
However, regexp-special characters (like ".", "*", etc.) in
$string must be escaped for use as the grep regexp.
In the existing uses, only "." needs to be escaped, but
might as well escape them all, in case someone copies the code
to use elsewhere.  I think the sed command below
will do it (please double check!).

Also, using printf is slightly safer that echo, in case someone
ever applies this function to strings containing a \c sequence
that's interpreted by echo:

Note the new function name and no double quotes needed
on assignment with a simple RHS:

add_if_missing() {
    string=$1
    file=$2

    # backslash-escape regexp-special characters in the string
    string_re=$(printf %s "$string"|sed 's,\([][|?{}\.+()*]\),\\\1,g')

    grep -qE "^[[:space:]]*$string_re($|#|[[:space:]])" "$file" \
      || printf '%s\n' "$string" >> "$file"
}

> +echo "This script will make a number of changes to your system to enable it to"
> +echo "work as an oVirt node.  You can later undo these changes by"
> +echo "running /usr/sbin/ovirt-unconvert-host.  Do you want to proceed? [y/N]?"
> +read yesno
...
> diff --git a/scripts/ovirt-functions b/scripts/ovirt-functions
> index 083e13d..6ea034a 100644
> --- a/scripts/ovirt-functions
> +++ b/scripts/ovirt-functions
> @@ -5,6 +5,8 @@ OVIRT_LOGFILE=/var/log/ovirt.log
>  # label of the oVirt partition
>  OVIRT_LABEL=OVIRT
>
> +OVIRT_BACKUP_DIR=/var/lib/ovirt-backup
> +
>  find_srv()
>  {
>      local dnsreply
> @@ -21,3 +23,27 @@ die()
>  {
>      echo "$@" 1>&2; failure; echo 1>&2; exit 1
>  }
> +
> +ovirt_setup_libvirtd() {
> +    # just to get a boot warning to shut up
> +    touch /etc/resolv.conf
> +
> +    # make libvirtd listen on the external interfaces
> +    sed -i -e 's/^#\(LIBVIRTD_ARGS="--listen"\).*/\1/' \
> +	/etc/sysconfig/libvirtd
> +
> +    # set up qemu daemon to allow outside VNC connections
> +    sed -i -e 's/^[[:space:]]*#[[:space:]]*\(vnc_listen = "0.0.0.0"\).*/\1/' \
> +	/etc/libvirt/qemu.conf
> +    # set up libvirtd to listen on TCP (for kerberos)
> +    sed -i -e "s/^[[:space:]]*#[[:space:]]*\(listen_tcp\)\>.*/\1 = 1/" \
> +	-e "s/^[[:space:]]*#[[:space:]]*\(listen_tls\)\>.*/\1 = 0/" \
> +	/etc/libvirt/libvirtd.conf
> +
> +    # with libvirt (0.4.0), make sure we we setup gssapi in the mech_list
> +    sasl_conf=/etc/sasl2/libvirt.conf
> +    if ! grep -qE "^mech_list: gssapi" $sasl_conf ; then
> +	sed -i -e "s/^\([[:space:]]*mech_list.*\)/#\1/" $sasl_conf
> +	echo "mech_list: gssapi" >> $sasl_conf
> +    fi

Everything else looks ok, though using double quotes above in
echo and sed is unnecessary and a little bit fragile -- and not
consistent with use of single quotes in the two prior sed commands...


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]