X-Git-Url: https://git.octo.it/?a=blobdiff_plain;f=src%2Fvirt.c;h=4ec76957ff11b852e228c90048b065565a8c46d9;hb=a073b83ade22d475828889ed8c111d133b6f63f1;hp=5b4dbb8eb2caa9d9118261b010e89f38be4a134a;hpb=a64070832f14fe95b93c003348125126f064fa51;p=collectd.git

diff --git a/src/virt.c b/src/virt.c
index 5b4dbb8e..4ec76957 100644
--- a/src/virt.c
+++ b/src/virt.c
@@ -926,6 +926,10 @@ static void memory_stats_submit(gauge_t value, virDomainPtr dom,
     return;
   }
 
+  /* Skip 'last_update' reporting as that is not memory but timestamp */
+  if (tag_index == 9)
+    return;
+
   submit(dom, "memory", tags[tag_index], &(value_t){.gauge = value}, 1);
 }
 
@@ -1155,6 +1159,11 @@ static int lv_init_ignorelists() {
 /* Validates config option that may take multiple strings arguments.
  * Returns 0 on success, -1 otherwise */
 static int check_config_multiple_string_entry(const oconfig_item_t *ci) {
+  if (ci == NULL) {
+    ERROR(PLUGIN_NAME " plugin: ci oconfig_item can't be NULL");
+    return -1;
+  }
+
   if (ci->values_num < 1) {
     ERROR(PLUGIN_NAME
           " plugin: the '%s' option requires at least one string argument",
@@ -1184,25 +1193,19 @@ static int lv_config(oconfig_item_t *ci) {
     oconfig_item_t *c = ci->children + i;
 
     if (strcasecmp(c->key, "Connection") == 0) {
-      if (cf_util_get_string(c, &conn_string) != 0 || conn_string == NULL) {
-        ERROR(PLUGIN_NAME " plugin: Could not get 'Connection' parameter");
+      if (cf_util_get_string(c, &conn_string) != 0 || conn_string == NULL)
         return -1;
-      }
 
       continue;
     } else if (strcasecmp(c->key, "RefreshInterval") == 0) {
-      if (cf_util_get_int(c, &interval) != 0) {
-        ERROR(PLUGIN_NAME " plugin: Could not get 'RefreshInterval' parameter");
+      if (cf_util_get_int(c, &interval) != 0)
         return -1;
-      }
 
       continue;
     } else if (strcasecmp(c->key, "Domain") == 0) {
       char *domain_name = NULL;
-      if (cf_util_get_string(c, &domain_name) != 0 || domain_name == NULL) {
-        ERROR(PLUGIN_NAME " plugin: Could not get 'Domain' parameter");
+      if (cf_util_get_string(c, &domain_name) != 0)
         return -1;
-      }
 
       if (ignorelist_add(il_domains, domain_name)) {
         ERROR(PLUGIN_NAME " plugin: Adding '%s' to domain-ignorelist failed",
@@ -1215,10 +1218,8 @@ static int lv_config(oconfig_item_t *ci) {
       continue;
     } else if (strcasecmp(c->key, "BlockDevice") == 0) {
       char *device_name = NULL;
-      if (cf_util_get_string(c, &device_name) != 0 || device_name == NULL) {
-        ERROR(PLUGIN_NAME " plugin: Could not get 'BlockDevice' parameter");
+      if (cf_util_get_string(c, &device_name) != 0)
         return -1;
-      }
 
       if (ignorelist_add(il_block_devices, device_name) != 0) {
         ERROR(PLUGIN_NAME
@@ -1232,11 +1233,8 @@ static int lv_config(oconfig_item_t *ci) {
       continue;
     } else if (strcasecmp(c->key, "BlockDeviceFormat") == 0) {
       char *device_format = NULL;
-      if (cf_util_get_string(c, &device_format) != 0 || device_format == NULL) {
-        ERROR(PLUGIN_NAME
-              " plugin: Could not get 'BlockDeviceFormat' parameter");
+      if (cf_util_get_string(c, &device_format) != 0)
         return -1;
-      }
 
       if (strcasecmp(device_format, "target") == 0)
         blockdevice_format = target;
@@ -1252,20 +1250,14 @@ static int lv_config(oconfig_item_t *ci) {
       sfree(device_format);
       continue;
     } else if (strcasecmp(c->key, "BlockDeviceFormatBasename") == 0) {
-      if (cf_util_get_boolean(c, &blockdevice_format_basename) != 0) {
-        ERROR(PLUGIN_NAME
-              " plugin: Could not get 'BlockDeviceFormatBasename' parameter");
+      if (cf_util_get_boolean(c, &blockdevice_format_basename) != 0)
         return -1;
-      }
 
       continue;
     } else if (strcasecmp(c->key, "InterfaceDevice") == 0) {
       char *interface_name = NULL;
-      if (cf_util_get_string(c, &interface_name) != 0 ||
-          interface_name == NULL) {
-        ERROR(PLUGIN_NAME " plugin: Could not get 'InterfaceDevice' parameter");
+      if (cf_util_get_string(c, &interface_name) != 0)
         return -1;
-      }
 
       if (ignorelist_add(il_interface_devices, interface_name)) {
         ERROR(PLUGIN_NAME " plugin: Adding '%s' to interface-ignorelist failed",
@@ -1278,10 +1270,8 @@ static int lv_config(oconfig_item_t *ci) {
       continue;
     } else if (strcasecmp(c->key, "IgnoreSelected") == 0) {
       bool ignore_selected = false;
-      if (cf_util_get_boolean(c, &ignore_selected) != 0) {
-        ERROR(PLUGIN_NAME " plugin: Could not get 'IgnoreSelected' parameter");
+      if (cf_util_get_boolean(c, &ignore_selected) != 0)
         return -1;
-      }
 
       if (ignore_selected) {
         ignorelist_set_invert(il_domains, 0);
@@ -1295,25 +1285,21 @@ static int lv_config(oconfig_item_t *ci) {
 
       continue;
     } else if (strcasecmp(c->key, "HostnameMetadataNS") == 0) {
-      if (cf_util_get_string(c, &hm_ns) != 0) {
-        ERROR(PLUGIN_NAME
-              " plugin: Could not get 'HostnameMetadataNS' parameter");
+      if (cf_util_get_string(c, &hm_ns) != 0)
         return -1;
-      }
 
       continue;
     } else if (strcasecmp(c->key, "HostnameMetadataXPath") == 0) {
-      if (cf_util_get_string(c, &hm_xpath) != 0) {
-        ERROR(PLUGIN_NAME
-              " plugin: Could not get 'HostnameMetadataXPath' parameter");
+      if (cf_util_get_string(c, &hm_xpath) != 0)
         return -1;
-      }
 
       continue;
     } else if (strcasecmp(c->key, "HostnameFormat") == 0) {
       /* this option can take multiple strings arguments in one config line*/
-      if (check_config_multiple_string_entry(c) != 0)
+      if (check_config_multiple_string_entry(c) != 0) {
+        ERROR(PLUGIN_NAME " plugin: Could not get 'HostnameFormat' parameter");
         return -1;
+      }
 
       const int params_num = c->values_num;
       for (int i = 0; i < params_num; ++i) {
@@ -1339,8 +1325,11 @@ static int lv_config(oconfig_item_t *ci) {
       continue;
     } else if (strcasecmp(c->key, "PluginInstanceFormat") == 0) {
       /* this option can handle list of string parameters in one line*/
-      if (check_config_multiple_string_entry(c) != 0)
+      if (check_config_multiple_string_entry(c) != 0) {
+        ERROR(PLUGIN_NAME
+              " plugin: Could not get 'PluginInstanceFormat' parameter");
         return -1;
+      }
 
       const int params_num = c->values_num;
       for (int i = 0; i < params_num; ++i) {
@@ -1368,10 +1357,8 @@ static int lv_config(oconfig_item_t *ci) {
       continue;
     } else if (strcasecmp(c->key, "InterfaceFormat") == 0) {
       char *format = NULL;
-      if (cf_util_get_string(c, &format) != 0 || format == NULL) {
-        ERROR(PLUGIN_NAME " plugin: could not get 'InterfaceFormat' parameter");
+      if (cf_util_get_string(c, &format) != 0)
         return -1;
-      }
 
       if (strcasecmp(format, "name") == 0)
         interface_format = if_name;
@@ -1388,10 +1375,8 @@ static int lv_config(oconfig_item_t *ci) {
       sfree(format);
       continue;
     } else if (strcasecmp(c->key, "Instances") == 0) {
-      if (cf_util_get_int(c, &nr_instances) != 0) {
-        ERROR(PLUGIN_NAME " plugin: could not get 'Instances' parameter");
+      if (cf_util_get_int(c, &nr_instances) != 0)
         return -1;
-      }
 
       if (nr_instances <= 0) {
         ERROR(PLUGIN_NAME " plugin: Instances <= 0 makes no sense.");
@@ -1409,10 +1394,8 @@ static int lv_config(oconfig_item_t *ci) {
     } else if (strcasecmp(c->key, "ExtraStats") == 0) {
       char *ex_str = NULL;
 
-      if (cf_util_get_string(c, &ex_str) != 0 || ex_str == NULL) {
-        ERROR(PLUGIN_NAME " plugin: could not get 'ExtraStats' parameter");
+      if (cf_util_get_string(c, &ex_str) != 0)
         return -1;
-      }
 
       char *exstats[EX_STATS_MAX_FIELDS];
       int numexstats = strsplit(ex_str, exstats, STATIC_ARRAY_SIZE(exstats));
@@ -1433,30 +1416,21 @@ static int lv_config(oconfig_item_t *ci) {
       }
 #endif
 
-      /* ExtraStats parsed successfully*/
+      /* ExtraStats parsed successfully */
       continue;
     } else if (strcasecmp(c->key, "PersistentNotification") == 0) {
-      if (cf_util_get_boolean(c, &persistent_notification) != 0) {
-        ERROR(PLUGIN_NAME
-              " plugin: could not get 'PersistentNotification' parameter");
+      if (cf_util_get_boolean(c, &persistent_notification) != 0)
         return -1;
-      }
 
       continue;
     } else if (strcasecmp(c->key, "ReportBlockDevices") == 0) {
-      if (cf_util_get_boolean(c, &report_block_devices) != 0) {
-        ERROR(PLUGIN_NAME
-              " plugin: could not get 'ReportBlockDevices' parameter");
+      if (cf_util_get_boolean(c, &report_block_devices) != 0)
         return -1;
-      }
 
       continue;
     } else if (strcasecmp(c->key, "ReportNetworkInterfaces") == 0) {
-      if (cf_util_get_boolean(c, &report_network_interfaces) != 0) {
-        ERROR(PLUGIN_NAME
-              " plugin: could not get 'ReportNetworkInterfaces' parameter");
+      if (cf_util_get_boolean(c, &report_network_interfaces) != 0)
         return -1;
-      }
 
       continue;
     } else {
@@ -2153,11 +2127,15 @@ static int start_event_loop(virt_notif_thread_t *thread_data) {
     return -1;
   }
 
+  DEBUG(PLUGIN_NAME " plugin: starting event loop");
+
   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");
+    virt_notif_thread_set_active(thread_data, 0);
     virConnectDomainEventDeregisterAny(conn, thread_data->domain_event_cb_id);
+    thread_data->domain_event_cb_id = -1;
     return -1;
   }
 
@@ -2166,13 +2144,21 @@ static int start_event_loop(virt_notif_thread_t *thread_data) {
 
 /* stop event loop thread and deregister callback */
 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");
+  DEBUG(PLUGIN_NAME " plugin: stopping event loop");
+
+  /* Stopping loop */
+  if (virt_notif_thread_is_active(thread_data)) {
+    virt_notif_thread_set_active(thread_data, 0);
+    if (pthread_join(notif_thread.event_loop_tid, NULL) != 0)
+      ERROR(PLUGIN_NAME " plugin: stopping notification thread failed");
+  }
+
+  /* ... and de-registering event handler */
+  if (conn != NULL && thread_data->domain_event_cb_id != -1) {
+    virConnectDomainEventDeregisterAny(conn, thread_data->domain_event_cb_id);
+    thread_data->domain_event_cb_id = -1;
+  }
 }
 
 static int persistent_domains_state_notification(void) {
@@ -2403,8 +2389,6 @@ static int lv_init(void) {
   if (lv_connect() != 0)
     return -1;
 
-  DEBUG(PLUGIN_NAME " plugin: starting event loop");
-
   if (!persistent_notification) {
     virt_notif_thread_init(&notif_thread);
     if (start_event_loop(&notif_thread) != 0)
@@ -2664,6 +2648,24 @@ static void lv_add_network_interfaces(struct lv_read_state *state,
   xmlXPathFreeObject(xpath_obj);
 }
 
+static bool is_domain_ignored(virDomainPtr dom) {
+  const char *domname = virDomainGetName(dom);
+
+  if (domname == NULL) {
+    VIRT_ERROR(conn, "virDomainGetName failed, ignoring domain");
+    return true;
+  }
+
+  if (ignorelist_match(il_domains, domname) != 0) {
+    DEBUG(PLUGIN_NAME
+          " plugin: ignoring domain '%s' because of ignorelist option",
+          domname);
+    return true;
+  }
+
+  return false;
+}
+
 static int refresh_lists(struct lv_read_instance *inst) {
   struct lv_read_state *state = &inst->read_state;
   int n;
@@ -2713,8 +2715,9 @@ static int refresh_lists(struct lv_read_instance *inst) {
 
 #ifdef HAVE_LIST_ALL_DOMAINS
   for (int i = 0; i < m; ++i)
-    if (add_domain(state, domains_inactive[i], 0) < 0) {
-      ERROR(PLUGIN_NAME " plugin: malloc failed.");
+    if (is_domain_ignored(domains_inactive[i]) ||
+        add_domain(state, domains_inactive[i], 0) < 0) {
+      /* domain ignored or failed during adding to domains list*/
       virDomainFree(domains_inactive[i]);
       domains_inactive[i] = NULL;
       continue;
@@ -2735,8 +2738,10 @@ static int refresh_lists(struct lv_read_instance *inst) {
     }
 #endif
 
-    if (add_domain(state, dom, 1) < 0) {
+    if (is_domain_ignored(dom) || add_domain(state, dom, 1) < 0) {
       /*
+       * domain ignored or failed during adding to domains list
+       *
        * When domain is already tracked, then there is
        * no problem with memory handling (will be freed
        * with the rest of domains cached data)
@@ -2744,7 +2749,6 @@ static int refresh_lists(struct lv_read_instance *inst) {
        * before adding domain to track) we have to take
        * care it ourselves and call virDomainFree
        */
-      ERROR(PLUGIN_NAME " plugin: malloc failed.");
       virDomainFree(dom);
       continue;
     }
@@ -2768,9 +2772,6 @@ static int refresh_lists(struct lv_read_instance *inst) {
       continue;
     }
 
-    if (ignorelist_match(il_domains, domname) != 0)
-      continue;
-
     /* Get a list of devices for this domain. */
     xmlDocPtr xml_doc = NULL;
     xmlXPathContextPtr xpath_ctx = NULL;
@@ -2847,12 +2848,13 @@ static void free_domains(struct lv_read_state *state) {
 
 static int add_domain(struct lv_read_state *state, virDomainPtr dom,
                       bool active) {
-
   int new_size = sizeof(state->domains[0]) * (state->nr_domains + 1);
 
   domain_t *new_ptr = realloc(state->domains, new_size);
-  if (new_ptr == NULL)
+  if (new_ptr == NULL) {
+    ERROR(PLUGIN_NAME " plugin: realloc failed in add_domain()");
     return -1;
+  }
 
   state->domains = new_ptr;
   state->domains[state->nr_domains].ptr = dom;
@@ -2976,8 +2978,6 @@ static int lv_shutdown(void) {
     lv_fini_instance(i);
   }
 
-  DEBUG(PLUGIN_NAME " plugin: stopping event loop");
-
   if (!persistent_notification)
     stop_event_loop(&notif_thread);