From 650a058c8284d1fc845f13e11eecee064ea02b6b Mon Sep 17 00:00:00 2001
From: Francesco Romani <fromani@redhat.com>
Date: Tue, 14 Feb 2017 10:34:52 +0100
Subject: [PATCH] virt plugin: Address PR comments

Signed-off-by: Francesco Romani <fromani@redhat.com>
---
 src/collectd.conf.pod | 10 +++++++---
 src/virt.c            | 28 ++++++++++++++++------------
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/src/collectd.conf.pod b/src/collectd.conf.pod
index 57f2897d..9b32a781 100644
--- a/src/collectd.conf.pod
+++ b/src/collectd.conf.pod
@@ -8078,9 +8078,13 @@ If you are not sure, just use the default setting.
 
 =item B<ExtraStats> B<string>
 
-Enable extra statistics. This allows the plugin to reported more detailed
-statistics about the behaviour of Virtual Machines. The argument is a
-space-separated list of selectors. Currently only B<disk> is supported.
+Report additional extra statistics. The default is no extra statistics, preserving
+the previous behaviour of the plugin. If unsure, leave the default. If enabled,
+allows the plugin to reported more detailed statistics about the behaviour of
+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.
 
 =back
 
diff --git a/src/virt.c b/src/virt.c
index 06081097..5a4604c8 100644
--- a/src/virt.c
+++ b/src/virt.c
@@ -607,9 +607,8 @@ static int lv_config(const char *key, const char *value) {
     char *localvalue = sstrdup(value);
     if (localvalue != NULL) {
       char *exstats[EX_STATS_MAX_FIELDS];
-      int numexstats;
-
-      numexstats = strsplit(localvalue, exstats, STATIC_ARRAY_SIZE(exstats));
+      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'",
@@ -654,9 +653,7 @@ static int lv_domain_block_info(virDomainPtr dom, const char *path,
   virTypedParameterPtr params = NULL;
   int nparams = 0;
   int rc = -1;
-  int ret;
-
-  ret = virDomainBlockStatsFlags(dom, path, NULL, &nparams, 0);
+  int ret = virDomainBlockStatsFlags(dom, path, NULL, &nparams, 0);
   if (ret < 0 || nparams == 0) {
     VIRT_ERROR(conn, "getting the disk params count");
     return -1;
@@ -804,15 +801,22 @@ static int lv_read(user_data_t *ud) {
     if (lv_domain_block_info(bdev->dom, bdev->path, &binfo) < 0)
       continue;
 
-    char *type_instance = NULL;
-    if (blockdevice_format_basename && blockdevice_format == source)
-      type_instance = strdup(basename(bdev->path));
-    else
-      type_instance = strdup(bdev->path);
+    char *type_instance = bdev->path;
+    char *path = NULL;
+    if (blockdevice_format_basename && blockdevice_format == source) {
+      path = strdup(bdev->path);
+      if (path == NULL) {
+        WARNING(PLUGIN_NAME
+                " plugin: error extracting the basename for '%s', skipped",
+                bdev->path);
+        continue;
+      }
+      type_instance = basename(path);
+    }
 
     disk_submit(&binfo, bdev->dom, type_instance);
 
-    sfree(type_instance);
+    sfree(path);
   } /* for (nr_block_devices) */
 
   /* Get interface stats for each domain. */
-- 
2.11.0