[ovs-dev] [PATCH] ovs-lib: Return the correct exit status of the command 'status'
Gurucharan Shetty
shettyg at nicira.com
Tue Oct 8 22:23:17 UTC 2013
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.
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?
>
> 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.
>
>> + return ${rc}
>> + ;;
>
> Thanks,
>
> Ben.
More information about the dev
mailing list