On Mon, Oct 8, 2012 at 8:19 AM,  <charles_r...@dell.com> wrote:
[...]
>> A startup script that can do these:
>>
>> 1. Set OS Name, Version, System Hostname in BMC
>> 2. Get BMC URL/IP address from BMC
>>
>> Would benefit:
>> 1. users who would like to fetch hostname/OS Name/Version.
>> 2. launch web-interface to BMC where supported with "xdg-open $BMC_URL"
>> 3. Use the BMC_IP for other purposes like
>>     3a. snmp proxy from host to BMC when BMC agent present.
>>     3b. expose BMC IP/URL via other systems management interfaces (like 
>> wsman).
>>
>> ----------------------------------------------------------------------
>>
> Hello,
>
> I request feedback on this proposal that will help users who would like 
> certain pieces of OS/BMC
> information set during system start-up.
>
> Here is a start-up script that implements this proposal:
>     
> http://sourceforge.net/tracker/?func=detail&aid=3571446&group_id=95200&atid=610552
>

Hi Charles,

at first, I wanted to say I see no problem with committing this
script. And I still don't in general. However ...

Is this script useful to somebody else than Dell customers running
RHEL-based distribution?

Some Bashisms in the script:
~~~
bash-4.2$ perl checkbashisms exchange-bmc-os-info.sh
possible bashism in exchange-bmc-os-info.sh line 206 ($"foo" should be
eval_gettext "foo"):
        echo -n $"${SCRIPT_NAME}: "
possible bashism in exchange-bmc-os-info.sh line 210 ($"foo" should be
eval_gettext "foo"):
                echo $"BMC info not available"
possible bashism in exchange-bmc-os-info.sh line 218 ($"foo" should be
eval_gettext "foo"):
        echo $"Usage: $0 {start|stop|restart|reload|status}" 1>&2
possible bashism in exchange-bmc-os-info.sh line 235 ($"foo" should be
eval_gettext "foo"):
        2) echo $"${SCRIPT_NAME}: failed to communicate with BMC" 1>&2 ;;
possible bashism in exchange-bmc-os-info.sh line 236 ($"foo" should be
eval_gettext "foo"):
        3) echo $"${SCRIPT_NAME}: failed to set OS information in BMC" 1>&2 ;;
possible bashism in exchange-bmc-os-info.sh line 237 ($"foo" should be
eval_gettext "foo"):
        4) echo $"${SCRIPT_NAME}: failed to get BMC information" 1>&2 ;;
~~~

I especially "urge" to replace % echo -n; with appropriate % printf ;
Since "${SCRIPT_NAME} ..." is first, I'd write it as % echo --
"${SCRIPT_NAME} ..." just to be sure.
What if $RETVAL is something unexpected? Currently only values of
$RETVAL from 2-4 are covered. Unexpected $RETVAL should produce
something like % printf "%s: unexpected error. Return code: %s."
"${SCRIPT_NAME}" "${RETVAL}"
Please, clean up trailing tabs and spaces.

~~~
set_bmc_info()
{       
        touch ${BMC_INFO} && chmod 600 ${BMC_INFO} && \
        {
                echo "BMC_IPv4=${BMC_IPv4}"
                echo "BMC_URL=${BMC_URL}"
        } > ${BMC_INFO} || \
        RETVAL=4
}
~~~

Rewritten as:
~~~set_bmc_info()
{
        touch "${BMC_INFO}" && chmod 600 "${BMC_INFO}" && \
                printf "BMC_IPv4=${BMC_IPv4}\nBMC_URL=${BMC_URL}" >
"${BMC_INFO}" || \
                RETVAL=4
}
~~~

Can something like this:
~~~
if [ -z "${BMC_IPv4}" -o "${BMC_IPv4}" = "0.0.0.0" ]; then
~~~

be rewritten as:
~~~
if [ -z "${BMC_IPv4}" ] || [ "${BMC_IPv4}" = "0.0.0.0" ]; then
~~~
?

Or just have /bin/bash at shebang ;)
I'd also put "..." around variables wherever possible, although that's
my paranoia talking.

I hope you'll receive this sort of criticism ... "well"(?).

Regards,
Z.

>
> --
> Charles Rose
> Linux Engineering
> Dell Inc.
> ------------------------------------------------------------------------------
> Don't let slow site performance ruin your business. Deploy New Relic APM
> Deploy New Relic app performance management and know exactly
> what is happening inside your Ruby, Python, PHP, Java, and .NET app
> Try New Relic at no cost today and get our sweet Data Nerd shirt too!
> http://p.sf.net/sfu/newrelic-dev2dev
> _______________________________________________
> Ipmitool-devel mailing list
> Ipmitool-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ipmitool-devel

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct
_______________________________________________
Ipmitool-devel mailing list
Ipmitool-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel

Reply via email to