Andy,

Thanks for taking the time to test it out and report what you found,  it is very
much appreciated :)

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


On 4/10/2012 1:40 PM, Andy Cress wrote:

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  <mailto: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 
<mailto: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 <mailto: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 
<mailto:Ipmitool-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel


WARNING - This e-mail or its attachments may contain controlled technical data or controlled technology within the definition of the International Traffic in Arms Regulations (ITAR) or Export Administration Regulations (EAR), and are subject to the export control laws of the U.S. Government. Transfer of this data or technology by any means to a foreign person, whether in the United States or abroad, without an export license or other approval from the U.S. Government, is prohibited. The information contained in this document is CONFIDENTIAL and property of Kontron. Any unauthorized review, use, disclosure or distribution is prohibited without express written consent of Kontron. If you are not the intended recipient, please contact the sender and destroy all copies of the original message and enclosed attachments. ­­
------------------------------------------------------------------------------
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