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

[Perry Myers]

Jim Meyering wrote:
> "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.

Agreed.  Unfortunately our choices on the Node are limited due to efforts 
to keep image size small.  The options are C and bash presently.

Perhaps we should investigate what the cost would be to put *minimal* 
python environment on the Node.  Just core libraries and interpreter. 
Doing this we could eventually replace this bash stuff with python which 
is a little more maintainable :)

Perry