[ovs-dev,ovn] ovn-ctl: introduce ovsdb-{n, s}b-wrapper options

ovn-ctl has the following options to run ovn-northd, ovn-controller or
ovn-ic under strace or valgrind wrappers.

  --ovn-northd-wrapper
  --ovn-controller-wrapper
  --ovn-ic-wrapper

Introduce --ovsdb-nb-wrapper and --ovsdb-sb-wrapper to do the same for
ovsdb processes for ovn-{nb,sb} dbs

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 utilities/ovn-ctl       | 52 +++++++++++++++++++++++++++++++++++++----
 utilities/ovn-ctl.8.xml |  2 ++
 2 files changed, 49 insertions(+), 5 deletions(-)

Bleep bloop.  Greetings Lorenzo Bianconi, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 84 characters long (recommended limit is 79)
#47 FILE: utilities/ovn-ctl:299:
                log_failure_msg "valgrind not installed, running $daemon without it"

WARNING: Line is 82 characters long (recommended limit is 79)
#59 FILE: utilities/ovn-ctl:311:
                log_failure_msg "strace not installed, running $daemon without it"

WARNING: Line is 82 characters long (recommended limit is 79)
#65 FILE: utilities/ovn-ctl:317:
            log_failure_msg "unknown wrapper $wrapper, running $daemon without it"

Lines checked: 148, Warnings: 3, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot

On 7/21/20 4:39 PM, Lorenzo Bianconi wrote:
> ovn-ctl has the following options to run ovn-northd, ovn-controller or
> ovn-ic under strace or valgrind wrappers.
> 
>   --ovn-northd-wrapper
>   --ovn-controller-wrapper
>   --ovn-ic-wrapper
> 
> Introduce --ovsdb-nb-wrapper and --ovsdb-sb-wrapper to do the same for
> ovsdb processes for ovn-{nb,sb} dbs
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---
>  utilities/ovn-ctl       | 52 +++++++++++++++++++++++++++++++++++++----
>  utilities/ovn-ctl.8.xml |  2 ++
>  2 files changed, 49 insertions(+), 5 deletions(-)
> 
> diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl
> index 8afe68a0a..5e935543e 100755
> --- a/utilities/ovn-ctl
> +++ b/utilities/ovn-ctl
> @@ -145,7 +145,7 @@ promote_ic_sb() {
>  }
>  
>  start_ovsdb__() {
> -    local DB=$1 db=$2 schema_name=$3 table_name=$4
> +    local DB=$1 db=$2 schema_name=$3 table_name=$4 wrapper=$5
>      local db_pid_file
>      local cluster_local_addr
>      local cluster_local_port
> @@ -288,6 +288,36 @@ $cluster_remote_port
>          set "$@" --sync-from=`cat $active_conf_file`
>      fi
>  
> +    # wrapper
> +    daemon=ovsdb-$db
> +    case $wrapper in
> +        valgrind)
> +            if (valgrind --version) > /dev/null 2>&1; then
> +                set valgrind -q --leak-check=full --time-stamp=yes \
> +                    --log-file="$ovn_logdir/$daemon.valgrind.log.%p" "$@"
> +            else
> +                log_failure_msg "valgrind not installed, running $daemon without it"
> +            fi
> +            ;;
> +        strace)
> +            if (strace -V) > /dev/null 2>&1; then
> +                strace="strace -tt -T -s 256 -ff"
> +                if (strace -DV) > /dev/null 2>&1; then
> +                    # Has the -D option.
> +                    set $strace -D -o "$ovn_logdir/$daemon.strace.log" "$@"
> +                    strace=""
> +                fi
> +            else
> +                log_failure_msg "strace not installed, running $daemon without it"
> +            fi
> +            ;;
> +        '')
> +            ;;
> +        *)
> +            log_failure_msg "unknown wrapper $wrapper, running $daemon without it"
> +            ;;
> +    esac
> +

Hi Lorenzo,

This part duplicates the code from start_ovn_daemon() in ovn-lib.in.
Would it be possible to refactor the scripts so we can reuse the same
function?

