[ovs-dev] [PATCH] ovs-lib: Return the correct exit status of the command 'status'

[Ben Pfaff]

On Tue, Oct 08, 2013 at 03:23:17PM -0700, Gurucharan Shetty wrote:
> On Tue, Oct 8, 2013 at 3:02 PM, Ben Pfaff <blp at nicira.com> wrote:
> > On Mon, Oct 07, 2013 at 02:32:10PM -0700, Gurucharan Shetty wrote:
> >> commit 46528f78e5c(debian, rhel, xenserver: Ability to collect ovs-ctl logs)
> >> made changes in the startup scripts such that the o/p of ovs-ctl is logged
> >> into ovs-ctl.log. But it had an unintended consequence that the exit status
> >> of ovs-ctl was no longer returned. We would always return success(the exit
> >> status of tee).
> >>
> >> With this commit, we return the exit status of ovs-ctl instead of tee.
> >>
> >> Signed-off-by: Gurucharan Shetty <gshetty at nicira.com>
> >
> > We log most of the ovs-ctl commands because they change state.
> > "status" doesn't change the state of anything.  Do we need to log it
> > at all?
> The only reason to log it is to debug bug statements that _may_ say that
> 'status' command returned "not running" and then it became fine. Logging it
> may provide a clue to whether there was a upgrade running at the same time
> or not.

Ah.  That could be useful.  Fine, let's keep the logging.

> I can get rid of logging for 'status', if you feel the above
> information is not worth it.
> 
> >
> > The new version doesn't appear to print the status on stdout anymore.
> I re-tested it and it does print it for me. Is your startup script unchanged?

I didn't test it, I just read it, and I forgot that "tee -a" would
print the same text on stdout as it did to the file.

> > One inline comment:
> >
> >> +        "status")
> >> +            # In case of the command 'status', we should return the exit status
> >> +            # of ovs-ctl. It is also useful to document the o/p in ovs-ctl.log.
> >> +            display=`"${datadir}/scripts/ovs-ctl" "$@" 2>&1`
> >> +            rc=$?
> >> +            echo "${display}" | tee -a "${logdir}/ovs-ctl.log"
> >
> > Is this different from
> >    echo "${display}" >> "${logdir}/ovs-ctl.log"
> The former should print both to stdout and the log. The latter only to the log.
> But based on your earlier comment, it sounds like your environment is
> different than mine.

No, I just made a mistake.

Acked-by: Ben Pfaff <blp at nicira.com>