virt plugin: Add physical cpu reporting

Add a metric to report physical cpu consumed by the hypervisor,
split per user/system time. This is provided by libvirt since version
0.9.11, and used by oVirt.

We also extend the newly-added 'ExtraStats' option to allow
users to toggle on/off the new metric. Default is off.

Signed-off-by: Francesco Romani <fromani@redhat.com>

diff --git a/src/collectd.conf.in b/src/collectd.conf.in
index 982cba9..e4ab111 100644 (file)
--- a/src/collectd.conf.in
+++ b/src/collectd.conf.in
@@ -1459,7 +1459,7 @@
 #      InterfaceFormat name
 #      PluginInstanceFormat name
 #      Instances 1
-#      ExtraStats "disk"
+#      ExtraStats "disk pcpu"
 #</Plugin>
 
 #<Plugin vmem>

diff --git a/src/collectd.conf.pod b/src/collectd.conf.pod
index 9b32a78..e1c7567 100644 (file)
--- a/src/collectd.conf.pod
+++ b/src/collectd.conf.pod
@@ -8085,6 +8085,7 @@ Virtual Machines. The argument is a space-separated list of selectors.
 Currently supported selectors are:
 B<disk> report extra statistics like number of flush operations and total
 service time for read, write and flush operations.
+B<pcpu> report the physical user/system cpu time consumed by the hypervisor, per-vm.
 
 =back
 

diff --git a/src/virt.c b/src/virt.c
index 5a4604c..f2dccec 100644 (file)
--- a/src/virt.c
+++ b/src/virt.c
@@ -38,11 +38,17 @@
 #define PLUGIN_NAME "virt"
 
 #ifdef LIBVIR_CHECK_VERSION
+
 #if LIBVIR_CHECK_VERSION(0, 9, 5)
 #define HAVE_BLOCK_STATS_FLAGS 1
 #endif
+
+#if LIBVIR_CHECK_VERSION(0, 9, 11)
+#define HAVE_CPU_STATS 1
 #endif
 
