[Ovirt-devel] [PATCH node] Moved network configuration work files to a temporary directory.

[Jim Meyering]

"Darryl L. Pierce" <dpierce at redhat.com> wrote:
> Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
> ---
>  scripts/ovirt-config-networking |   18 ++++++++----------
>  1 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/ovirt-config-networking b/scripts/ovirt-config-networking
> index 2f8363c..5c8cfa9 100755
> --- a/scripts/ovirt-config-networking
> +++ b/scripts/ovirt-config-networking
> @@ -3,15 +3,18 @@
>  # Iterates over the list of network devices on the node and prompts the user
>  # to configure each.
>
> +WORKDIR=$(mktemp -d)

How about this, so if mktemp fails, the script exits right away:

  WORKDIR=$(mktemp -d) || exit 1

>  CONFIG_FILE_ROOT="/file/etc/sysconfig/network-scripts/ifcfg"
> -CONFIG_LOG_FILE="/var/log/ovirt-network-setup.log"
> +#CONFIG_LOG_FILE="/var/log/ovirt-network-setup.log"
> +CONFIG_LOG_FILE="ovirt-network-setup.log"
>
>  function configure_interface
>  {
>      NIC=$1
>      BRIDGE=ovirtbr`echo $NIC | cut -b4-`
> -    IF_FILENAME="/var/tmp/augtool-$NIC"
> -    BR_FILENAME="/var/tmp/augtool-$BRIDGE"
> +    IF_FILENAME="${WORKDIR}/augtool-$NIC"
> +    BR_FILENAME="${WORKDIR}/augtool-$BRIDGE"

No need for curly braces in general:

       IF_FILENAME="$WORKDIR/augtool-$NIC"
       BR_FILENAME="$WORKDIR/augtool-$BRIDGE"

>      printf "\nConfigure $BRIDGE for use by $NIC..\n\n"
>
> @@ -70,10 +73,6 @@ function setup_menu
>      PS3="Please select a network interface to configure:"
>  }
>
> -# clean up any left over configurations
> -rm -f /var/tmp/config-augtool
> -rm -f /var/tmp/augtool-*
> -
>  setup_menu
>
>  select NIC in $NICS
> @@ -88,10 +87,9 @@ done
>
>  # Merge together all generated files and run augtool
>
> -cat /var/tmp/augtool-* > /var/tmp/config-augtool
> -printf "save\n" >> /var/tmp/config-augtool
>  {
> +cat ${WORKDIR}/augtool-* > ${WORKDIR}/config-augtool
> +printf "save\n" >> /var/tmp/config-augtool

You can drop the curly braces here, too.

Try not to use hard-coded names in /tmp or /var/tmp.
Use a file in $WORKDIR instead.
Otherwise, someone can pre-seed config-augtool with
bad things, or create a symlink by that name to make
you clobber whatever file they choose.

>  augtool < /var/tmp/config-augtool
>  service network restart
>  } > $CONFIG_LOG_FILE 2>> $CONFIG_LOG_FILE
> -