A quick comment, there appears to be new OEM options under the 'lan'
command of ipmitool.
+ lprintf(LOG_NOTICE, " tftp ipaddr <x.x.x.x> Set tftp
server IP address");
+ lprintf(LOG_NOTICE, " ntp ipaddr <x.x.x.x> Set ntp
server IP address");
+ lprintf(LOG_NOTICE, " tftp port <num> Set tftp
server UDP port num ");
+ lprintf(LOG_NOTICE, " ntp port <num> Set ntp
server UDP port num ");
<http://sources.calxeda.com/gitweb/?p=ipmitool.git;a=commit;h=671b435>
IMO, they should all be moved under the cxoem command. It appears that
these settings are done via Get/Set Lan Parameters, which is why it's in
this section. However, they are OEM specific parameters. Because it's
under the 'lan' section, your average user might believe that it is
available on all IPMI machines, rather than being an OEM specific option.
In addition, what if another vendor comes along and has an alternate OEM
tftp/ntp IP/port setting. Where will it go?
Al
On Tue, Jul 23, 2013 at 1:25 PM, Jeffrey Bastian <jbast...@redhat.com>wrote:
> On Tue, Jul 23, 2013 at 07:48:29PM +0200, Zdenek Styblik wrote:
> > On Mon, Jul 22, 2013 at 10:33 PM, Jeffrey Bastian <jbast...@redhat.com>
> wrote:
> > > I have gone through Calxeda's commits and extracted only those that
> > > touch the ipmi_cxoem.{c,h} files.
> >
> > Funny, because that's not what you have attached.
>
> I should clarify: the commits that created ipmi_cxoem.{c,h} also
> modified other files like ipmi_lanp.{c,h}. For example,
> http://sources.calxeda.com/gitweb/?p=ipmitool.git;a=commit;h=671b435
>
> I just excluded the commits that focused solely on other code like fru,
> ekanalyzer, sel, etc.
>
>
> > I wanted to say adding 'cxoem' shouldn't be a big deal, but I see
> > couple issues already.
>
> Thanks for the review. I'll pass the comments along back to the
> developers at Calxeda.
>
>
> > > I've also modified get_lan_param_select() from lib/ipmi_lanp.c as I
> > > described in my email last Friday.
> >
> > Your patch is incorrect. I see nothing wrong with code in question.
>
> I'll add more details to the other email thread since that bug is
> independent
> of the cxoem patches.
>
>
> > You really weren't serious about this one, were you???!!!
>
> No, I'm an idiot. I forgot the -d flag when I ran 'cvs update' so I
> was missing the src/plugins/serial directory. Sorry about that...
> PEBKAC... (I use mostly git now so my cvs is very rusty.)
>
> Thanks again!
> Jeff
>
>
> ------------------------------------------------------------------------------
> See everything from the browser to the database with AppDynamics
> Get end-to-end visibility with application monitoring from AppDynamics
> Isolate bottlenecks and diagnose root cause in seconds.
> Start your free trial of AppDynamics Pro today!
> http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
> _______________________________________________
> Ipmitool-devel mailing list
> Ipmitool-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
>
------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
Ipmitool-devel mailing list
Ipmitool-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
