[Ovirt-devel] [PATCH ovirt-node] Adds automatic non-interactive configuration for the standalone node.

[Jim Meyering]

Alan Pevec <apevec at redhat.com> wrote:
> From: Darryl L. Pierce <dpierce at redhat.com>
> NOTE: rebased to apply after:
>     Moved network configuration work files to a temporary directory.
>     Adds a "Save" option to save the configuration.
>
> If ovirt-config-network is called as follows:
>
> ovirt-config-network AUTO  [NIC] [IP NETMASK GATEWAY BROADCAST]
>
> where NIC is the management interface to be configured.
>
> If the IP details are provided then they are used to configure the device.
> Otherwise DHCP will be used.
>
> The kernel arguments are:
>  * OVIRT_MGMT_IFACE:   the management network interface name
>  * OVIRT_IP_ADDRESS:   the IP address
>  * OVIRT_IP_NETMASK:   the netmask
>  * OVIRT_IP_GATEWAY:   the network gateway
>  * OVIRT_IP_BROADCAST: the network broadcast address
>
> Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
> ---
>  scripts/ovirt-config-networking |   84 +++++++++++++++++++++++++--------------
>  scripts/ovirt-firstboot         |   12 +++++-
>  2 files changed, 65 insertions(+), 31 deletions(-)
>
> diff --git a/scripts/ovirt-config-networking b/scripts/ovirt-config-networking
...
> -{
> -config="$WORKDIR"/config-augtool
> -{ cat "$WORKDIR"/augtool-* && printf "save\n"; } > $config \
> -&& augtool < "$WORKDIR"/config-augtool
> +    select NIC in $NICS
> +    do
> +        printf "\n"
> +        case "$NIC" in
> +            "Save") RESTART="Y"; break ;;
> +            "Quit") break ;;
> +            *) configure_interface $NIC $IFACE_NUMBER ;;
> +        esac
> +        setup_menu
> +    done
>
> -service network restart
> -} > $CONFIG_LOG_FILE 2>> $CONFIG_LOG_FILE
> +fi
> +
> +if [ "$RESTART" == "Y" ]; then
> +    # Merge together all generated files and run augtool
> +    {
> +    config="$WORKDIR"/config-augtool
> +    { cat "$WORKDIR"/augtool-* && printf "save\n"; } > $config \
> +    && augtool < "$WORKDIR"/config-augtool
> +    service network restart
> +    } > $CONFIG_LOG_FILE 2>> $CONFIG_LOG_FILE

The above looks fishy, with ">"; I suspect it truncates rather
than appending.  So maybe you want this instead:

    } >> $CONFIG_LOG_FILE 2>&1

Also, how about performing the network restart only if augtool etc.
succeeded? [this also factors out another $config]

    {
      config="$WORKDIR"/config-augtool
      { cat "$WORKDIR"/augtool-* && printf "save\n"; } > $config \
        && augtool < $config \
        && service network restart
    } >> $CONFIG_LOG_FILE 2>&1

With iteration upon iteration like this, you can see
how using a shared development/unstable/pu branch (with very
permissive "push" policy) might help.