Ben Pfaff <b...@ovn.org> wrote on 08/10/2016 11:22:49 AM:
> From: Ben Pfaff <b...@ovn.org>
> To: Conner Herriges/US/IBM@IBM
> Cc: dev@openvswitch.org
> Date: 08/10/2016 11:23 AM
> Subject: Re: [ovs-dev] [PATCH v2] Add monitoring to ovsdb
>
> On Wed, Aug 10, 2016 at 11:10:25AM -0500, Conner Herriges wrote:
> > Monitoring is a valid option on POSIX based platforms. The monitor
> > creates an additional process to monitor the ovsdb-server daemon. If
> > the daemon dies due to a programming error, then the monitor process
> > starts a new copy of it. If the daemon dies or exits for another
> > reason, the monitor process exits.
> >
> > The monitor option is not currently enabled for either OVN Northboud or
> > Southbound databases. It would be a trivial task to enable the monitor
> > in the start_ovsdb function in ovn-ctl. However,
> > this is an attempt to do so using the start_daemon function in ovs-lib,
> > which is currently used by ovn-northd and ovn-controller.
> >
> > Signed-off-by: Conner Herriges <conner.herri...@ibm.com>
>
> Hi, this patch is space-damaged, notice "$DB_N B_ADDR" and similar.
>
> Also, please don't add a change log as comments in code.
>
> Why does this disable features for ovsdb-server in the start_daemon
> function?
>
> Thanks,
>
> Ben.
Hi Ben,
I'm not exactly sure what you mean by adding a change log as comments
in code. Can you clarify?
The only feature that ovsdb-server requires in the start_daemon function
is the --monitor. The rest of the features are enabled in ovn-ctl and are
dependent on defaults and variables set there.
Thanks,
Conner
>
> > ovn/utilities/ovn-ctl | 19 ++++++++++++-------
> > utilities/ovs-lib.in | 30 +++++++++++++++++-------------
> > 2 files changed, 29 insertions(+), 20 deletions(-)
> >
> > diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
> > index a4a9817..d7d14e5 100755
> > --- a/ovn/utilities/ovn-ctl
> > +++ b/ovn/utilities/ovn-ctl
> > @@ -51,10 +51,9 @@ start_ovsdb () {
> > upgrade_db "$DB_NB_FILE" "$DB_NB_SCHEMA" 1>/dev/null
2>/dev/null
> >
> > set ovsdb-server
> > -
> > - set "$@" --detach $OVN_NB_LOG --log-file=$OVN_NB_LOGFILE
> --remote=punix:$DB_NB_SOCK --remote=ptcp:$DB_NB_PORT:$DB_NB_ADDR --
> pidfile=$DB_NB_PID --unixctl=ovnnb_db.ctl
> > -
> > - $@ $DB_NB_FILE
> > +#added start_daemon here
> > + set "$@" --detach $OVN_NB_LOG --log-file=$OVN_NB_LOGFILE
> --remote=punix:$DB_NB_SOCK --remote=ptcp:$DB_NB_PORT:$DB_N B_ADDR
> --pidfile=$DB_NB_PID --unixctl=ovnnb_db.ctl $DB_NB_FILE
> > + OVS_RUNDIR=${OVN_RUNDIR} start_daemon "$OVN_NB_PRIORITY"
> "$OVN_NB_WRAPPER" "$@"
> > fi
> >
> > # Check and eventually start ovsdb-server for Southbound DB
> > @@ -62,9 +61,9 @@ start_ovsdb () {
> > upgrade_db "$DB_SB_FILE" "$DB_SB_SCHEMA" 1>/dev/null
2>/dev/null
> >
> > set ovsdb-server
> > -
> > - set "$@" --detach $OVN_SB_LOG --log-file=$OVN_SB_LOGFILE
> --remote=punix:$DB_SB_SOCK --remote=ptcp:$DB_SB_PORT:$DB_SB_ADDR --
> pidfile=$DB_SB_PID --unixctl=ovnsb_db.ctl
> > - $@ $DB_SB_FILE
> > +#added start_daemon here
> > + set "$@" --detach $OVN_SB_LOG --log-file=$OVN_SB_LOGFILE
> --remote=punix:$DB_SB_SOCK --remote=ptcp:$DB_SB_PORT:$DB_S B_ADDR
> --pidfile=$DB_SB_PID --unixctl=ovnsb_db.ctl $DB_SB_FILE
> > + OVS_RUNDIR=${OVN_RUNDIR} start_daemon "$OVN_SB_PRIORITY"
> "$OVN_SB_WRAPPER" "$@"
> > fi
> > }
> >
> > @@ -189,6 +188,12 @@ set_defaults () {
> > DB_SOCK=$rundir/db.sock
> > DB_CONF_FILE=$dbdir/conf.db
> >
> > +#added ovsdb priority and wrapper here for consistency with
> > +#northd and controller calls to start_daemon
> > + OVN_NB_PRIORITY=-10
> > + OVN_NB_WRAPPER=
> > + OVN_SB_PRIORITY=-10
> > + OVN_SB_WRAPPER=
> > OVN_NORTHD_PRIORITY=-10
> > OVN_NORTHD_WRAPPER=
> > OVN_CONTROLLER_PRIORITY=-10
> > diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
> > index cbad85a..776170d 100644
> > --- a/utilities/ovs-lib.in
> > +++ b/utilities/ovs-lib.in
> > @@ -155,19 +155,23 @@ start_daemon () {
> > daemon=$1
> > strace=""
> >
> > - # drop core files in a sensible place
> > - test -d "$DAEMON_CWD" || install -d -m 755 -o root -g root
> "$DAEMON_CWD"
> > - set "$@" --no-chdir
> > - cd "$DAEMON_CWD"
> > -
> > - # log file
> > - test -d "$logdir" || install -d -m 755 -o root -g root "$logdir"
> > - set "$@" --log-file="$logdir/$daemon.log"
> > -
> > - # pidfile and monitoring
> > - test -d "$rundir" || install -d -m 755 -o root -g root "$rundir"
> > - set "$@" --pidfile="$rundir/$daemon.pid"
> > - set "$@" --detach
> > + #specific for ovn-northd and ovn-controller
> > + if [ "$daemon" != "ovsdb-server" ]
> > + then
> > + # drop core files in a sensible place
> > + test -d "$DAEMON_CWD" || install -d -m 755 -o root -g
> root "$DAEMON_CWD"
> > + set "$@" --no-chdir
> > + cd "$DAEMON_CWD"
> > +
> > + # log file
> > + test -d "$logdir" || install -d -m 755 -o root -g root
"$logdir"
> > + set "$@" --log-file="$logdir/$daemon.log"
> > +
> > + # pidfile and monitoring
> > + test -d "$rundir" || install -d -m 755 -o root -g root
"$rundir"
> > + set "$@" --pidfile="$rundir/$daemon.pid"
> > + set "$@" --detach
> > + fi
> > test X"$MONITOR" = Xno || set "$@" --monitor
> >
> > # wrapper
> > --
> > 2.7.4 (Apple Git-66)
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
