Agree w. Z; you could check for success/fail on that line. A few minor
additional comments (I didn't run it, just looked at it, since I didn't have
the patch installed to try):
1) if the /run directory doesn't exist this will not work, as you don't check
for its existence, only the file /run/bmc-info (is that a agentfreemgmt thing?
I searched for it and only found Run DMC references ;) Generally new top level
dirs suck; I'd expect it in /var/xyz or some other location.)
2) restart should do "stop", then "start", not just "start"
3) reload shouldn't just do a start. I'm not even sure it's appropriate to
include or what it should do (e.g. you're not sending a signal to a daemon,
you're querying the BMC.)
4) probably shouldn't assume channel 1 is the right LAN channel; I looked
around… in this note:
http://ingvar.blog.redpill-linpro.com/2011/12/09/setting-the-ip-address-on-hp-ilo-from-linux/
The author searches for the right LAN channel with a little one liner:
for i in `seq 1 14`; do ipmitool lan print $i 2>/dev/null | grep -q ^Set &&
echo Channel $i; done
And then does the LAN print (in their example it was 2). Not sure what the
most appropriate way to find it, but….
5) Instead of:
eval_gettext "Usage: $0 {start|stop|restart|reload|status}"; echo 1>&2
You probably want/meant:
eval_gettext "Usage: $SCRIPT_NAME {start|stop|restart|status}"
6) In case of version weirdness you might put in a version check that just
bails from the start (same if the binary doesn't exist; why do anything if it's
not there or if it's the wrong version? You say "failed to communicate with
BMC" when the binary doesn't exist… yes, that's true, but not very informative
;)
7) In non-dell systems you assume that the URL is "https://($BMC_IPv4):443".
I don't think you should assume a web server is running, let alone that it's on
443 or using HTTPS…?
8) you sometimes use "echo" and other times "echo 1>&2". Not that it really
matters… but pick one (not sure why echo needs the 1>&2, but you never know :))
9) (security paranoia on this one, but I can't pass it up….) You're taking
unfiltered values from the BMC and using them in a shell script on the host OS.
This means that an attacker can set values in BMC land that would affect the
server side (backticks and the like.) Ensuring that the values are kosher
(e.g. all chars or #'s or whatever you would expect) would be a good thing.
-- d
^..^
On Nov 1, 2012, at 1:37 AM, Zdenek Styblik <zdenek.styb...@gmail.com> wrote:
> On Mon, Oct 29, 2012 at 9:02 AM, <charles_r...@dell.com> wrote:
> [...]
>>
>> Uploaded both script and config files to Feature Tracker.
>>
>> http://sourceforge.net/tracker/?func=detail&aid=3571445&group_id=95200&atid=610553
>>
>
> The only concern I have what happens if somebody doesn't have gettext
> installed, resp. gettext.sh is not in path.
> Other than that, I'd say it's ok.
>
> Regards,
> Z.
>
> ------------------------------------------------------------------------------
> 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
------------------------------------------------------------------------------
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
