nut plugin: Multi-threaded ups polling with connect timeout.

Changes:

* Implemented use of 'plugin_register_complex_read', so querying will be multi-threaded;
* Added 'ConnectTimeout' option, implemented with use of 'upscli_tryconnect()' added in nut-2.6.2;
* The `upscli_cleanup()` should be called only at Collectd shutdown, excessive calls was removed;
* Added `upscli_init()` call if `ForceSSL` enabled.
* Added a check for duplicated values in `UPS` option.

Closes: #2344

diff --git a/configure.ac b/configure.ac
index 3357375ead98e8bd27269d756a455d6f360e458b..35b139c45ba21326cd0b2c9473d683f710a55f2e 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -5375,6 +5375,10 @@ if test "x$with_libupsclient" = "xyes"; then
     [AC_DEFINE([HAVE_UPSCLI_INIT], [1], [Define when upscli_init() (since version 2-7) is available.])]
   )
 
+  AC_CHECK_LIB([upsclient], [upscli_tryconnect],
+    [AC_DEFINE([HAVE_UPSCLI_TRYCONNECT], [1], [Define when upscli_tryconnect() (since version 2.6.2) is available.])]
+  )
+
   LDFLAGS="$SAVE_LDFLAGS"
 fi
 

diff --git a/src/collectd.conf.in b/src/collectd.conf.in
index 3eaeb5d535096c0f0813d8f928666af6df31459b..6b59d4d550aef4ce33842c16f122e03ed3d3751d 100644 (file)
--- a/src/collectd.conf.in
+++ b/src/collectd.conf.in
@@ -969,6 +969,7 @@
 #      ForceSSL true
 #      VerifyPeer true
 #      CAPath "/path/to/folder"
+#      #ConnectTimeout 5000
 #</Plugin>
 
 #<Plugin olsrd>

diff --git a/src/collectd.conf.pod b/src/collectd.conf.pod
index 9b31de7ec28d16324ba74f43048b2fef763271fb..de5dc983cc4522ebfffd5eef00e9d08111fee6c3 100644 (file)
--- a/src/collectd.conf.pod
+++ b/src/collectd.conf.pod
@@ -5291,6 +5291,11 @@ generate links like the one described above for ALL certs in a given folder.
 Example usage:
 C<c_rehash /path/to/certs/folder>
 
+=item B<ConnectTimeout> I<Milliseconds>
+
+The B<ConnectTimeout> option sets the connect timeout, in milliseconds.
+By default, the configured B<Interval> is used to set the timeout.
+
 =back
 
 =head2 Plugin C<olsrd>

diff --git a/src/nut.c b/src/nut.c
index 2173af9b5fe3c211bce339378bf60dfe934ebce2..e3820951258f27569e4d8c9da710a3b3b2c2b051 100644 (file)
--- a/src/nut.c
+++ b/src/nut.c
@@ -22,6 +22,7 @@
  *
  * Authors:
  *   Florian octo Forster <octo at collectd.org>
+ *   Pavel Rochnyak <pavel2000 ngs.ru>
  **/
 
 #include "collectd.h"
@@ -49,18 +50,20 @@ struct nut_ups_s {
   nut_ups_t *next;
 };
 
-static nut_ups_t *upslist_head = NULL;
-
-static pthread_mutex_t read_lock = PTHREAD_MUTEX_INITIALIZER;
-static int read_busy = 0;
-
-static const char *config_keys[] = {"UPS", "FORCESSL", "VERIFYPEER", "CAPATH"};
+static const char *config_keys[] = {"UPS", "FORCESSL", "VERIFYPEER", "CAPATH",
+                                    "CONNECTTIMEOUT"};
 static int config_keys_num = STATIC_ARRAY_SIZE(config_keys);
 static int force_ssl = 0;   // Initialized to default of 0 (false)
 static int verify_peer = 0; // Initialized to default of 0 (false)
+static int ssl_flags = UPSCLI_CONN_TRYSSL;
+static int connect_timeout = -1;
 static char *ca_path = NULL;
 
