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 <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
------------------------------------------------------------------------------ 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