Thanks,
Dumitru

>      "$@" "$file"
>  
>      # Initialize the database if it's NOT joining a cluster.
> @@ -298,10 +328,16 @@ $cluster_remote_port
>      if test $mode = cluster; then
>          upgrade_cluster "$schema" "unix:$sock"
>      fi
> +
> +    if test X"$strace" != X; then
> +        # Strace doesn't have the -D option so we attach after the fact.
> +        setsid $strace -o "$ovn_logdir/$daemon.strace.log" \
> +            -p `cat $ovn_rundir/$daemon.pid` > /dev/null 2>&1 &
> +    fi
>  }
>  
>  start_nb_ovsdb() {
> -    start_ovsdb__ NB nb OVN_Northbound NB_Global
> +    start_ovsdb__ NB nb OVN_Northbound NB_Global "$OVSDB_NB_WRAPPER"
>  }
>  
>  start_sb_ovsdb() {
> @@ -313,7 +349,7 @@ start_sb_ovsdb() {
>          ulimit -n $MAXFD
>      fi
>  
> -    start_ovsdb__ SB sb OVN_Southbound SB_Global
> +    start_ovsdb__ SB sb OVN_Southbound SB_Global "$OVSDB_SB_WRAPPER"
>  }
>  
>  start_ovsdb () {
> @@ -322,11 +358,13 @@ start_ovsdb () {
>  }
>  
>  start_ic_nb_ovsdb() {
> -    start_ovsdb__ IC_NB ic_nb OVN_IC_Northbound IC_NB_Global
> +    start_ovsdb__ IC_NB ic_nb OVN_IC_Northbound IC_NB_Global \
> +                  "$OVSDB_NB_WRAPPER"
>  }
>  
>  start_ic_sb_ovsdb() {
> -    start_ovsdb__ IC_SB ic_sb OVN_IC_Southbound IC_SB_Global
> +    start_ovsdb__ IC_SB ic_sb OVN_IC_Southbound IC_SB_Global \
> +                  "$OVSDB_SB_WRAPPER"
>  }
>  
>  start_ic_ovsdb () {
> @@ -692,6 +730,8 @@ set_defaults () {
>      OVN_IC_WRAPPER=
>      OVN_CONTROLLER_PRIORITY=-10
>      OVN_CONTROLLER_WRAPPER=
> +    OVSDB_NB_WRAPPER=
> +    OVSDB_SB_WRAPPER=
>  
>      OVN_USER=
>  
> @@ -908,6 +948,8 @@ Options:
>    --ovn-ic-sb-db-ssl-ca-cert=CERT OVN IC Southbound DB SSL CA certificate file
>    --ovn-user="user[:group]"      pass the --user flag to the ovn daemons
>    --ovs-user="user[:group]"      pass the --user flag to ovs daemons
> +  --ovsdb-nb-wrapper=WRAPPER     run with a wrapper like valgrind for debugging
> +  --ovsdb-sb-wrapper=WRAPPER     run with a wrapper like valgrind for debugging
>    -h, --help                     display this help message
>  
>  File location options:
> diff --git a/utilities/ovn-ctl.8.xml b/utilities/ovn-ctl.8.xml
> index f5b7f7aeb..a1d39b22b 100644
> --- a/utilities/ovn-ctl.8.xml
> +++ b/utilities/ovn-ctl.8.xml
> @@ -64,6 +64,8 @@
>      <p><code>--ovn-controller-wrapper=<var>WRAPPER</var></code></p>
>      <p><code>--ovn-ic-priority=<var>NICE</var></code></p>
>      <p><code>--ovn-ic-wrapper=<var>WRAPPER</var></code></p>
> +    <p><code>--ovsdb-nb-wrapper=<var>WRAPPER</var></code></p>
> +    <p><code>--ovsdb-sb-wrapper=<var>WRAPPER</var></code></p>
>      <p><code>--ovn-user=<var>USER:GROUP</var></code></p>
>      <p><code>--ovs-user=<var>USER:GROUP</var></code></p>
>      <p><code>-h</code> | <code>--help</code></p>
>