Jim,

 

I tested your patch on Intel S5520UR and S2600CO systems and it worked
ok.   It didn't mess anything up that previously worked.  

 

Andy

 

From: Jim Mankovich [mailto:jm...@hp.com] 
Sent: Tuesday, April 10, 2012 1:45 PM
To: Hank Bruning
Cc: ipmitool-devel@lists.sourceforge.net
Subject: Re: [Ipmitool-devel] Correct Threshold/Discrete/Analog Display

 

Hank,

Sound like a plan, but if you can get the review done sooner that would
be appreciated.

Are you going to be able to do any testing in this time frame or just a
code review?

-- Jim Mankovich | jm...@hp.com --


On 4/9/2012 6:36 PM, Hank Bruning wrote: 

Please wait for three weeks while this is reviewed.
Hank

On Mon, Apr 9, 2012 at 3:51 PM, Jim Mankovich <jm...@hp.com> wrote:

All,

Anyone care to code review this before I submit this to the CVS
repository?
I'm planning on submitting this on Wed April 11 unless I hear from
someone before then.

Prior Message,

Attached is a patch for the TOB CVS repository to resolve how ipmitool
incorrectly displays all Threshold
Sensors as having analog readings instead of properly determining
whether or not the Threshold Sensor
is analog or discrete.  This is a fairly large change and I would
appreciate if I could get someone to
do a code review for me.   This code has already been reviewed by one
other person here that is helping
out with impitool bug fixes.


In order to perform testing, I #ifdef'ed out the call to
ipmi_sdr_get_sensor_reading
in ipmi_sdr_get_sensor_reading_ipmb and I plan on pushing this change to
cvs as well unless someone
has a better idea.  This change makes the CVS code the same as 1.8.11
again.   I also ran across a
defect ipmi_lanplus_send_payload() where timeout was getting reset to
the default value on each invocation.
I found a patch already existed for this, but the patch was flawed.   I
resolved this problem by loading
the session timeout (after intf->open !) into a local variable instead
of overwriting the session timeout in
this function.

This patch has been fairly well tested on a variety of HP Proliant
Platforms and one Dell Platform.
I would appreciate if others could try out this patch on other platforms
and let me know if they see
any problems. I used code coverage analysis in my testing to verify that
the code changes I made
were executed and I also verified that the ipmitool output for "sdr
list", "sdr elist" and "sensor list"
with -v, -c, no arguments, and -c with -v were correct when this patch
is applied. The testing I did was
mainly via the lanplus and lan interfaces.

The following outlines the visible changes which will occur after
application of this patch and
some information pertaining to why the changes were necessary.

Only Full Sensor Records of type Threshold can return Analog readings

   See Section 42.1 Event/Reading Type Codes
   Threshold == (SDR field - Event/Reading Type Code == 0x01)
   Changes to properly decode Threshold/Analog and Threshold/Discrete


Full and Compact Sensor Records can both be of type Threshold
Threshold can return Discrete or Analog readings

   See Sections 43.1 Full Sensor Records/43.2 Compact Sensor Records
   Discrete is identified by (SDR field - Sensor Units 1 [7:6] == 11b)
   Changes to properly decode Threshold/Discrete compact sensors


Get Sensor Reading Command - Sensor reading should be ignored if the
sensor does not return
an numeric (analog) reading

   See Section 35.14 Get Sensor Reading Command
   "sdr list" prints "Sensor Reading" for Full Sensors (Discrete and
Analog)
   Changes to display Sensor Reading for Compact


verbose output OEM field is only printed for Discrete Sensors (Full and
Compact)

   Changed to be printed for Threshold


verbose output Event Message Control is only printed for Full Sensors
(Discrete and Analog)

   Changed to be printed for Compact


For Compact Sensors verbose output occurs instead of csv formatted when
both -v and -c.
For Full sensors, csv output is more verbose when you use -v with -c.

   Changed to make Compact Sensor output exactly the same as Full Sensor
output when using -c with -v.


Verbose output display for "Sensor Type" used the output string
"Analog".

   Changed to correctly display "Sensor Type" as "Threshold" or
"Discrete".
   "Analog" is no longer output as a "Sensor Type".


Overview of the Code Changes

The four separate sdr and sensor full and compact display routines have
been merged into
two separate routines. There is now a single routine for displaying a
full and compact sdrs and
a single routine for displaying full and compact sensors. These two
routines were modified to
properly decode analog and discrete Threshold sensors and Discrete
sensors. The common
header components of the compact and full SDR recodr structures were
moved into a single new
sdr record called sdr_record_common_sensor so that the common components
of a compact and
full sdr could be used by the new routines.

-- 
-- Jim Mankovich | jm...@hp.com --


------------------------------------------------------------------------
------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2

_______________________________________________
Ipmitool-devel mailing list
Ipmitool-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel

 

------------------------------------------------------------------------------
Better than sec? Nothing is better than sec when it comes to
monitoring Big Data applications. Try Boundary one-second 
resolution app monitoring today. Free.
http://p.sf.net/sfu/Boundary-dev2dev
_______________________________________________
Ipmitool-devel mailing list
Ipmitool-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel

Reply via email to