Beware, excavations below.

Looking at the proposed change and at the values already printed...

The new: (evt->sel_type.standard_type.event_dir ? "Deasserted" : "Asserted"),
And old: ((evt->sel_type.standard_type.event_data[0] & 0xf) % 2) ? ">" : "<",

The old line does not make much sense to me. The spec 32.1 redirected me to 
table 29-6
and the "event_data[0] & 0xf) %2" should be the lowest bit [0] from of data 1
in all 3 sensor classes. However I am new to SEL records...
And I found change from year 2005 that does:

@@ -68,9 +68,7 @@ struct sel_get_rq {
        uint8_t length;
 } __attribute__ ((packed));
 
-struct sel_event_record {
-       uint16_t        record_id;
-       uint8_t record_type;
+struct standard_spec_sel_rec{
        uint32_t        timestamp;
        uint16_t        gen_id;
        uint8_t evm_rev;
@@ -87,22 +85,28 @@ struct sel_event_record {
 #define DATA_BYTE3_SPECIFIED_MASK 0x30    /* event_data[0] bit mask */
 #define EVENT_OFFSET_MASK         0x0f    /* event_data[0] bit mask */
        uint8_t event_data[3];
-} __attribute__ ((packed));
+};

Based on that I would say replace the old line's event_data[0]... with the 
event_dir
and keep the short ">" : "<" form. Is there anyone who can confirm this?



----- Original Message -----
From: "Corey Minyard" <tcminy...@gmail.com>
To: "Ales Ledvinka" <aledv...@redhat.com>
Cc: ipmitool-devel@lists.sourceforge.net
Sent: Friday, March 22, 2013 8:12:58 PM
Subject: Re: [Ipmitool-devel] [PATCH 0/2] Some minor enhancements to SOL and 
event printing

On 03/22/2013 11:50 AM, Ales Ledvinka wrote:
> Similar issue with ancient patch.
> https://sourceforge.net/tracker/?func=detail&aid=2221609&group_id=95200&atid=610552
> Please check whether the comment there is relevant to your change as well.
>
> This is neither ACK nor NACK. Did not check your patch yet.
> Just pointing out that there might be few more places to fix and handle that 
> consistently.
>
> ----- Original Message -----
> From: "Corey Minyard" <tcminy...@gmail.com>
> To: ipmitool-devel@lists.sourceforge.net
> Sent: Friday, March 22, 2013 2:39:02 PM
> Subject: [Ipmitool-devel] [PATCH 0/2] Some minor enhancements to SOL and      
> event printing
>
> I sent both of these in previously and I didn't hear anything. These are
> some minor enhancements to ipmitool
>
> -corey
>
> ------------------------------------------------------------------------------
> 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_d2d_mar
> _______________________________________________
> Ipmitool-devel mailing list
> Ipmitool-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
Yes, thanks, I didn't know about ipmievd, I guess I was assuming there 
would be one place to handle this.  And it turns out that due to a 
missing %s, Asserted or Deasserted was never printed for type 1 SDRs, 
even on discrete events.

How about the following patch:

Index: lib/ipmi_sel.c
===================================================================
RCS file: /cvsroot/ipmitool/ipmitool/lib/ipmi_sel.c,v
retrieving revision 1.90
diff -u -r1.90 ipmi_sel.c
--- lib/ipmi_sel.c    16 Jan 2013 12:27:29 -0000    1.90
+++ lib/ipmi_sel.c    22 Mar 2013 19:10:06 -0000
@@ -1709,17 +1709,15 @@
          free(description);
      }

-    if (evt->sel_type.standard_type.event_type == 0x6f) {
-        if (csv_output)
-            printf(",");
-        else
-            printf(" | ");
+    if (csv_output)
+        printf(",");
+    else
+        printf(" | ");

-        if (evt->sel_type.standard_type.event_dir)
-            printf("Deasserted");
-        else
-            printf("Asserted");
-    }
+    if (evt->sel_type.standard_type.event_dir)
+        printf("Deasserted");
+    else
+        printf("Asserted");

      if (sdr != NULL && evt->sel_type.standard_type.event_type == 1) {
          /*
Index: src/ipmievd.c
===================================================================
RCS file: /cvsroot/ipmitool/ipmitool/src/ipmievd.c,v
retrieving revision 1.44
diff -u -r1.44 ipmievd.c
--- src/ipmievd.c    28 Sep 2012 18:47:01 -0000    1.44
+++ src/ipmievd.c    22 Mar 2013 19:10:06 -0000
@@ -278,11 +278,13 @@
                      sdr->record.full, 
evt->sel_type.standard_type.event_data[2]);
              }

-            lprintf(LOG_NOTICE, "%s%s sensor %s %s (Reading %.*f %s 
Threshold %.*f %s)",
+            lprintf(LOG_NOTICE, "%s%s sensor %s %s %s (Reading %.*f %s 
Threshold %.*f %s)",
                  eintf->prefix,
                  type,
                  sdr->record.full->id_string,
                  desc ? : "",
+                (evt->sel_type.standard_type.event_dir
+                 ? "Deasserted" : "Asserted"),
                  (trigger_reading==(int)trigger_reading) ? 0 : 2,
                  trigger_reading,
                  ((evt->sel_type.standard_type.event_data[0] & 0xf) % 
2) ? ">" : "<",
@@ -298,8 +300,11 @@
              /*
               * Discrete Event
               */
-            lprintf(LOG_NOTICE, "%s%s sensor %s %s",
-                eintf->prefix, type, sdr->record.full->id_string, desc 
? : "");
+            lprintf(LOG_NOTICE, "%s%s sensor %s %s %s",
+                eintf->prefix, type,
+                sdr->record.full->id_string, desc ? : "",
+                (evt->sel_type.standard_type.event_dir
+                 ? "Deasserted" : "Asserted"));
              if (((evt->sel_type.standard_type.event_data[0] >> 6) & 3) 
== 1) {
                  /* previous state and/or severity in event data byte 2 */
              }
@@ -308,23 +313,20 @@
              /*
               * OEM Event
               */
-            lprintf(LOG_NOTICE, "%s%s sensor %s %s",
-                eintf->prefix, type, sdr->record.full->id_string, desc 
? : "");
+            lprintf(LOG_NOTICE, "%s%s sensor %s %s %s",
+                eintf->prefix, type,
+                sdr->record.full->id_string, desc ? : "",
+                (evt->sel_type.standard_type.event_dir
+                 ? "Deasserted" : "Asserted"));
          }
          break;

      case SDR_RECORD_TYPE_COMPACT_SENSOR:
-        if (evt->sel_type.standard_type.event_type == 0x6f) {
-            lprintf(LOG_NOTICE, "%s%s sensor %s - %s %s",
-                eintf->prefix,
-                type, sdr->record.compact->id_string,
-                desc ? : "",
-                evt->sel_type.standard_type.event_dir ? "Deasserted" : 
"Asserted");
-        } else {
-            lprintf(LOG_NOTICE, "%s%s sensor %s - %s",
-                eintf->prefix, type,
-                sdr->record.compact->id_string, desc ? : "");
-        }
+        lprintf(LOG_NOTICE, "%s%s sensor %s - %s %s",
+            eintf->prefix, type,
+            sdr->record.compact->id_string, desc ? : "",
+            (evt->sel_type.standard_type.event_dir
+             ? "Deasserted" : "Asserted"));
          break;

      default:


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

Reply via email to