[Ovirt-devel] [PATCH node] Reworked logging for standalone mode

[Jim Meyering]

Perry Myers <pmyers at redhat.com> wrote:
> Entire firstboot is logged to both console/logfile using tee.  Selected
> sections of the code are set to only log stdout/stderr to logfile to prevent
> data overload on console.  In these cases log() function is used to write
> to the console.
...

ACK, but for some missing quotes and name tweaks

> diff --git a/scripts/ovirt-functions b/scripts/ovirt-functions
> index 814a8ae..13f9ad4 100644
> --- a/scripts/ovirt-functions
> +++ b/scripts/ovirt-functions
> @@ -1,6 +1,8 @@
>  # -*-Shell-script-*-
>
>  OVIRT_LOGFILE=/var/log/ovirt.log
> +TEE="tee -a $OVIRT_LOGFILE"
> +LOG_STATUS=0
>
>  # label of the oVirt partition
>  OVIRT_LABEL=OVIRT
> @@ -28,6 +30,37 @@ OVIRT_CONFIG_FILES="\
>   /etc/collectd.conf
>  "
>
> +# Save stdout to fd 6 and stderr to fd 7.  Redirect normal stdout/stderr
> +# to log file
> +start_log() {
> +    if [ $LOG_STATUS = 0 ]; then

It's better to quote all uses of LOG_STATUS,
in case it's ever defined to something else, or empty.

> +        exec 6>&1
> +        exec 7>&2
> +        exec 1>>$OVIRT_LOGFILE
> +        exec 2>&1
> +        LOG_STATUS=1
> +    fi
> +}
> +
> +# Restore stdout/stderr from fd 6 and 7, respectively.  Close fd 6 and 7
> +stop_log() {
> +    if [ $LOG_STATUS = 1 ]; then

It's slightly better to always use the same test.
I.e., test against only one of 0 or 1, not both, and negate if needed.
Otherwise, you have to wonder what happens if $LOG_STATUS is ever
set to something like 2.

E.g., you could test $LOG_STATUS != 1 in start_log,
since there are two tests of $LOG_STATUS = 1 already.
Or maybe you prefer "boolean" semantics and would
want to test against 0.

> +        exec 1>&6 6>&-
> +        exec 2>&7 7>&-
> +        LOG_STATUS=0
> +    fi
> +}
> +
> +log() {
> +    printf "$(date +'%b %d %H:%M:%S') "
> +
> +    if [ $LOG_STATUS = 1 ]; then
> +        echo $@ >&6
> +    else
> +        echo $@

Whoops.  this needs double quotes around each "$@".
Otherwise, logs lose TABs and sequences of spaces.  E.g.,

  $ bash -c 'foo() { echo $@; }; foo "a        b"'
  a b

> +    fi
> +}

All names defined/used above impinge on the name space
of each script that sources ovirt-functions, so it'd be
nice to use names that make collisions less likely e.g.,
_TEE, _log and $_LOG_STATUS.