From a735c52f7c585538821aafc7a409a1253678de58 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Tue, 6 Dec 2016 13:15:11 +0100 Subject: [PATCH] virt plugin: support virDomainBlockStatsFlags We want to export more disk stats, like disk_time. To do so, we must use the virDomainBlockStatsFlags API, but it could not be available on all the platforms collectd supports. To cope with that, we add a minimal layer around the API, and we use virDomainBlockStatsFlags only if available. This patch adds the code to use the new API and the fallback code in the case the platform' libvirt is too old. Signed-off-by: Francesco Romani --- src/virt.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 119 insertions(+), 14 deletions(-) diff --git a/src/virt.c b/src/virt.c index 830db513..bb5443cb 100644 --- a/src/virt.c +++ b/src/virt.c @@ -37,6 +37,12 @@ /* Plugin name */ #define PLUGIN_NAME "virt" +#ifdef LIBVIR_CHECK_VERSION +#if LIBVIR_CHECK_VERSION(0, 9, 5) +#define HAVE_BLOCK_STATS_FLAGS 1 +#endif +#endif + static const char *config_keys[] = {"Connection", "RefreshInterval", @@ -168,6 +174,65 @@ static time_t last_refresh = (time_t)0; static int refresh_lists(struct lv_read_instance *inst); +struct lv_block_info { + virDomainBlockStatsStruct bi; + + long long rd_total_times; + long long wr_total_times; + + long long fl_req; + long long fl_total_times; +}; + +static void init_block_info(struct lv_block_info *binfo) { + if (binfo == NULL) + return; + + binfo->bi.rd_req = -1; + binfo->bi.wr_req = -1; + binfo->bi.rd_bytes = -1; + binfo->bi.wr_bytes = -1; + + binfo->rd_total_times = -1; + binfo->wr_total_times = -1; + binfo->fl_req = -1; + binfo->fl_total_times = -1; +} + +#ifdef HAVE_BLOCK_STATS_FLAGS + +#define GET_BLOCK_INFO_VALUE(NAME, FIELD) \ + do { \ + if (!strcmp(param[i].field, NAME)) { \ + binfo->FIELD = param[i].value.l; \ + continue; \ + } \ + } while (0) + +static int get_block_info(struct lv_block_info *binfo, + virTypedParameterPtr param, int nparams) { + if (binfo == NULL || param == NULL) + return -1; + + for (int i = 0; i < nparams; ++i) { + /* ignore type. Everything must be LLONG anyway. */ + GET_BLOCK_INFO_VALUE("rd_operations", bi.rd_req); + GET_BLOCK_INFO_VALUE("wr_operations", bi.wr_req); + GET_BLOCK_INFO_VALUE("rd_bytes", bi.rd_bytes); + GET_BLOCK_INFO_VALUE("wr_bytes", bi.wr_bytes); + GET_BLOCK_INFO_VALUE("rd_total_times", rd_total_times); + GET_BLOCK_INFO_VALUE("wr_total_times", wr_total_times); + GET_BLOCK_INFO_VALUE("flush_operations", fl_req); + GET_BLOCK_INFO_VALUE("flush_total_times", fl_total_times); + } + + return 0; +} + +#undef GET_BLOCK_INFO_VALUE + +#endif /* HAVE_BLOCK_STATS_FLAGS */ + /* ERROR(...) macro for virterrors. */ #define VIRT_ERROR(conn, s) \ do { \ @@ -306,6 +371,17 @@ 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 disk_submit(struct lv_block_info *binfo, virDomainPtr dom, + const char *type_instance) { + if ((binfo->bi.rd_req != -1) && (binfo->bi.wr_req != -1)) + submit_derive2("disk_ops", (derive_t)binfo->bi.rd_req, + (derive_t)binfo->bi.wr_req, dom, type_instance); + + if ((binfo->bi.rd_bytes != -1) && (binfo->bi.wr_bytes != -1)) + submit_derive2("disk_octets", (derive_t)binfo->bi.rd_bytes, + (derive_t)binfo->bi.wr_bytes, dom, type_instance); +} + static int lv_config(const char *key, const char *value) { if (virInitialize() != 0) return 1; @@ -527,6 +603,42 @@ static void lv_disconnect(void) { WARNING(PLUGIN_NAME " plugin: closed connection to libvirt"); } +static int lv_domain_block_info(virDomainPtr dom, const char *path, + struct lv_block_info *binfo) { +#ifdef HAVE_BLOCK_STATS_FLAGS + virTypedParameterPtr params = NULL; + int nparams = 0; + int rc = -1; + int ret; + + ret = virDomainBlockStatsFlags(dom, path, NULL, &nparams, 0); + if (ret < 0 || nparams == 0) { + VIRT_ERROR(conn, "getting the disk params count"); + return -1; + } + + params = calloc(nparams, sizeof(virTypedParameter)); + if (params == NULL) { + ERROR("virt plugin: alloc(%i) for block=%s parameters failed.", nparams, + path); + return -1; + } + ret = virDomainBlockStatsFlags(dom, path, params, &nparams, 0); + if (ret < 0) { + VIRT_ERROR(conn, "getting the disk params values"); + goto done; + } + + rc = get_block_info(binfo, params, nparams); + +done: + virTypedParamsFree(params, nparams); + return rc; +#else + return virDomainBlockStats(dom, path, &(binfo->bi), sizeof(binfo->bi)); +#endif /* HAVE_BLOCK_STATS_FLAGS */ +} + static int lv_read(user_data_t *ud) { time_t t; struct lv_read_instance *inst = NULL; @@ -640,27 +752,20 @@ static int lv_read(user_data_t *ud) { /* Get block device stats for each domain. */ for (int i = 0; i < state->nr_block_devices; ++i) { - struct _virDomainBlockStats stats; + struct block_device *bdev = &(state->block_devices[i]); + struct lv_block_info binfo; + init_block_info(&binfo); - if (virDomainBlockStats(state->block_devices[i].dom, - state->block_devices[i].path, &stats, - sizeof stats) != 0) + 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(state->block_devices[i].path)); + type_instance = strdup(basename(bdev->path)); else - type_instance = strdup(state->block_devices[i].path); - - if ((stats.rd_req != -1) && (stats.wr_req != -1)) - submit_derive2("disk_ops", (derive_t)stats.rd_req, (derive_t)stats.wr_req, - state->block_devices[i].dom, type_instance); + type_instance = strdup(bdev->path); - if ((stats.rd_bytes != -1) && (stats.wr_bytes != -1)) - submit_derive2("disk_octets", (derive_t)stats.rd_bytes, - (derive_t)stats.wr_bytes, state->block_devices[i].dom, - type_instance); + disk_submit(&binfo, bdev->dom, type_instance); sfree(type_instance); } /* for (nr_block_devices) */ -- 2.30.2