Merge pull request #2742 from elfiesmelfie/ipmi_bugfix_sensor_option
[collectd.git] / src / ipmi.c
index 32d6e51..98cc3b0 100644 (file)
@@ -38,6 +38,8 @@
 #include <OpenIPMI/ipmi_smi.h>
 #include <OpenIPMI/ipmiif.h>
 
+#define ERR_BUF_SIZE 1024
+
 /*
  * Private data types
  */
@@ -77,23 +79,30 @@ struct c_ipmi_sensor_list_s {
   ipmi_sensor_id_t sensor_id;
   char sensor_name[DATA_MAX_NAME_LEN];
   char sensor_type[DATA_MAX_NAME_LEN];
+  char type_instance[DATA_MAX_NAME_LEN];
   int sensor_not_present;
   c_ipmi_sensor_list_t *next;
   c_ipmi_instance_t *instance;
   unsigned int use;
 };
 
+struct c_ipmi_db_type_map_s {
+  enum ipmi_unit_type_e type;
+  const char *type_name;
+};
+typedef struct c_ipmi_db_type_map_s c_ipmi_db_type_map_t;
+
 /*
  * Module global variables
  */
-static os_handler_t *os_handler;
+static os_handler_t *os_handler = NULL;
 static c_ipmi_instance_t *instances = NULL;
 
 /*
  * Misc private functions
  */
 static void c_ipmi_error(c_ipmi_instance_t *st, const char *func, int status) {
-  char errbuf[4096] = {0};
+  char errbuf[ERR_BUF_SIZE] = {0};
 
   if (IPMI_IS_OS_ERR(status) || IPMI_IS_RMCPP_ERR(status) ||
       IPMI_IS_IPMI_ERR(status)) {
@@ -110,7 +119,7 @@ static void c_ipmi_error(c_ipmi_instance_t *st, const char *func, int status) {
 
 static void c_ipmi_log(os_handler_t *handler, const char *format,
                        enum ipmi_log_type_e log_type, va_list ap) {
-  char msg[1024];
+  char msg[ERR_BUF_SIZE];
 
   vsnprintf(msg, sizeof(msg), format, ap);
 
@@ -133,11 +142,11 @@ static void c_ipmi_log(os_handler_t *handler, const char *format,
 #if COLLECT_DEBUG
   case IPMI_LOG_DEBUG_START:
   case IPMI_LOG_DEBUG:
-    fprintf(stderr, "ipmi plugin: %s\n", msg);
+    DEBUG("ipmi plugin: %s", msg);
     break;
   case IPMI_LOG_DEBUG_CONT:
   case IPMI_LOG_DEBUG_END:
-    fprintf(stderr, "%s\n", msg);
+    DEBUG("%s", msg);
     break;
 #else
   case IPMI_LOG_DEBUG_START:
@@ -149,6 +158,14 @@ static void c_ipmi_log(os_handler_t *handler, const char *format,
   }
 } /* void c_ipmi_log */
 
+static notification_t c_ipmi_notification_init(c_ipmi_instance_t const *st,
+                                               int severity) {
+  notification_t n = {severity, cdtime(), "", "", "ipmi", "", "", "", NULL};
+
+  sstrncpy(n.host, (st->host != NULL) ? st->host : hostname_g, sizeof(n.host));
+  return n;
+} /* notification_t c_ipmi_notification_init */
+
 /*
  * Sensor handlers
  */
@@ -162,7 +179,7 @@ static void sensor_read_handler(ipmi_sensor_t *sensor, int err,
                                 void *user_data) {
   value_list_t vl = VALUE_LIST_INIT;
 
-  c_ipmi_sensor_list_t *list_item = (c_ipmi_sensor_list_t *)user_data;
+  c_ipmi_sensor_list_t *list_item = user_data;
   c_ipmi_instance_t *st = list_item->instance;
 
   list_item->use--;
@@ -178,12 +195,9 @@ static void sensor_read_handler(ipmi_sensor_t *sensor, int err,
              list_item->sensor_name, st->name);
 
         if (st->notify_notpresent) {
-          notification_t n = {
-              NOTIF_WARNING, cdtime(), "", "", "ipmi", "", "", "", NULL};
+          notification_t n = c_ipmi_notification_init(st, NOTIF_WARNING);
 
-          sstrncpy(n.host, (st->host != NULL) ? st->host : hostname_g,
-                   sizeof(n.host));
-          sstrncpy(n.type_instance, list_item->sensor_name,
+          sstrncpy(n.type_instance, list_item->type_instance,
                    sizeof(n.type_instance));
           sstrncpy(n.type, list_item->sensor_type, sizeof(n.type));
           snprintf(n.message, sizeof(n.message), "sensor %s not present",
@@ -202,7 +216,7 @@ static void sensor_read_handler(ipmi_sensor_t *sensor, int err,
       INFO("ipmi plugin: sensor_read_handler: Sensor `%s` of `%s` timed out.",
            list_item->sensor_name, st->name);
     } else {
-      char errbuf[128] = {0};
+      char errbuf[ERR_BUF_SIZE] = {0};
       ipmi_get_error_string(err, errbuf, sizeof(errbuf) - 1);
 
       if (IPMI_IS_IPMI_ERR(err))
@@ -234,12 +248,9 @@ static void sensor_read_handler(ipmi_sensor_t *sensor, int err,
          list_item->sensor_name, st->name);
 
     if (st->notify_notpresent) {
-      notification_t n = {NOTIF_OKAY, cdtime(), "", "",  "ipmi",
-                          "",         "",       "", NULL};
+      notification_t n = c_ipmi_notification_init(st, NOTIF_OKAY);
 
-      sstrncpy(n.host, (st->host != NULL) ? st->host : hostname_g,
-               sizeof(n.host));
-      sstrncpy(n.type_instance, list_item->sensor_name,
+      sstrncpy(n.type_instance, list_item->type_instance,
                sizeof(n.type_instance));
       sstrncpy(n.type, list_item->sensor_type, sizeof(n.type));
       snprintf(n.message, sizeof(n.message), "sensor %s present",
@@ -281,7 +292,8 @@ static void sensor_read_handler(ipmi_sensor_t *sensor, int err,
     sstrncpy(vl.host, st->host, sizeof(vl.host));
   sstrncpy(vl.plugin, "ipmi", sizeof(vl.plugin));
   sstrncpy(vl.type, list_item->sensor_type, sizeof(vl.type));
-  sstrncpy(vl.type_instance, list_item->sensor_name, sizeof(vl.type_instance));
+  sstrncpy(vl.type_instance, list_item->type_instance,
+           sizeof(vl.type_instance));
 
   plugin_dispatch_values(&vl);
 } /* void sensor_read_handler */
@@ -334,6 +346,24 @@ static void sensor_get_name(ipmi_sensor_t *sensor, char *buffer, int buf_len) {
   sstrncpy(buffer, sensor_name, buf_len);
 }
 
+static const char *sensor_unit_to_type(ipmi_sensor_t *sensor) {
+  static const c_ipmi_db_type_map_t ipmi_db_type_map[] = {
+      {IPMI_UNIT_TYPE_WATTS, "power"}, {IPMI_UNIT_TYPE_CFM, "flow"}};
+
+  /* check the modifier and rate of the sensor value */
+  if ((ipmi_sensor_get_modifier_unit_use(sensor) != IPMI_MODIFIER_UNIT_NONE) ||
+      (ipmi_sensor_get_rate_unit(sensor) != IPMI_RATE_UNIT_NONE))
+    return NULL;
+
+  /* find the db type by using sensor base unit type */
+  enum ipmi_unit_type_e ipmi_type = ipmi_sensor_get_base_unit(sensor);
+  for (int i = 0; i < STATIC_ARRAY_SIZE(ipmi_db_type_map); i++)
+    if (ipmi_db_type_map[i].type == ipmi_type)
+      return ipmi_db_type_map[i].type_name;
+
+  return NULL;
+} /* const char* sensor_unit_to_type */
+
 static int sensor_list_add(c_ipmi_instance_t *st, ipmi_sensor_t *sensor) {
   ipmi_sensor_id_t sensor_id;
   c_ipmi_sensor_list_t *list_item;
@@ -373,10 +403,10 @@ static int sensor_list_add(c_ipmi_instance_t *st, ipmi_sensor_t *sensor) {
    *
    * ipmi_sensor_id_get_reading() supports only 'Threshold' sensors.
    * See lib/sensor.c:4842, stand_ipmi_sensor_get_reading() for details.
-  */
+   */
   if (!ipmi_sensor_get_is_readable(sensor)) {
     INFO("ipmi plugin: sensor_list_add: Ignore sensor `%s` of `%s`, "
-         "because it don't readable! Its type: (%#x, %s). ",
+         "because it isn't readable! Its type: (%#x, %s). ",
          sensor_name_ptr, st->name, sensor_type,
          ipmi_sensor_get_sensor_type_string(sensor));
     return -1;
@@ -409,11 +439,22 @@ static int sensor_list_add(c_ipmi_instance_t *st, ipmi_sensor_t *sensor) {
     type = "fanspeed";
     break;
 
+  case IPMI_SENSOR_TYPE_MEMORY:
+    type = "memory";
+    break;
+
   default: {
+    /* try to get collectd DB type based on sensor base unit type */
+    if ((type = sensor_unit_to_type(sensor)) != NULL)
+      break;
+
     INFO("ipmi plugin: sensor_list_add: Ignore sensor `%s` of `%s`, "
-         "because I don't know how to handle its type (%#x, %s). "
-         "If you need this sensor, please file a bug report.",
-         sensor_name_ptr, st->name, sensor_type,
+         "because I don't know how to handle its units (%#x, %#x, %#x). "
+         "Sensor type: (%#x, %s). If you need this sensor, please file "
+         "a bug report at http://collectd.org/.",
+         sensor_name_ptr, st->name, ipmi_sensor_get_base_unit(sensor),
+         ipmi_sensor_get_modifier_unit(sensor),
+         ipmi_sensor_get_rate_unit(sensor), sensor_type,
          ipmi_sensor_get_sensor_type_string(sensor));
     return -1;
   }
@@ -448,6 +489,18 @@ static int sensor_list_add(c_ipmi_instance_t *st, ipmi_sensor_t *sensor) {
   else
     st->sensor_list = list_item;
 
+  /* if sensor provides the percentage value, use "percent" collectd type
+     and add the `percent` to the type instance of the reported value */
+  if (ipmi_sensor_get_percentage(sensor)) {
+    snprintf(list_item->type_instance, sizeof(list_item->type_instance),
+             "percent-%s", sensor_name_ptr);
+    type = "percent";
+  } else {
+    /* use type instance as a name of the sensor */
+    sstrncpy(list_item->type_instance, sensor_name_ptr,
+             sizeof(list_item->type_instance));
+  }
+
   sstrncpy(list_item->sensor_name, sensor_name_ptr,
            sizeof(list_item->sensor_name));
   sstrncpy(list_item->sensor_type, type, sizeof(list_item->sensor_type));
@@ -455,11 +508,10 @@ static int sensor_list_add(c_ipmi_instance_t *st, ipmi_sensor_t *sensor) {
   pthread_mutex_unlock(&st->sensor_list_lock);
 
   if (st->notify_add && (st->init_in_progress == 0)) {
-    notification_t n = {NOTIF_OKAY, cdtime(), "", "", "ipmi", "", "", "", NULL};
+    notification_t n = c_ipmi_notification_init(st, NOTIF_OKAY);
 
-    sstrncpy(n.host, (st->host != NULL) ? st->host : hostname_g,
-             sizeof(n.host));
-    sstrncpy(n.type_instance, list_item->sensor_name, sizeof(n.type_instance));
+    sstrncpy(n.type_instance, list_item->type_instance,
+             sizeof(n.type_instance));
     sstrncpy(n.type, list_item->sensor_type, sizeof(n.type));
     snprintf(n.message, sizeof(n.message), "sensor %s added",
              list_item->sensor_name);
@@ -503,12 +555,10 @@ static int sensor_list_remove(c_ipmi_instance_t *st, ipmi_sensor_t *sensor) {
   pthread_mutex_unlock(&st->sensor_list_lock);
 
   if (st->notify_remove && st->active) {
-    notification_t n = {NOTIF_WARNING, cdtime(), "", "", "ipmi", "", "", "",
-                        NULL};
+    notification_t n = c_ipmi_notification_init(st, NOTIF_WARNING);
 
-    sstrncpy(n.host, (st->host != NULL) ? st->host : hostname_g,
-             sizeof(n.host));
-    sstrncpy(n.type_instance, list_item->sensor_name, sizeof(n.type_instance));
+    sstrncpy(n.type_instance, list_item->type_instance,
+             sizeof(n.type_instance));
     sstrncpy(n.type, list_item->sensor_type, sizeof(n.type));
     snprintf(n.message, sizeof(n.message), "sensor %s removed",
              list_item->sensor_name);
@@ -603,7 +653,7 @@ static int sensor_threshold_event_handler(
     enum ipmi_value_present_e value_present, unsigned int raw_value,
     double value, void *cb_data, ipmi_event_t *event) {
 
-  c_ipmi_instance_t *st = (c_ipmi_instance_t *)cb_data;
+  c_ipmi_instance_t *st = cb_data;
 
   /* From the IPMI specification Chapter 2: Events.
    * If a callback handles the event, then all future callbacks called due to
@@ -613,9 +663,9 @@ static int sensor_threshold_event_handler(
   if (event == NULL)
     return IPMI_EVENT_NOT_HANDLED;
 
+  notification_t n = c_ipmi_notification_init(st, NOTIF_OKAY);
   /* offset is a table index and it's represented as enum of strings that are
      organized in the way - high and low for each threshold severity level */
-  notification_t n = {NOTIF_OKAY, cdtime(), "", "", "ipmi", "", "", "", NULL};
   unsigned int offset = (2 * threshold) + high_low;
   unsigned int event_type = ipmi_sensor_get_event_reading_type(sensor);
   unsigned int sensor_type = ipmi_sensor_get_sensor_type(sensor);
@@ -633,7 +683,6 @@ static int sensor_threshold_event_handler(
 
   DEBUG("Threshold event received for sensor %s", n.type_instance);
 
-  sstrncpy(n.host, (st->host != NULL) ? st->host : hostname_g, sizeof(n.host));
   sstrncpy(n.type, ipmi_sensor_get_sensor_type_string(sensor), sizeof(n.type));
   n.severity = sensor_convert_threshold_severity(threshold);
   n.time = NS_TO_CDTIME_T(ipmi_event_get_timestamp(event));
@@ -675,7 +724,7 @@ static int sensor_discrete_event_handler(ipmi_sensor_t *sensor,
                                          int severity, int prev_severity,
                                          void *cb_data, ipmi_event_t *event) {
 
-  c_ipmi_instance_t *st = (c_ipmi_instance_t *)cb_data;
+  c_ipmi_instance_t *st = cb_data;
 
   /* From the IPMI specification Chapter 2: Events.
    * If a callback handles the event, then all future callbacks called due to
@@ -685,7 +734,7 @@ static int sensor_discrete_event_handler(ipmi_sensor_t *sensor,
   if (event == NULL)
     return IPMI_EVENT_NOT_HANDLED;
 
-  notification_t n = {NOTIF_OKAY, cdtime(), "", "", "ipmi", "", "", "", NULL};
+  notification_t n = c_ipmi_notification_init(st, NOTIF_OKAY);
   unsigned int event_type = ipmi_sensor_get_event_reading_type(sensor);
   unsigned int sensor_type = ipmi_sensor_get_sensor_type(sensor);
   const char *event_state =
@@ -696,7 +745,6 @@ static int sensor_discrete_event_handler(ipmi_sensor_t *sensor,
 
   DEBUG("Discrete event received for sensor %s", n.type_instance);
 
-  sstrncpy(n.host, (st->host != NULL) ? st->host : hostname_g, sizeof(n.host));
   sstrncpy(n.type, ipmi_sensor_get_sensor_type_string(sensor), sizeof(n.type));
   n.time = NS_TO_CDTIME_T(ipmi_event_get_timestamp(event));
 
@@ -729,7 +777,7 @@ static void
 entity_sensor_update_handler(enum ipmi_update_e op,
                              ipmi_entity_t __attribute__((unused)) * entity,
                              ipmi_sensor_t *sensor, void *user_data) {
-  c_ipmi_instance_t *st = (c_ipmi_instance_t *)user_data;
+  c_ipmi_instance_t *st = user_data;
 
   if ((op == IPMI_ADDED) || (op == IPMI_CHANGED)) {
     /* Will check for duplicate entries.. */
@@ -737,11 +785,9 @@ entity_sensor_update_handler(enum ipmi_update_e op,
 
     if (st->sel_enabled) {
       int status = 0;
-      /* register threshold event if threshold sensor support events */
-      if ((ipmi_sensor_get_event_reading_type(sensor) ==
-           IPMI_EVENT_READING_TYPE_THRESHOLD) &&
-          (ipmi_sensor_get_threshold_access(sensor) !=
-           IPMI_THRESHOLD_ACCESS_SUPPORT_NONE))
+      /* register threshold event handler */
+      if (ipmi_sensor_get_event_reading_type(sensor) ==
+          IPMI_EVENT_READING_TYPE_THRESHOLD)
         status = ipmi_sensor_add_threshold_event_handler(
             sensor, sensor_threshold_event_handler, st);
       /* register discrete handler if discrete/specific sensor support events */
@@ -778,7 +824,7 @@ domain_entity_update_handler(enum ipmi_update_e op,
                              ipmi_domain_t __attribute__((unused)) * domain,
                              ipmi_entity_t *entity, void *user_data) {
   int status;
-  c_ipmi_instance_t *st = (c_ipmi_instance_t *)user_data;
+  c_ipmi_instance_t *st = user_data;
 
   if (op == IPMI_ADDED) {
     status = ipmi_entity_add_sensor_update_handler(
@@ -823,7 +869,7 @@ static void domain_connection_change_handler(ipmi_domain_t *domain, int err,
         "user_data = %p);",
         (void *)domain, err, conn_num, port_num, still_connected, user_data);
 
-  c_ipmi_instance_t *st = (c_ipmi_instance_t *)user_data;
+  c_ipmi_instance_t *st = user_data;
 
   if (err != 0)
     c_ipmi_error(st, "domain_connection_change_handler", err);
@@ -831,11 +877,8 @@ static void domain_connection_change_handler(ipmi_domain_t *domain, int err,
   if (!still_connected) {
 
     if (st->notify_conn && st->connected && st->init_in_progress == 0) {
-      notification_t n = {NOTIF_FAILURE, cdtime(), "", "", "ipmi", "", "", "",
-                          NULL};
+      notification_t n = c_ipmi_notification_init(st, NOTIF_FAILURE);
 
-      sstrncpy(n.host, (st->host != NULL) ? st->host : hostname_g,
-               sizeof(n.host));
       sstrncpy(n.message, "IPMI connection lost", sizeof(n.plugin));
 
       plugin_dispatch_notification(&n);
@@ -846,10 +889,8 @@ static void domain_connection_change_handler(ipmi_domain_t *domain, int err,
   }
 
   if (st->notify_conn && !st->connected && st->init_in_progress == 0) {
-    notification_t n = {NOTIF_OKAY, cdtime(), "", "", "ipmi", "", "", "", NULL};
+    notification_t n = c_ipmi_notification_init(st, NOTIF_OKAY);
 
-    sstrncpy(n.host, (st->host != NULL) ? st->host : hostname_g,
-             sizeof(n.host));
     sstrncpy(n.message, "IPMI connection restored", sizeof(n.plugin));
 
     plugin_dispatch_notification(&n);
@@ -875,16 +916,11 @@ static int c_ipmi_thread_init(c_ipmi_instance_t *st) {
   int status;
 
   if (st->connaddr != NULL) {
-    char *ip_addrs[1] = {NULL}, *ports[1] = {NULL};
-
-    ip_addrs[0] = strdup(st->connaddr);
-    ports[0] = strdup(IPMI_LAN_STD_PORT_STR);
-
-    status = ipmi_ip_setup_con(ip_addrs, ports, 1, st->authtype,
-                               (unsigned int)IPMI_PRIVILEGE_USER, st->username,
-                               strlen(st->username), st->password,
-                               strlen(st->password), os_handler,
-                               /* user data = */ NULL, &st->connection);
+    status = ipmi_ip_setup_con(
+        &st->connaddr, &(char *){IPMI_LAN_STD_PORT_STR}, 1, st->authtype,
+        (unsigned int)IPMI_PRIVILEGE_USER, st->username, strlen(st->username),
+        st->password, strlen(st->password), os_handler,
+        /* user data = */ NULL, &st->connection);
     if (status != 0) {
       c_ipmi_error(st, "ipmi_ip_setup_con", status);
       return -1;
@@ -898,19 +934,13 @@ static int c_ipmi_thread_init(c_ipmi_instance_t *st) {
     }
   }
 
-  size_t open_option_num = 0;
-  ipmi_open_option_t open_option[2];
-
-  open_option[open_option_num].option = IPMI_OPEN_OPTION_ALL;
-  open_option[open_option_num].ival = 1;
-  open_option_num++;
-
+  ipmi_open_option_t opts[] = {
+      {.option = IPMI_OPEN_OPTION_ALL, {.ival = 1}},
 #ifdef IPMI_OPEN_OPTION_USE_CACHE
-  // This option appeared in OpenIPMI-2.0.17
-  open_option[open_option_num].option = IPMI_OPEN_OPTION_USE_CACHE;
-  open_option[open_option_num].ival = 0; /* Disable SDR cache in local file */
-  open_option_num++;
+      /* OpenIPMI-2.0.17 and later: Disable SDR cache in local file */
+      {.option = IPMI_OPEN_OPTION_USE_CACHE, {.ival = 0}},
 #endif
+  };
 
   /*
    * NOTE: Domain names must be unique. There is static `domains_list` common
@@ -919,8 +949,8 @@ static int c_ipmi_thread_init(c_ipmi_instance_t *st) {
   status = ipmi_open_domain(
       st->name, &st->connection, /* num_con = */ 1,
       domain_connection_change_handler, /* user data = */ (void *)st,
-      /* domain_fully_up_handler = */ NULL, /* user data = */ NULL, open_option,
-      open_option_num, &domain_id);
+      /* domain_fully_up_handler = */ NULL, /* user data = */ NULL, opts,
+      STATIC_ARRAY_SIZE(opts), &domain_id);
   if (status != 0) {
     c_ipmi_error(st, "ipmi_open_domain", status);
     return -1;
@@ -930,7 +960,7 @@ static int c_ipmi_thread_init(c_ipmi_instance_t *st) {
 } /* int c_ipmi_thread_init */
 
 static void *c_ipmi_thread_main(void *user_data) {
-  c_ipmi_instance_t *st = (c_ipmi_instance_t *)user_data;
+  c_ipmi_instance_t *st = user_data;
 
   int status = c_ipmi_thread_init(st);
   if (status != 0) {
@@ -1032,31 +1062,31 @@ static int c_ipmi_config_add_instance(oconfig_item_t *ci) {
   for (int i = 0; i < ci->children_num; i++) {
     oconfig_item_t *child = ci->children + i;
 
-    if (strcasecmp("Sensor", child->key) == 0)
-      ignorelist_add(st->ignorelist, ci->values[0].value.string);
-    else if (strcasecmp("IgnoreSelected", child->key) == 0) {
-      if (ci->values[0].value.boolean)
-        ignorelist_set_invert(st->ignorelist, /* invert = */ 0);
-      else
-        ignorelist_set_invert(st->ignorelist, /* invert = */ 1);
+    if (strcasecmp("Sensor", child->key) == 0) {
+      char *value = NULL;
+      status = cf_util_get_string(child, &value);
+      if (status != 0)
+        break;
+      ignorelist_add(st->ignorelist, value);
+      sfree(value);
+    } else if (strcasecmp("IgnoreSelected", child->key) == 0) {
+      _Bool t;
+      status = cf_util_get_boolean(child, &t);
+      if (status != 0)
+        break;
+      ignorelist_set_invert(st->ignorelist, /* invert = */ !t);
     } else if (strcasecmp("NotifyIPMIConnectionState", child->key) == 0) {
-      if (ci->values[0].value.boolean)
-        st->notify_conn = 1;
+      status = cf_util_get_boolean(child, &st->notify_conn);
     } else if (strcasecmp("NotifySensorAdd", child->key) == 0) {
-      if (ci->values[0].value.boolean)
-        st->notify_add = 1;
+      status = cf_util_get_boolean(child, &st->notify_add);
     } else if (strcasecmp("NotifySensorRemove", child->key) == 0) {
-      if (ci->values[0].value.boolean)
-        st->notify_remove = 1;
+      status = cf_util_get_boolean(child, &st->notify_remove);
     } else if (strcasecmp("NotifySensorNotPresent", child->key) == 0) {
-      if (ci->values[0].value.boolean)
-        st->notify_notpresent = 1;
+      status = cf_util_get_boolean(child, &st->notify_notpresent);
     } else if (strcasecmp("SELEnabled", child->key) == 0) {
-      if (ci->values[0].value.boolean)
-        st->sel_enabled = 1;
+      status = cf_util_get_boolean(child, &st->sel_enabled);
     } else if (strcasecmp("SELClearEvent", child->key) == 0) {
-      if (ci->values[0].value.boolean)
-        st->sel_clear_event = 1;
+      status = cf_util_get_boolean(child, &st->sel_clear_event);
     } else if (strcasecmp("Host", child->key) == 0)
       status = cf_util_get_string(child, &st->host);
     else if (strcasecmp("Address", child->key) == 0)
@@ -1105,19 +1135,26 @@ static int c_ipmi_config(oconfig_item_t *ci) {
     oconfig_item_t *child = ci->children + i;
 
     if (strcasecmp("Instance", child->key) == 0) {
-      c_ipmi_config_add_instance(child);
+      int status = c_ipmi_config_add_instance(child);
+      if (status != 0)
+        return status;
+
       have_instance_block = 1;
     } else if (!have_instance_block) {
       /* Non-instance option: Assume legacy configuration (without <Instance />
        * blocks) and call c_ipmi_config_add_instance with the <Plugin /> block.
        */
+      WARNING("ipmi plugin: Legacy configuration found! Please update your "
+              "config file.");
       return c_ipmi_config_add_instance(ci);
-    } else
+    } else {
       WARNING("ipmi plugin: The configuration option "
               "\"%s\" is not allowed here. Did you "
               "forget to add an <Instance /> block "
               "around the configuration?",
               child->key);
+      return -1;
+    }
   } /* for (ci->children) */
 
   return 0;
@@ -1126,7 +1163,7 @@ static int c_ipmi_config(oconfig_item_t *ci) {
 static int c_ipmi_read(user_data_t *user_data) {
   c_ipmi_instance_t *st = user_data->data;
 
-  if ((st->active == 0) || (st->thread_id == (pthread_t)0)) {
+  if (st->active == 0) {
     INFO("ipmi plugin: c_ipmi_read: I'm not active, returning false.");
     return -1;
   }
@@ -1145,10 +1182,13 @@ static int c_ipmi_read(user_data_t *user_data) {
 } /* int c_ipmi_read */
 
 static int c_ipmi_init(void) {
-  int status;
   c_ipmi_instance_t *st;
   char callback_name[3 * DATA_MAX_NAME_LEN];
 
+  if (os_handler != NULL) {
+    return 0;
+  }
+
   os_handler = ipmi_posix_thread_setup_os_handler(SIGIO);
   if (os_handler == NULL) {
     ERROR("ipmi plugin: ipmi_posix_thread_setup_os_handler failed.");
@@ -1163,9 +1203,6 @@ static int c_ipmi_init(void) {
     return -1;
   };
 
-  /* Don't send `ADD' notifications during startup (~ 1 minute) */
-  time_t iv = CDTIME_T_TO_TIME_T(plugin_get_interval());
-
   if (instances == NULL) {
     /* No instances were configured, let's start a default instance. */
     st = c_ipmi_init_instance();
@@ -1175,6 +1212,9 @@ static int c_ipmi_init(void) {
     c_ipmi_add_instance(st);
   }
 
+  /* Don't send `ADD' notifications during startup (~ 1 minute) */
+  int cycles = 1 + (int)(TIME_T_TO_CDTIME_T(60) / plugin_get_interval());
+
   st = instances;
   while (NULL != st) {
     /* The `st->name` is used as "domain name" for ipmi_open_domain().
@@ -1186,7 +1226,7 @@ static int c_ipmi_init(void) {
         .data = st,
     };
 
-    status = plugin_register_complex_read(
+    int status = plugin_register_complex_read(
         /* group     = */ "ipmi",
         /* name      = */ callback_name,
         /* callback  = */ c_ipmi_read,
@@ -1194,12 +1234,11 @@ static int c_ipmi_init(void) {
         /* user_data = */ &ud);
 
     if (status != 0) {
-      st->active = 0;
       st = st->next;
       continue;
     }
 
-    st->init_in_progress = 1 + (60 / iv);
+    st->init_in_progress = cycles;
     st->active = 1;
 
     status = plugin_thread_create(&st->thread_id, /* attr = */ NULL,
@@ -1208,7 +1247,7 @@ static int c_ipmi_init(void) {
 
     if (status != 0) {
       st->active = 0;
-      st->thread_id = (pthread_t)0;
+      st->thread_id = (pthread_t){0};
 
       plugin_unregister_read(callback_name);
 
@@ -1231,9 +1270,9 @@ static int c_ipmi_shutdown(void) {
     st->next = NULL;
     st->active = 0;
 
-    if (st->thread_id != (pthread_t)0) {
+    if (!pthread_equal(st->thread_id, (pthread_t){0})) {
       pthread_join(st->thread_id, NULL);
-      st->thread_id = (pthread_t)0;
+      st->thread_id = (pthread_t){0};
     }
 
     sensor_list_remove_all(st);
@@ -1243,6 +1282,7 @@ static int c_ipmi_shutdown(void) {
   }
 
   os_handler->free_os_handler(os_handler);
+  os_handler = NULL;
 
   return 0;
 } /* int c_ipmi_shutdown */