Merge branch 'collectd-5.8'
[collectd.git] / src / virt.c
index 99483c4..c52a8a7 100644 (file)
@@ -34,6 +34,7 @@
 #include <libxml/tree.h>
 #include <libxml/xpath.h>
 #include <libxml/xpathInternals.h>
+#include <stdbool.h>
 
 /* Plugin name */
 #define PLUGIN_NAME "virt"
 
 #endif /* LIBVIR_CHECK_VERSION */
 
+/* structure used for aggregating notification-thread data*/
+typedef struct virt_notif_thread_s {
+  pthread_t event_loop_tid;
+  int domain_event_cb_id;
+  pthread_mutex_t active_mutex; /* protects 'is_active' member access*/
+  bool is_active;
+} virt_notif_thread_t;
+
 static const char *config_keys[] = {"Connection",
 
                                     "RefreshInterval",
@@ -130,12 +139,10 @@ static const char *config_keys[] = {"Connection",
                                     NULL};
 
 /* PersistentNotification is false by default */
-static _Bool persistent_notification = 0;
-
-/* libvirt event loop */
-static pthread_t event_loop_tid;
+static bool persistent_notification = false;
 
-static int domain_event_cb_id;
+/* Thread used for handling libvirt notifications events */
+static virt_notif_thread_t notif_thread;
 
 const char *domain_states[] = {
         [VIR_DOMAIN_NOSTATE] = "no state",
@@ -461,8 +468,8 @@ const char *domain_reasons[][DOMAIN_STATE_REASON_MAX_SIZE] = {
   } while (0)
 
 /* Connection. */
-static virConnectPtr conn = 0;
-static char *conn_string = NULL;
+static virConnectPtr conn;
+static char *conn_string;
 static c_complain_t conn_complain = C_COMPLAIN_INIT_STATIC;
 
 /* Node information required for %CPU */
@@ -472,11 +479,11 @@ static virNodeInfo nodeinfo;
 static int interval = 60;
 
 /* List of domains, if specified. */
-static ignorelist_t *il_domains = NULL;
+static ignorelist_t *il_domains;
 /* List of block devices, if specified. */
-static ignorelist_t *il_block_devices = NULL;
+static ignorelist_t *il_block_devices;
 /* List of network interface devices, if specified. */
-static ignorelist_t *il_interface_devices = NULL;
+static ignorelist_t *il_interface_devices;
 
 static int ignore_device_match(ignorelist_t *, const char *domname,
                                const char *devpath);
@@ -498,7 +505,7 @@ struct interface_device {
 typedef struct domain_s {
   virDomainPtr ptr;
   virDomainInfo info;
-  _Bool active;
+  bool active;
 } domain_t;
 
 struct lv_read_state {
@@ -515,7 +522,7 @@ struct lv_read_state {
 
 static void free_domains(struct lv_read_state *state);
 static int add_domain(struct lv_read_state *state, virDomainPtr dom,
-                      _Bool active);
+                      bool active);
 
 static void free_block_devices(struct lv_read_state *state);
 static int add_block_device(struct lv_read_state *state, virDomainPtr dom,
@@ -623,7 +630,7 @@ static const struct ex_stats_item ex_stats_table[] = {
 };
 
 /* BlockDeviceFormatBasename */
-_Bool blockdevice_format_basename = 0;
+static bool blockdevice_format_basename;
 static enum bd_field blockdevice_format = target;
 static enum if_field interface_format = if_name;
 
@@ -1009,7 +1016,7 @@ static unsigned int parse_ex_stats_flags(char **exstats, int numexstats) {
 }
 
 static void domain_state_submit_notif(virDomainPtr dom, int state, int reason) {
-  if ((state < 0) || (state >= STATIC_ARRAY_SIZE(domain_states))) {
+  if ((state < 0) || ((size_t)state >= STATIC_ARRAY_SIZE(domain_states))) {
     ERROR(PLUGIN_NAME ": Array index out of bounds: state=%d", state);
     return;
   }
@@ -1017,7 +1024,8 @@ static void domain_state_submit_notif(virDomainPtr dom, int state, int reason) {
   char msg[DATA_MAX_NAME_LEN];
   const char *state_str = domain_states[state];
 #ifdef HAVE_DOM_REASON
-  if ((reason < 0) || (reason >= STATIC_ARRAY_SIZE(domain_reasons[0]))) {
+  if ((reason < 0) ||
+      ((size_t)reason >= STATIC_ARRAY_SIZE(domain_reasons[0]))) {
     ERROR(PLUGIN_NAME ": Array index out of bounds: reason=%d", reason);
     return;
   }
@@ -1116,7 +1124,7 @@ static int lv_config(const char *key, const char *value) {
     return 0;
   }
   if (strcasecmp(key, "BlockDeviceFormatBasename") == 0) {
-    blockdevice_format_basename = IS_TRUE(value);
+    blockdevice_format_basename = IS_TRUE(value) ? true : false;
     return 0;
   }
   if (strcasecmp(key, "InterfaceDevice") == 0) {
@@ -1390,7 +1398,7 @@ static void vcpu_pin_submit(virDomainPtr dom, int max_cpus, int vcpu,
                             unsigned char *cpu_maps, int cpu_map_len) {
   for (int cpu = 0; cpu < max_cpus; ++cpu) {
     char type_instance[DATA_MAX_NAME_LEN];
-    _Bool is_set = VIR_CPU_USABLE(cpu_maps, cpu_map_len, vcpu, cpu) ? 1 : 0;
+    bool is_set = VIR_CPU_USABLE(cpu_maps, cpu_map_len, vcpu, cpu);
 
     snprintf(type_instance, sizeof(type_instance), "vcpu_%d-cpu_%d", vcpu, cpu);
     submit(dom, "cpu_affinity", type_instance, &(value_t){.gauge = is_set}, 1);
@@ -1577,7 +1585,7 @@ static int get_block_stats(struct block_device *block_dev) {
 
 #define NM_ADD_STR_ITEMS(_items, _size)                                        \
   do {                                                                         \
-    for (int _i = 0; _i < _size; ++_i) {                                       \
+    for (size_t _i = 0; _i < _size; ++_i) {                                    \
       DEBUG(PLUGIN_NAME                                                        \
             " plugin: Adding notification metadata name=%s value=%s",          \
             _items[_i].name, _items[_i].value);                                \
@@ -1602,7 +1610,7 @@ static int fs_info_notify(virDomainPtr domain, virDomainFSInfoPtr fs_info) {
       {.name = "name", .value = fs_info->name},
       {.name = "fstype", .value = fs_info->fstype}};
 
-  for (int i = 0; i < fs_info->ndevAlias; ++i) {
+  for (size_t i = 0; i < fs_info->ndevAlias; ++i) {
     fs_dev_alias[i].name = "devAlias";
     fs_dev_alias[i].value = fs_info->devAlias[i];
   }
@@ -1817,7 +1825,7 @@ static int get_if_dev_stats(struct interface_device *if_dev) {
   return 0;
 }
 
-static int domain_lifecycle_event_cb(__attribute__((unused)) virConnectPtr conn,
+static int domain_lifecycle_event_cb(__attribute__((unused)) virConnectPtr con_,
                                      virDomainPtr dom, int event, int detail,
                                      __attribute__((unused)) void *opaque) {
   int domain_state = map_domain_event_to_state(event);
@@ -1842,9 +1850,30 @@ static int register_event_impl(void) {
   return 0;
 }
 
+static void virt_notif_thread_set_active(virt_notif_thread_t *thread_data,
+                                         const bool active) {
+  assert(thread_data != NULL);
+  pthread_mutex_lock(&thread_data->active_mutex);
+  thread_data->is_active = active;
+  pthread_mutex_unlock(&thread_data->active_mutex);
+}
+
+static bool virt_notif_thread_is_active(virt_notif_thread_t *thread_data) {
+  bool active = false;
+
+  assert(thread_data != NULL);
+  pthread_mutex_lock(&thread_data->active_mutex);
+  active = thread_data->is_active;
+  pthread_mutex_unlock(&thread_data->active_mutex);
+
+  return active;
+}
+
 /* worker function running default event implementation */
-static void *event_loop_worker(__attribute__((unused)) void *arg) {
-  while (1) {
+static void *event_loop_worker(void *arg) {
+  virt_notif_thread_t *thread_data = (virt_notif_thread_t *)arg;
+
+  while (virt_notif_thread_is_active(thread_data)) {
     if (virEventRunDefaultImpl() < 0) {
       virErrorPtr err = virGetLastError();
       ERROR(PLUGIN_NAME " plugin: failed to run event loop: %s\n",
@@ -1855,19 +1884,44 @@ static void *event_loop_worker(__attribute__((unused)) void *arg) {
   return NULL;
 }
 
+static int virt_notif_thread_init(virt_notif_thread_t *thread_data) {
+  int ret;
+
+  assert(thread_data != NULL);
+  ret = pthread_mutex_init(&thread_data->active_mutex, NULL);
+  if (ret != 0) {
+    ERROR(PLUGIN_NAME ": Failed to initialize mutex, err %u", ret);
+    return ret;
+  }
+
+  /**
+   * '0' and positive integers are meaningful ID's, therefore setting
+   * domain_event_cb_id to '-1'
+   */
+  thread_data->domain_event_cb_id = -1;
+  pthread_mutex_lock(&thread_data->active_mutex);
+  thread_data->is_active = false;
+  pthread_mutex_unlock(&thread_data->active_mutex);
+
+  return 0;
+}
+
 /* register domain event callback and start event loop thread */
-static int start_event_loop(void) {
-  domain_event_cb_id = virConnectDomainEventRegisterAny(
+static int start_event_loop(virt_notif_thread_t *thread_data) {
+  assert(thread_data != NULL);
+  thread_data->domain_event_cb_id = virConnectDomainEventRegisterAny(
       conn, NULL, VIR_DOMAIN_EVENT_ID_LIFECYCLE,
       VIR_DOMAIN_EVENT_CALLBACK(domain_lifecycle_event_cb), NULL, NULL);
-  if (domain_event_cb_id == -1) {
+  if (thread_data->domain_event_cb_id == -1) {
     ERROR(PLUGIN_NAME " plugin: error while callback registering");
     return -1;
   }
 
-  if (pthread_create(&event_loop_tid, NULL, event_loop_worker, NULL)) {
+  virt_notif_thread_set_active(thread_data, 1);
+  if (pthread_create(&thread_data->event_loop_tid, NULL, event_loop_worker,
+                     thread_data)) {
     ERROR(PLUGIN_NAME " plugin: failed event loop thread creation");
-    virConnectDomainEventDeregisterAny(conn, domain_event_cb_id);
+    virConnectDomainEventDeregisterAny(conn, thread_data->domain_event_cb_id);
     return -1;
   }
 
@@ -1875,15 +1929,14 @@ static int start_event_loop(void) {
 }
 
 /* stop event loop thread and deregister callback */
-static void stop_event_loop(void) {
-  if (pthread_cancel(event_loop_tid) != 0)
-    ERROR(PLUGIN_NAME " plugin: cancelling thread %lu failed", event_loop_tid);
-
-  if (pthread_join(event_loop_tid, NULL) != 0)
-    ERROR(PLUGIN_NAME " plugin: stopping thread %lu failed", event_loop_tid);
-
-  if (conn != NULL && domain_event_cb_id != -1)
-    virConnectDomainEventDeregisterAny(conn, domain_event_cb_id);
+static void stop_event_loop(virt_notif_thread_t *thread_data) {
+  /* stopping loop and de-registering event handler*/
+  virt_notif_thread_set_active(thread_data, 0);
+  if (conn != NULL && thread_data->domain_event_cb_id != -1)
+    virConnectDomainEventDeregisterAny(conn, thread_data->domain_event_cb_id);
+
+  if (pthread_join(notif_thread.event_loop_tid, NULL) != 0)
+    ERROR(PLUGIN_NAME " plugin: stopping notification thread failed");
 }
 
 static int persistent_domains_state_notification(void) {
@@ -1971,7 +2024,7 @@ static int lv_read(user_data_t *ud) {
   inst = ud->data;
   state = &inst->read_state;
 
-  _Bool reconnect = conn == NULL ? 1 : 0;
+  bool reconnect = conn == NULL ? true : false;
   /* event implementation must be registered before connection is opened */
   if (inst->id == 0) {
     if (!persistent_notification && reconnect)
@@ -1982,7 +2035,7 @@ static int lv_read(user_data_t *ud) {
       return -1;
 
     if (!persistent_notification && reconnect && conn != NULL)
-      if (start_event_loop() != 0)
+      if (start_event_loop(&notif_thread) != 0)
         return -1;
   }
 
@@ -1994,7 +2047,7 @@ static int lv_read(user_data_t *ud) {
     if (refresh_lists(inst) != 0) {
       if (inst->id == 0) {
         if (!persistent_notification)
-          stop_event_loop();
+          stop_event_loop(&notif_thread);
         lv_disconnect();
       }
       return -1;
@@ -2048,7 +2101,7 @@ static int lv_read(user_data_t *ud) {
       ERROR(PLUGIN_NAME
             " failed to get stats for block device (%s) in domain %s",
             state->block_devices[i].path,
-            virDomainGetName(state->domains[i].ptr));
+            virDomainGetName(state->block_devices[i].dom));
   }
 
   /* Get interface stats for each domain. */
@@ -2111,9 +2164,11 @@ static int lv_init(void) {
 
   DEBUG(PLUGIN_NAME " plugin: starting event loop");
 
-  if (!persistent_notification)
-    if (start_event_loop() != 0)
+  if (!persistent_notification) {
+    virt_notif_thread_init(&notif_thread);
+    if (start_event_loop(&notif_thread) != 0)
       return -1;
+  }
 
   DEBUG(PLUGIN_NAME " plugin: starting %i instances", nr_instances);
 
@@ -2271,6 +2326,8 @@ static int refresh_lists(struct lv_read_instance *inst) {
   for (int i = 0; i < m; ++i)
     if (add_domain(state, domains_inactive[i], 0) < 0) {
       ERROR(PLUGIN_NAME " plugin: malloc failed.");
+      virDomainFree(domains_inactive[i]);
+      domains_inactive[i] = NULL;
       continue;
     }
 #endif
@@ -2298,6 +2355,20 @@ static int refresh_lists(struct lv_read_instance *inst) {
     }
 #endif
 
+    if (add_domain(state, dom, 1) < 0) {
+      /*
+       * When domain is already tracked, then there is
+       * no problem with memory handling (will be freed
+       * with the rest of domains cached data)
+       * But in case of error like this (error occurred
+       * before adding domain to track) we have to take
+       * care it ourselves and call virDomainFree
+       */
+      ERROR(PLUGIN_NAME " plugin: malloc failed.");
+      virDomainFree(dom);
+      goto cont;
+    }
+
     name = virDomainGetName(dom);
     if (name == NULL) {
       VIRT_ERROR(conn, "virDomainGetName");
@@ -2343,11 +2414,6 @@ static int refresh_lists(struct lv_read_instance *inst) {
     if (!lv_instance_include_domain(inst, name, tag))
       goto cont;
 
-    if (add_domain(state, dom, 1) < 0) {
-      ERROR(PLUGIN_NAME " plugin: malloc failed.");
-      goto cont;
-    }
-
     /* Block devices. */
     const char *bd_xmlpath = "/domain/devices/disk/target[@dev]";
     if (blockdevice_format == source)
@@ -2438,11 +2504,10 @@ static int refresh_lists(struct lv_read_instance *inst) {
   }
 
 #ifdef HAVE_LIST_ALL_DOMAINS
-  for (int i = 0; i < n; ++i)
-    virDomainFree(domains[i]);
+  /* NOTE: domains_active and domains_inactive data will be cleared during
+     refresh of all domains (inside lv_clean_read_state function) so we need
+     to free here only allocated arrays */
   sfree(domains);
-  for (int i = 0; i < m; ++i)
-    virDomainFree(domains_inactive[i]);
   sfree(domains_inactive);
 #else
   sfree(domids);
@@ -2469,7 +2534,7 @@ static void free_domains(struct lv_read_state *state) {
 }
 
 static int add_domain(struct lv_read_state *state, virDomainPtr dom,
-                      _Bool active) {
+                      bool active) {
   domain_t *new_ptr;
   int new_size = sizeof(state->domains[0]) * (state->nr_domains + 1);
 
@@ -2545,7 +2610,7 @@ static int add_interface_device(struct lv_read_state *state, virDomainPtr dom,
   struct interface_device *new_ptr;
   int new_size =
       sizeof(state->interface_devices[0]) * (state->nr_interface_devices + 1);
-  char *path_copy, *address_copy, number_string[15];
+  char *path_copy, *address_copy, number_string[21];
 
   if ((path == NULL) || (address == NULL))
     return EINVAL;
@@ -2584,12 +2649,12 @@ static int add_interface_device(struct lv_read_state *state, virDomainPtr dom,
 static int ignore_device_match(ignorelist_t *il, const char *domname,
                                const char *devpath) {
   char *name;
-  int n, r;
+  int r;
 
   if ((domname == NULL) || (devpath == NULL))
     return 0;
 
-  n = strlen(domname) + strlen(devpath) + 2;
+  size_t n = strlen(domname) + strlen(devpath) + 2;
   name = malloc(n);
   if (name == NULL) {
     ERROR(PLUGIN_NAME " plugin: malloc failed.");
@@ -2609,7 +2674,7 @@ static int lv_shutdown(void) {
   DEBUG(PLUGIN_NAME " plugin: stopping event loop");
 
   if (!persistent_notification)
-    stop_event_loop();
+    stop_event_loop(&notif_thread);
 
   lv_disconnect();