+#endif /* LIBVIR_CHECK_VERSION */
+
 static const char *config_keys[] = {"Connection",
 
                                     "RefreshInterval",
@@ -167,9 +173,16 @@ enum if_field { if_address, if_name, if_number };
 
 /* ExtraStats */
 #define EX_STATS_MAX_FIELDS 8
-#define EX_STATS_DISK "disk"
-enum ex_stats { ex_stats_none = 0, ex_stats_disk = 1 };
-static enum ex_stats extra_stats = ex_stats_none;
+enum ex_stats { ex_stats_none = 0, ex_stats_disk = 1, ex_stats_pcpu = 2 };
+static unsigned int extra_stats = ex_stats_none;
+
+struct ex_stats_item {
+  const char *name;
+  enum ex_stats flag;
+};
+static const struct ex_stats_item ex_stats_table[] = {
+    {"disk", ex_stats_disk}, {"pcpu", ex_stats_pcpu}, {NULL, ex_stats_none},
+};
 
 /* BlockDeviceFormatBasename */
 _Bool blockdevice_format_basename = 0;
@@ -181,6 +194,12 @@ 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;
 
@@ -249,6 +268,54 @@ static int get_block_info(struct lv_block_info *binfo,
       ERROR("%s: %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) {
+    virTypedParamsFree(param, nparams);
+    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;
+  }
+
+  virTypedParamsFree(param, nparams);
+#endif /* HAVE_CPU_STATS */
+
+  return 0;
+}
+
 static void init_value_list(value_list_t *vl, virDomainPtr dom) {
   int n;
   const char *name;
@@ -355,6 +422,23 @@ static void memory_stats_submit(gauge_t value, virDomainPtr dom,
   submit(dom, "memory", tags[tag_index], &(value_t){.gauge = value}, 1);
 }
 
+static void submit_derive2(const char *type, derive_t v0, derive_t v1,
+                           virDomainPtr dom, const char *devname) {
+  value_t values[] = {
+      {.derive = v0}, {.derive = 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 void cpu_submit(unsigned long long value, virDomainPtr dom,
                        const char *type) {
   submit(dom, type, NULL, &(value_t){.derive = (derive_t)value}, 1);
@@ -369,15 +453,6 @@ static void vcpu_submit(derive_t value, virDomainPtr dom, int vcpu_nr,
   submit(dom, type, type_instance, &(value_t){.derive = value}, 1);
 }
 
-static void submit_derive2(const char *type, derive_t v0, derive_t v1,
-                           virDomainPtr dom, const char *devname) {
-  value_t values[] = {
-      {.derive = v0}, {.derive = v1},
-  };
-
-  submit(dom, type, devname, values, STATIC_ARRAY_SIZE(values));
-} /* void submit_derive2 */
-
 static void disk_submit(struct lv_block_info *binfo, virDomainPtr dom,
                         const char *type_instance) {
   char flush_type_instance[DATA_MAX_NAME_LEN];
@@ -409,6 +484,21 @@ static void disk_submit(struct lv_block_info *binfo, virDomainPtr dom,
   }
 }
 
+static unsigned int parse_ex_stats_flags(const char *exstats, int numexstats) {
+  int extra_stats = ex_stats_none;
+  for (int i = 0; i < numexstats; i++) {
+    for (int j = 0; ex_stats_table[j].name != NULL; j++) {
+      if (strcasecmp(exstats[i], ex_stats_table[j].name) == 0) {
+        DEBUG(PLUGIN_NAME " plugin: enabling extra stats for '%s'",
+              ex_stats_table[j].name);
+        extra_stats |= ex_stats_table[j].flag;
+        break;
+      }
+    }
+  }
+  return extra_stats;
+}
+
 static int lv_config(const char *key, const char *value) {
   if (virInitialize() != 0)
     return 1;
@@ -604,18 +694,12 @@ static int lv_config(const char *key, const char *value) {
   }
 
   if (strcasecmp(key, "ExtraStats") == 0) {
-    char *localvalue = sstrdup(value);
+    char *localvalue = strdup(value);
     if (localvalue != NULL) {
       char *exstats[EX_STATS_MAX_FIELDS];
       int numexstats =
           strsplit(localvalue, exstats, STATIC_ARRAY_SIZE(exstats));
-      for (int i = 0; i < numexstats; i++) {
-        if (strcasecmp(exstats[i], EX_STATS_DISK) == 0) {
-          DEBUG(PLUGIN_NAME " plugin: enabling extra stats for '%s'",
-                EX_STATS_DISK);
-          extra_stats |= ex_stats_disk;
-        }
-      }
+      extra_stats = parse_ex_stats_flags(exstats, numexstats);
       sfree(localvalue);
     }
   }
@@ -727,33 +811,35 @@ static int lv_read(user_data_t *ud) {
 
   /* Get CPU usage, memory, VCPU usage for each domain. */
   for (int i = 0; i < state->nr_domains; ++i) {
-    virDomainInfo info;
+    struct lv_info info;
     virVcpuInfoPtr vinfo = NULL;
     virDomainMemoryStatPtr minfo = NULL;
     int status;
 
-    status = virDomainGetInfo(state->domains[i], &info);
+    init_lv_info(&info);
+    status = lv_domain_info(state->domains[i], &info);
     if (status != 0) {
       ERROR(PLUGIN_NAME " plugin: virDomainGetInfo failed with status %i.",
             status);
       continue;
     }
 
-    if (info.state != VIR_DOMAIN_RUNNING) {
+    if (info.di.state != VIR_DOMAIN_RUNNING) {
       /* only gather stats for running domains */
       continue;
     }
 
-    cpu_submit(info.cpuTime, state->domains[i], "virt_cpu_total");
-    memory_submit((gauge_t)info.memory * 1024, state->domains[i]);
+    pcpu_submit(state->domains[i], &info);
+    cpu_submit(info.di.cpuTime, state->domains[i], "virt_cpu_total");
+    memory_submit((gauge_t)info.di.memory * 1024, state->domains[i]);
 
-    vinfo = malloc(info.nrVirtCpu * sizeof(vinfo[0]));
+    vinfo = malloc(info.di.nrVirtCpu * sizeof(vinfo[0]));
     if (vinfo == NULL) {
       ERROR(PLUGIN_NAME " plugin: malloc failed.");
       continue;
     }
 
-    status = virDomainGetVcpus(state->domains[i], vinfo, info.nrVirtCpu,
+    status = virDomainGetVcpus(state->domains[i], vinfo, info.di.nrVirtCpu,
                                /* cpu map = */ NULL, /* cpu map length = */ 0);
     if (status < 0) {
       ERROR(PLUGIN_NAME " plugin: virDomainGetVcpus failed with status %i.",
@@ -762,7 +848,7 @@ static int lv_read(user_data_t *ud) {
       continue;
     }
 
-    for (int j = 0; j < info.nrVirtCpu; ++j)
+    for (int j = 0; j < info.di.nrVirtCpu; ++j)
       vcpu_submit(vinfo[j].cpuTime, state->domains[i], vinfo[j].number,
                   "virt_vcpu");