virt plugin: Cleanup
[collectd.git] / src / virt.c
index 2899177..c6ac590 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",
@@ -457,12 +464,12 @@ const char *domain_reasons[][DOMAIN_STATE_REASON_MAX_SIZE] = {
   do {                                                                         \
     status = _f(__VA_ARGS__);                                                  \
     if (status != 0)                                                           \
-      ERROR(PLUGIN_NAME ": Failed to get " _name);                             \
+      ERROR(PLUGIN_NAME " plugin: Failed to get " _name);                      \
   } 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;
 
@@ -632,12 +639,6 @@ static time_t last_refresh = (time_t)0;
 
 static int refresh_lists(struct lv_read_instance *inst);
 
-struct lv_info {
-  virDomainInfo di;
-  unsigned long long total_user_cpu_time;
-  unsigned long long total_syst_cpu_time;
-};
-
 struct lv_block_info {
   virDomainBlockStatsStruct bi;
 
@@ -701,59 +702,9 @@ static int get_block_info(struct lv_block_info *binfo,
     virErrorPtr err;                                                           \
     err = (conn) ? virConnGetLastError((conn)) : virGetLastError();            \
     if (err)                                                                   \
-      ERROR("%s: %s", (s), err->message);                                      \
+      ERROR(PLUGIN_NAME " plugin: %s failed: %s", (s), err->message);          \
   } while (0)
 
-static void init_lv_info(struct lv_info *info) {
-  if (info != NULL)
-    memset(info, 0, sizeof(*info));
-}
-
-static int lv_domain_info(virDomainPtr dom, struct lv_info *info) {
-#ifdef HAVE_CPU_STATS
-  virTypedParameterPtr param = NULL;
-  int nparams = 0;
-#endif /* HAVE_CPU_STATS */
-  int ret = virDomainGetInfo(dom, &(info->di));
-  if (ret != 0) {
-    return ret;
-  }
-
-#ifdef HAVE_CPU_STATS
-  nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, 0);
-  if (nparams < 0) {
-    VIRT_ERROR(conn, "getting the CPU params count");
-    return -1;
-  }
-
-  param = calloc(nparams, sizeof(virTypedParameter));
-  if (param == NULL) {
-    ERROR("virt plugin: alloc(%i) for cpu parameters failed.", nparams);
-    return -1;
-  }
-
-  ret = virDomainGetCPUStats(dom, param, nparams, -1, 1, 0); // total stats.
-  if (ret < 0) {
-    virTypedParamsClear(param, nparams);
-    sfree(param);
-    VIRT_ERROR(conn, "getting the disk params values");
-    return -1;
-  }
-
-  for (int i = 0; i < nparams; ++i) {
-    if (!strcmp(param[i].field, "user_time"))
-      info->total_user_cpu_time = param[i].value.ul;
-    else if (!strcmp(param[i].field, "system_time"))
-      info->total_syst_cpu_time = param[i].value.ul;
-  }
-
-  virTypedParamsClear(param, nparams);
-  sfree(param);
-#endif /* HAVE_CPU_STATS */
-
-  return 0;
-}
-
 static void init_value_list(value_list_t *vl, virDomainPtr dom) {
   const char *name;
   char uuid[VIR_UUID_STRING_BUFLEN];
@@ -819,7 +770,7 @@ static int init_notif(notification_t *notif, const virDomainPtr domain,
   value_list_t vl = VALUE_LIST_INIT;
 
   if (!notif) {
-    ERROR(PLUGIN_NAME ": init_notif: NULL pointer");
+    ERROR(PLUGIN_NAME " plugin: init_notif: NULL pointer");
     return -1;
   }
 
@@ -885,14 +836,6 @@ static void submit_derive2(const char *type, derive_t v0, derive_t v1,
   submit(dom, type, devname, values, STATIC_ARRAY_SIZE(values));
 } /* void submit_derive2 */
 
-static void pcpu_submit(virDomainPtr dom, struct lv_info *info) {
-#ifdef HAVE_CPU_STATS
-  if (extra_stats & ex_stats_pcpu)
-    submit_derive2("ps_cputime", info->total_user_cpu_time,
-                   info->total_syst_cpu_time, dom, NULL);
-#endif /* HAVE_CPU_STATS */
-}
-
 static double cpu_ns_to_percent(unsigned int node_cpus,
                                 unsigned long long cpu_time_old,
                                 unsigned long long cpu_time_new) {
@@ -906,7 +849,7 @@ static double cpu_ns_to_percent(unsigned int node_cpus,
               (time_diff_sec * node_cpus * NANOSEC_IN_SEC);
   }
 
-  DEBUG(PLUGIN_NAME ": node_cpus=%u cpu_time_old=%" PRIu64
+  DEBUG(PLUGIN_NAME " plugin: node_cpus=%u cpu_time_old=%" PRIu64
                     " cpu_time_new=%" PRIu64 "cpu_time_diff=%" PRIu64
                     " time_diff_sec=%f percent=%f",
         node_cpus, (uint64_t)cpu_time_old, (uint64_t)cpu_time_new,
@@ -1001,7 +944,8 @@ static unsigned int parse_ex_stats_flags(char **exstats, int numexstats) {
       }
 
       if (ex_stats_table[j + 1].name == NULL) {
-        ERROR(PLUGIN_NAME ": Unmatched ExtraStats option: %s", exstats[i]);
+        ERROR(PLUGIN_NAME " plugin: Unmatched ExtraStats option: %s",
+              exstats[i]);
       }
     }
   }
@@ -1009,16 +953,17 @@ 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))) {
-    ERROR(PLUGIN_NAME ": Array index out of bounds: state=%d", state);
+  if ((state < 0) || ((size_t)state >= STATIC_ARRAY_SIZE(domain_states))) {
+    ERROR(PLUGIN_NAME " plugin: Array index out of bounds: state=%d", state);
     return;
   }
 
   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]))) {
-    ERROR(PLUGIN_NAME ": Array index out of bounds: reason=%d", reason);
+  if ((reason < 0) ||
+      ((size_t)reason >= STATIC_ARRAY_SIZE(domain_reasons[0]))) {
+    ERROR(PLUGIN_NAME " plugin: Array index out of bounds: reason=%d", reason);
     return;
   }
 
@@ -1027,8 +972,8 @@ static void domain_state_submit_notif(virDomainPtr dom, int state, int reason) {
    * have different number of reasons. We need to check if reason was
    * successfully parsed */
   if (!reason_str) {
-    ERROR(PLUGIN_NAME ": Invalid reason (%d) for domain state: %s", reason,
-          state_str);
+    ERROR(PLUGIN_NAME " plugin: Invalid reason (%d) for domain state: %s",
+          reason, state_str);
     return;
   }
 #else
@@ -1057,7 +1002,7 @@ static void domain_state_submit_notif(virDomainPtr dom, int state, int reason) {
     severity = NOTIF_FAILURE;
     break;
   default:
-    ERROR(PLUGIN_NAME ": Unrecognized domain state (%d)", state);
+    ERROR(PLUGIN_NAME " plugin: Unrecognized domain state (%d)", state);
     return;
   }
   submit_notif(dom, severity, msg, "domain_state", NULL);
@@ -1116,7 +1061,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) {
@@ -1139,17 +1084,14 @@ static int lv_config(const char *key, const char *value) {
   }
 
   if (strcasecmp(key, "HostnameFormat") == 0) {
-    char *value_copy;
-    char *fields[HF_MAX_FIELDS];
-    int n;
-
-    value_copy = strdup(value);
+    char *value_copy = strdup(value);
     if (value_copy == NULL) {
       ERROR(PLUGIN_NAME " plugin: strdup failed.");
       return -1;
     }
 
-    n = strsplit(value_copy, fields, HF_MAX_FIELDS);
+    char *fields[HF_MAX_FIELDS];
+    int n = strsplit(value_copy, fields, HF_MAX_FIELDS);
     if (n < 1) {
       sfree(value_copy);
       ERROR(PLUGIN_NAME " plugin: HostnameFormat: no fields");
@@ -1179,17 +1121,14 @@ static int lv_config(const char *key, const char *value) {
   }
 
   if (strcasecmp(key, "PluginInstanceFormat") == 0) {
-    char *value_copy;
-    char *fields[PLGINST_MAX_FIELDS];
-    int n;
-
-    value_copy = strdup(value);
+    char *value_copy = strdup(value);
     if (value_copy == NULL) {
       ERROR(PLUGIN_NAME " plugin: strdup failed.");
       return -1;
     }
 
-    n = strsplit(value_copy, fields, PLGINST_MAX_FIELDS);
+    char *fields[PLGINST_MAX_FIELDS];
+    int n = strsplit(value_copy, fields, PLGINST_MAX_FIELDS);
     if (n < 1) {
       sfree(value_copy);
       ERROR(PLUGIN_NAME " plugin: PluginInstanceFormat: no fields");
@@ -1305,7 +1244,7 @@ static int lv_connect(void) {
     }
     int status = virNodeGetInfo(conn, &nodeinfo);
     if (status != 0) {
-      ERROR(PLUGIN_NAME ": virNodeGetInfo failed");
+      ERROR(PLUGIN_NAME " plugin: virNodeGetInfo failed");
       return -1;
     }
   }
@@ -1390,7 +1329,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);
@@ -1403,13 +1342,13 @@ static int get_vcpu_stats(virDomainPtr domain, unsigned short nr_virt_cpu) {
 
   virVcpuInfoPtr vinfo = calloc(nr_virt_cpu, sizeof(vinfo[0]));
   if (vinfo == NULL) {
-    ERROR(PLUGIN_NAME " plugin: malloc failed.");
+    ERROR(PLUGIN_NAME " plugin: calloc failed.");
     return -1;
   }
 
   unsigned char *cpumaps = calloc(nr_virt_cpu, cpu_map_len);
   if (cpumaps == NULL) {
-    ERROR(PLUGIN_NAME " plugin: malloc failed.");
+    ERROR(PLUGIN_NAME " plugin: calloc failed.");
     sfree(vinfo);
     return -1;
   }
@@ -1435,6 +1374,49 @@ static int get_vcpu_stats(virDomainPtr domain, unsigned short nr_virt_cpu) {
   return 0;
 }
 
+#ifdef HAVE_CPU_STATS
+static int get_pcpu_stats(virDomainPtr dom) {
+  int nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, 0);
+  if (nparams < 0) {
+    VIRT_ERROR(conn, "getting the CPU params count");
+    return -1;
+  }
+
+  virTypedParameterPtr param = calloc(nparams, sizeof(virTypedParameter));
+  if (param == NULL) {
+    ERROR(PLUGIN_NAME " plugin: alloc(%i) for cpu parameters failed.", nparams);
+    return -1;
+  }
+
+  int ret = virDomainGetCPUStats(dom, param, nparams, -1, 1, 0); // total stats.
+  if (ret < 0) {
+    virTypedParamsClear(param, nparams);
+    sfree(param);
+    VIRT_ERROR(conn, "getting the CPU params values");
+    return -1;
+  }
+
+  unsigned long long total_user_cpu_time = 0;
+  unsigned long long total_syst_cpu_time = 0;
+
+  for (int i = 0; i < nparams; ++i) {
+    if (!strcmp(param[i].field, "user_time"))
+      total_user_cpu_time = param[i].value.ul;
+    else if (!strcmp(param[i].field, "system_time"))
+      total_syst_cpu_time = param[i].value.ul;
+  }
+
+  if (total_user_cpu_time > 0 || total_syst_cpu_time > 0)
+    submit_derive2("ps_cputime", total_user_cpu_time, total_syst_cpu_time, dom,
+                   NULL);
+
+  virTypedParamsClear(param, nparams);
+  sfree(param);
+
+  return 0;
+}
+#endif /* HAVE_CPU_STATS */
+
 #ifdef HAVE_DOM_REASON
 
 static void domain_state_submit(virDomainPtr dom, int state, int reason) {
@@ -1577,7 +1559,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 +1584,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];
   }
@@ -1708,15 +1690,13 @@ static int get_job_stats(virDomainPtr domain) {
 #endif /* HAVE_JOB_STATS */
 
 static int get_domain_metrics(domain_t *domain) {
-  struct lv_info info;
-
   if (!domain || !domain->ptr) {
-    ERROR(PLUGIN_NAME ": get_domain_metrics: NULL pointer");
+    ERROR(PLUGIN_NAME " plugin: get_domain_metrics: NULL pointer");
     return -1;
   }
 
-  init_lv_info(&info);
-  int status = lv_domain_info(domain->ptr, &info);
+  virDomainInfo info;
+  int status = virDomainGetInfo(domain->ptr, &info);
   if (status != 0) {
     ERROR(PLUGIN_NAME " plugin: virDomainGetInfo failed with status %i.",
           status);
@@ -1734,15 +1714,19 @@ static int get_domain_metrics(domain_t *domain) {
   }
 
   /* Gather remaining stats only for running domains */
-  if (info.di.state != VIR_DOMAIN_RUNNING)
+  if (info.state != VIR_DOMAIN_RUNNING)
     return 0;
 
-  pcpu_submit(domain->ptr, &info);
-  cpu_submit(domain, info.di.cpuTime);
+#ifdef HAVE_CPU_STATS
+  if (extra_stats & ex_stats_pcpu)
+    get_pcpu_stats(domain->ptr);
+#endif
 
-  memory_submit(domain->ptr, (gauge_t)info.di.memory * 1024);
+  cpu_submit(domain, info.cpuTime);
 
-  GET_STATS(get_vcpu_stats, "vcpu stats", domain->ptr, info.di.nrVirtCpu);
+  memory_submit(domain->ptr, (gauge_t)info.memory * 1024);
+
+  GET_STATS(get_vcpu_stats, "vcpu stats", domain->ptr, info.nrVirtCpu);
   GET_STATS(get_memory_stats, "memory stats", domain->ptr);
 
 #ifdef HAVE_PERF_STATS
@@ -1767,7 +1751,7 @@ static int get_domain_metrics(domain_t *domain) {
 #endif
 
   /* Update cached virDomainInfo. It has to be done after cpu_submit */
-  memcpy(&domain->info, &info.di, sizeof(domain->info));
+  memcpy(&domain->info, &info, sizeof(domain->info));
 
   return 0;
 }
@@ -1817,7 +1801,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 +1826,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 +1860,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 " plugin: 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 +1905,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) {
@@ -1919,9 +1948,9 @@ static int persistent_domains_state_notification(void) {
   if (n > 0) {
     int *domids;
     /* Get list of domains. */
-    domids = malloc(sizeof(*domids) * n);
+    domids = calloc(n, sizeof(*domids));
     if (domids == NULL) {
-      ERROR(PLUGIN_NAME " plugin: malloc failed.");
+      ERROR(PLUGIN_NAME " plugin: calloc failed.");
       return -1;
     }
     n = virConnectListDomains(conn, domids, n);
@@ -1941,14 +1970,15 @@ static int persistent_domains_state_notification(void) {
         continue;
       }
       status = virDomainGetInfo(dom, &info);
-      if (status != 0) {
+      if (status == 0)
+        /* virDomainGetState is not available. Submit 0, which corresponds to
+         * unknown reason. */
+        domain_state_submit_notif(dom, info.state, 0);
+      else
         ERROR(PLUGIN_NAME " plugin: virDomainGetInfo failed with status %i.",
               status);
-        continue;
-      }
-      /* virDomainGetState is not available. Submit 0, which corresponds to
-       * unknown reason. */
-      domain_state_submit_notif(dom, info.state, 0);
+
+      virDomainFree(dom);
     }
     sfree(domids);
   }
@@ -1970,7 +2000,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)
@@ -1981,7 +2011,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;
   }
 
@@ -1990,17 +2020,10 @@ static int lv_read(user_data_t *ud) {
   /* Need to refresh domain or device lists? */
   if ((last_refresh == (time_t)0) ||
       ((interval > 0) && ((last_refresh + interval) <= t))) {
-    if (inst->id == 0 && persistent_notification) {
-      int status = persistent_domains_state_notification();
-      if (status != 0)
-        DEBUG(PLUGIN_NAME " plugin: persistent_domains_state_notifications "
-                          "returned with status %i",
-              status);
-    }
     if (refresh_lists(inst) != 0) {
       if (inst->id == 0) {
         if (!persistent_notification)
-          stop_event_loop();
+          stop_event_loop(&notif_thread);
         lv_disconnect();
       }
       return -1;
@@ -2008,6 +2031,15 @@ static int lv_read(user_data_t *ud) {
     last_refresh = t;
   }
 
+  /* persistent domains state notifications are handled by instance 0 */
+  if (inst->id == 0 && persistent_notification) {
+    int status = persistent_domains_state_notification();
+    if (status != 0)
+      DEBUG(PLUGIN_NAME " plugin: persistent_domains_state_notifications "
+                        "returned with status %i",
+            status);
+  }
+
 #if COLLECT_DEBUG
   for (int i = 0; i < state->nr_domains; ++i)
     DEBUG(PLUGIN_NAME " plugin: domain %s",
@@ -2034,7 +2066,7 @@ static int lv_read(user_data_t *ud) {
 #endif
 
     if (status != 0)
-      ERROR(PLUGIN_NAME " failed to get metrics for domain=%s",
+      ERROR(PLUGIN_NAME " plugin: failed to get metrics for domain=%s",
             virDomainGetName(dom->ptr));
   }
 
@@ -2043,19 +2075,20 @@ static int lv_read(user_data_t *ud) {
     int status = get_block_stats(&state->block_devices[i]);
     if (status != 0)
       ERROR(PLUGIN_NAME
-            " failed to get stats for block device (%s) in domain %s",
+            " plugin: 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. */
   for (int i = 0; i < state->nr_interface_devices; ++i) {
     int status = get_if_dev_stats(&state->interface_devices[i]);
     if (status != 0)
-      ERROR(PLUGIN_NAME
-            " failed to get interface stats for device (%s) in domain %s",
-            state->interface_devices[i].path,
-            virDomainGetName(state->interface_devices[i].dom));
+      ERROR(
+          PLUGIN_NAME
+          " plugin: failed to get interface stats for device (%s) in domain %s",
+          state->interface_devices[i].path,
+          virDomainGetName(state->interface_devices[i].dom));
   }
 
   return 0;
@@ -2108,9 +2141,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);
 
@@ -2243,9 +2278,9 @@ static int refresh_lists(struct lv_read_instance *inst) {
   int *domids;
 
   /* Get list of domains. */
-  domids = malloc(sizeof(*domids) * n);
+  domids = calloc(n, sizeof(*domids));
   if (domids == NULL) {
-    ERROR(PLUGIN_NAME " plugin: malloc failed.");
+    ERROR(PLUGIN_NAME " plugin: calloc failed.");
     return -1;
   }
 
@@ -2268,6 +2303,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
@@ -2295,6 +2332,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");
@@ -2340,11 +2391,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)
@@ -2435,11 +2481,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);
@@ -2466,15 +2511,11 @@ static void free_domains(struct lv_read_state *state) {
 }
 
 static int add_domain(struct lv_read_state *state, virDomainPtr dom,
-                      _Bool active) {
-  domain_t *new_ptr;
-  int new_size = sizeof(state->domains[0]) * (state->nr_domains + 1);
+                      bool active) {
 
-  if (state->domains)
-    new_ptr = realloc(state->domains, new_size);
-  else
-    new_ptr = malloc(new_size);
+  int new_size = sizeof(state->domains[0]) * (state->nr_domains + 1);
 
+  domain_t *new_ptr = realloc(state->domains, new_size);
   if (new_ptr == NULL)
     return -1;
 
@@ -2499,20 +2540,15 @@ static void free_block_devices(struct lv_read_state *state) {
 
 static int add_block_device(struct lv_read_state *state, virDomainPtr dom,
                             const char *path) {
-  struct block_device *new_ptr;
-  int new_size =
-      sizeof(state->block_devices[0]) * (state->nr_block_devices + 1);
-  char *path_copy;
 
-  path_copy = strdup(path);
+  char *path_copy = strdup(path);
   if (!path_copy)
     return -1;
 
-  if (state->block_devices)
-    new_ptr = realloc(state->block_devices, new_size);
-  else
-    new_ptr = malloc(new_size);
+  int new_size =
+      sizeof(state->block_devices[0]) * (state->nr_block_devices + 1);
 
+  struct block_device *new_ptr = realloc(state->block_devices, new_size);
   if (new_ptr == NULL) {
     sfree(path_copy);
     return -1;
@@ -2539,54 +2575,58 @@ static void free_interface_devices(struct lv_read_state *state) {
 static int add_interface_device(struct lv_read_state *state, virDomainPtr dom,
                                 const char *path, const char *address,
                                 unsigned int number) {
-  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];
 
   if ((path == NULL) || (address == NULL))
     return EINVAL;
 
-  path_copy = strdup(path);
+  char *path_copy = strdup(path);
   if (!path_copy)
     return -1;
 
-  address_copy = strdup(address);
+  char *address_copy = strdup(address);
   if (!address_copy) {
     sfree(path_copy);
     return -1;
   }
 
+  char number_string[21];
   snprintf(number_string, sizeof(number_string), "interface-%u", number);
+  char *number_copy = strdup(number_string);
+  if (!number_copy) {
+    sfree(path_copy);
+    sfree(address_copy);
+    return -1;
+  }
 
-  if (state->interface_devices)
-    new_ptr = realloc(state->interface_devices, new_size);
-  else
-    new_ptr = malloc(new_size);
+  int new_size =
+      sizeof(state->interface_devices[0]) * (state->nr_interface_devices + 1);
 
+  struct interface_device *new_ptr =
+      realloc(state->interface_devices, new_size);
   if (new_ptr == NULL) {
     sfree(path_copy);
     sfree(address_copy);
+    sfree(number_copy);
     return -1;
   }
+
   state->interface_devices = new_ptr;
   state->interface_devices[state->nr_interface_devices].dom = dom;
   state->interface_devices[state->nr_interface_devices].path = path_copy;
   state->interface_devices[state->nr_interface_devices].address = address_copy;
-  state->interface_devices[state->nr_interface_devices].number =
-      strdup(number_string);
+  state->interface_devices[state->nr_interface_devices].number = number_copy;
   return state->nr_interface_devices++;
 }
 
 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.");
@@ -2606,7 +2646,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();