-static void free_nut_ups_t(nut_ups_t *ups) {
+static int nut_read(user_data_t *user_data);
+
+static void free_nut_ups_t(void *arg) {
+  nut_ups_t *ups = arg;
+
   if (ups->conn != NULL) {
     upscli_disconnect(ups->conn);
     sfree(ups->conn);
@@ -73,6 +76,7 @@ static void free_nut_ups_t(nut_ups_t *ups) {
 static int nut_add_ups(const char *name) {
   nut_ups_t *ups;
   int status;
+  char *cb_name;
 
   DEBUG("nut plugin: nut_add_ups (name = %s);", name);
 
@@ -89,13 +93,24 @@ static int nut_add_ups(const char *name) {
     return 1;
   }
 
-  if (upslist_head == NULL)
-    upslist_head = ups;
-  else {
-    nut_ups_t *last = upslist_head;
-    while (last->next != NULL)
-      last = last->next;
-    last->next = ups;
+  cb_name = ssnprintf_alloc("nut/%s", name);
+
+  status = plugin_register_complex_read(
+    /* group = */ "nut",
+    /* name      = */ cb_name,
+    /* callback  = */ nut_read,
+    /* interval  = */ 0,
+    /* user_data = */ &(user_data_t){
+                        .data = ups, .free_func = free_nut_ups_t,
+                      });
+
+  sfree(cb_name);
+
+  if (status == EINVAL) {
+    WARNING("nut plugin: UPS \"%s\" already added. "
+            "Please check your configuration.",
+            name);
+    return -1;
   }
 
   return 0;
@@ -137,6 +152,24 @@ static int nut_ca_path(const char *value) {
   return 0;
 } /* int nut_ca_path */
 
+static int nut_set_connect_timeout(const char *value) {
+#if HAVE_UPSCLI_TRYCONNECT
+  long ret;
+
+  errno = 0;
+  ret = strtol(value, /* endptr = */ NULL, /* base = */ 10);
+  if (errno == 0)
+    connect_timeout = ret;
+  else 
+    WARNING("nut plugin: The ConnectTimeout option requires numeric argument. "
+            "Setting ignored.");
+#else /* #if HAVE_UPSCLI_TRYCONNECT */
+  WARNING("nut plugin: Dependency libupsclient version insufficient (<2.6.2) "
+          "for ConnectTimeout option support. Setting ignored.");
+#endif
+  return 0;
+} /* int nut_set_connect_timeout */
+
 static int nut_config(const char *key, const char *value) {
   if (strcasecmp(key, "UPS") == 0)
     return nut_add_ups(value);
@@ -146,6 +179,8 @@ static int nut_config(const char *key, const char *value) {
     return nut_verify_peer(value);
   else if (strcasecmp(key, "CAPATH") == 0)
     return nut_ca_path(value);
+  else if (strcasecmp(key, "CONNECTTIMEOUT") == 0)
+    return nut_set_connect_timeout(value);
   else
     return -1;
 } /* int nut_config */
@@ -169,48 +204,23 @@ static void nut_submit(nut_ups_t *ups, const char *type,
 } /* void nut_submit */
 
 static int nut_connect(nut_ups_t *ups) {
-#if HAVE_UPSCLI_INIT
-  int status;
-  int ssl_status;
-  int ssl_flags;
-
-  if (verify_peer == 1 && force_ssl == 0) {
-    WARNING("nut plugin: nut_connect: VerifyPeer true but ForceSSL "
-            "false. Setting ForceSSL to true.");
-    force_ssl = 1;
-  }
-
-  if (verify_peer == 1 && ca_path == NULL) {
-    ERROR("nut plugin: nut_connect: VerifyPeer true but missing "
-          "CAPath value.");
-    return -1;
-  }
-
-  if (verify_peer == 1) {
-    status = upscli_init(verify_peer, ca_path, NULL, NULL);
-
-    if (status != 1) {
-      ERROR("nut plugin: nut_connect: upscli_init (%i, %s) failed: %s",
-            verify_peer, ca_path, upscli_strerror(ups->conn));
-      upscli_cleanup();
-      return -1;
-    }
-  } /* if (verify_peer == 1) */
+  int status, ssl_status;
 
-  if (verify_peer == 1)
-    ssl_flags = (UPSCLI_CONN_REQSSL | UPSCLI_CONN_CERTVERIF);
-  else if (force_ssl == 1)
-    ssl_flags = UPSCLI_CONN_REQSSL;
-  else
-    ssl_flags = UPSCLI_CONN_TRYSSL;
+#if HAVE_UPSCLI_TRYCONNECT
+  struct timeval tv;
+  tv.tv_sec = connect_timeout / 1000;
+  tv.tv_usec = connect_timeout % 1000;
 
+  status = upscli_tryconnect(ups->conn, ups->hostname, ups->port, ssl_flags,
+                             &tv);
+#else /* #if HAVE_UPSCLI_TRYCONNECT */
   status = upscli_connect(ups->conn, ups->hostname, ups->port, ssl_flags);
+#endif
 
   if (status != 0) {
     ERROR("nut plugin: nut_connect: upscli_connect (%s, %i) failed: %s",
           ups->hostname, ups->port, upscli_strerror(ups->conn));
     sfree(ups->conn);
-    upscli_cleanup();
     return -1;
   } /* if (status != 0) */
 
@@ -231,57 +241,13 @@ static int nut_connect(nut_ups_t *ups) {
     ERROR("nut plugin: nut_connect: upscli_ssl failed: %s",
           upscli_strerror(ups->conn));
     sfree(ups->conn);
-    upscli_cleanup();
     return -1;
   } /* if (ssl_status == 1 && verify_peer == 1) */
   return 0;
-
-#else /* #if HAVE_UPSCLI_INIT */
-  int status;
-  int ssl_status;
-  int ssl_flags;
-
-  if (verify_peer == 1 || ca_path != NULL) {
-    WARNING("nut plugin: nut_connect: Dependency libupsclient version "
-            "insufficient (<2.7) for VerifyPeer support. Ignoring VerifyPeer "
-            "and CAPath.");
-  }
-
-  if (force_ssl == 1)
-    ssl_flags = UPSCLI_CONN_REQSSL;
-  else
-    ssl_flags = UPSCLI_CONN_TRYSSL;
-
-  status = upscli_connect(ups->conn, ups->hostname, ups->port, ssl_flags);
-
-  if (status != 0) {
-    ERROR("nut plugin: nut_connect: upscli_connect (%s, %i) failed: %s",
-          ups->hostname, ups->port, upscli_strerror(ups->conn));
-    sfree(ups->conn);
-    return -1;
-  } /* if (status != 0) */
-
-  INFO("nut plugin: Connection to (%s, %i) established.", ups->hostname,
-       ups->port);
-
-  // Output INFO or WARNING based on SSL
-  ssl_status = upscli_ssl(ups->conn); // 1 for SSL, 0 for not, -1 for error
-  if (ssl_status == 1) {
-    INFO("nut plugin: Connection is secured with SSL with no verification "
-         "of server SSL certificate.");
-  } else if (ssl_status == 0) {
-    WARNING("nut plugin: Connection is unsecured (no SSL).");
-  } else {
-    ERROR("nut plugin: nut_connect: upscli_ssl failed: %s",
-          upscli_strerror(ups->conn));
-    sfree(ups->conn);
-    return -1;
-  } /* if (ssl_status == 1 && verify_peer == 1) */
-  return 0;
-#endif
 }
 
-static int nut_read_one(nut_ups_t *ups) {
+static int nut_read(user_data_t *user_data) {
+  nut_ups_t *ups = user_data->data;
   const char *query[3] = {"VAR", ups->upsname, NULL};
   unsigned int query_num = 2;
   char **answer;
@@ -306,13 +272,10 @@ static int nut_read_one(nut_ups_t *ups) {
    * error */
   status = upscli_list_start(ups->conn, query_num, query);
   if (status != 0) {
-    ERROR("nut plugin: nut_read_one: upscli_list_start (%s) failed: %s",
+    ERROR("nut plugin: nut_read: upscli_list_start (%s) failed: %s",
           ups->upsname, upscli_strerror(ups->conn));
     upscli_disconnect(ups->conn);
     sfree(ups->conn);
-#if HAVE_UPSCLI_INIT
-    upscli_cleanup();
-#endif
     return -1;
   }
 
@@ -366,40 +329,58 @@ static int nut_read_one(nut_ups_t *ups) {
   } /* while (upscli_list_next) */
 
   return 0;
-} /* int nut_read_one */
+} /* int nut_read */
 
-static int nut_read(void) {
-  int success = 0;
+static int nut_init(void) {
+#if HAVE_UPSCLI_INIT
+  if (verify_peer == 1 && force_ssl == 0) {
+    WARNING("nut plugin: nut_connect: VerifyPeer true but ForceSSL "
+            "false. Setting ForceSSL to true.");
+    force_ssl = 1;
+  }
 
-  pthread_mutex_lock(&read_lock);
-  success = read_busy;
-  read_busy = 1;
-  pthread_mutex_unlock(&read_lock);
+  if (verify_peer == 1 && ca_path == NULL) {
+    ERROR("nut plugin: nut_connect: VerifyPeer true but missing "
+          "CAPath value.");
+    plugin_unregister_read_group("nut");
+    return -1;
+  }
 
-  if (success != 0)
-    return 0;
+  if (verify_peer == 1 || force_ssl == 1) {
+    int status = upscli_init(verify_peer, ca_path, NULL, NULL);
 
-  for (nut_ups_t *ups = upslist_head; ups != NULL; ups = ups->next)
-    if (nut_read_one(ups) == 0)
-      success++;
+    if (status != 1) {
+      ERROR("nut plugin: upscli_init (%i, %s) failed", verify_peer, ca_path);
+      upscli_cleanup();
+      plugin_unregister_read_group("nut");
+      return -1;
+    }
+  } /* if (verify_peer == 1) */
 
-  pthread_mutex_lock(&read_lock);
-  read_busy = 0;
-  pthread_mutex_unlock(&read_lock);
+  if (verify_peer == 1)
+    ssl_flags = (UPSCLI_CONN_REQSSL | UPSCLI_CONN_CERTVERIF);
+  else if (force_ssl == 1)
+    ssl_flags = UPSCLI_CONN_REQSSL;
 
-  return (success != 0) ? 0 : -1;
-} /* int nut_read */
+#else /* #if HAVE_UPSCLI_INIT */
+  if (verify_peer == 1 || ca_path != NULL) {
+    WARNING("nut plugin: nut_connect: Dependency libupsclient version "
+            "insufficient (<2.7) for VerifyPeer support. Ignoring VerifyPeer "
+            "and CAPath.");
+    verify_peer = 0;
+  }
 
-static int nut_shutdown(void) {
-  nut_ups_t *this;
-  nut_ups_t *next;
+  if (force_ssl == 1)
+    ssl_flags = UPSCLI_CONN_REQSSL;
+#endif
 
-  this = upslist_head;
-  while (this != NULL) {
-    next = this->next;
-    free_nut_ups_t(this);
-    this = next;
-  }
+  if (connect_timeout <= 0)
+    connect_timeout = (long)CDTIME_T_TO_MS(plugin_get_interval());
+
+  return 0;
+} /* int nut_init */
+
+static int nut_shutdown(void) {
 #if HAVE_UPSCLI_INIT
   upscli_cleanup();
 #endif
@@ -409,6 +390,6 @@ static int nut_shutdown(void) {
 
 void module_register(void) {
   plugin_register_config("nut", nut_config, config_keys, config_keys_num);
-  plugin_register_read("nut", nut_read);
+  plugin_register_init("nut", nut_init);
   plugin_register_shutdown("nut", nut_shutdown);
 } /* void module_register */