[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 |   22 ++++++++++------------
>  1 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/scripts/ovirt-config-networking b/scripts/ovirt-config-networking
> index 2f8363c..592bef4 100755
> --- a/scripts/ovirt-config-networking
> +++ b/scripts/ovirt-config-networking
> @@ -1,17 +1,20 @@
> -#!/bin/bash
> +$#!/bin/bash

Hi Darryl,

Typo?

>  #
>  # Iterates over the list of network devices on the node and prompts the user
>  # to configure each.
>
> +WORKDIR=$(mktemp -d) || exit 1

Nothing removes $WORKDIR.
You could just remove it at the end.
Doing it right isn't trivial, but once you've seen the idiom,
it's easy to reuse (this is from coreutils/tests/test-lib.sh).
Just insert this:

# Remove $WORKDIR upon interrupt (and HUP, PIPE, TERM) and upon normal
# termination, being careful not to change the exit status.
trap '__st=$?; rm -rf "$WORKDIR"; exit $__st' 0
trap 'exit $?' 1 2 13 15

>  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"
>
>      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
>  {
> -augtool < /var/tmp/config-augtool
> +cat $WORKDIR/augtool-* > $WORKDIR/config-augtool
> +printf "save\n" >> /var/tmp/config-augtool

Oops.  one remaining /var/tmp/...

> +augtool < $WORKDIR/config-augtool

I've just noticed that these uses of $WORKDIR need to be
double-quoted-for-the-paranoid.
(in case $TMPDIR contains bogus characters).
Also, ideally, you'd run augtool only if all preceding writes
completed without error, so,

cf="$WORKDIR/config-augtool"
{ cat "$WORKDIR"/augtool-* && printf "save\n"; } > "$cf" \
  && augtool < "$cf"

>  service network restart
>  } > $CONFIG_LOG_FILE 2>> $CONFIG_LOG_FILE

I really, really prefer not to program in shell.