Mailinglist Archive: vhostmd (42 mails)

< Previous Next >
[vhostmd] [PATCH 7/7 v3] Add CONNECT substitution <action>s.
  • From: "Richard W.M. Jones" <rjones@xxxxxxxxxx>
  • Date: Thu, 15 Oct 2009 18:01:48 +0100
  • Message-id: <20091015170148.GF31010@xxxxxxxxxxxxxxxxxxxx>
Jim pointed out another case where there could be a memory leak along
an error path. Update patch attached.

GC FTW ...

Rich.

--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
From ea2f772da718adb868150e9c4074ad49f0c374cb Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones@xxxxxxxxxx>
Date: Thu, 15 Oct 2009 13:10:18 +0100
Subject: [PATCH 1/2] Add CONNECT substitution <action>s.

Allow CONNECT to be substituted in any <action>s in order to
pass the correct libvirt URI down to actions such as virsh. The
correct way to run virsh is now:

virsh -r CONNECT cmd [...]

which substitutes as either:

virsh -r --connect 'uri' cmd [...]

or:

virsh -r cmd [...]

Note that you should always run virsh readonly (-r option).

Also corrects the documentation: ID should be VMID.

Also this cleans up the string replacement code and ensures that
it doesn't leak memory on failure.
---
docs/man/vhostmd.8 | 24 ++++++++++---
vhostmd.xml | 6 ++--
vhostmd/metric.c | 97 +++++++++++++++++++++++++++++++++++----------------
3 files changed, 87 insertions(+), 40 deletions(-)

diff --git a/docs/man/vhostmd.8 b/docs/man/vhostmd.8
index 6eca1dd..dfcb301 100644
--- a/docs/man/vhostmd.8
+++ b/docs/man/vhostmd.8
@@ -137,12 +137,24 @@ host and vm.

Currently, the metric element contains 3 elements: name, action, and variable.
The name element defines the metric's name. The action element describes a
-command or pipeline of commands used to gather the metric. For metrics of
-vm context, the tokens NAME, ID, and UUID may be used where these attributes
-of a VM are normally provided in a command. When the metric is sampled, these
-tokens will be substituted with the actual name, ID, or UUID of the vm
currently
-being sampled by vhostmd. If the metric type is xml, action is expected to
-retrun valid metric XML as defined below in "XML Format of Content".
+command or pipeline of commands used to gather the metric.
+
+Any <action> element can contain the magic token CONNECT which is
+replaced with the string "--connect 'uri'" where uri is the libvirt
+connection URI (specified on the command line to vhostmd as the -c
+option). If it wasn't specified, then the token CONNECT is
+substituted with the empty string. This allows you to write virsh
+commands like this:
+
+ virsh -r CONNECT command ...
+
+For metrics of vm context, the tokens NAME, VMID, and UUID may be used
+where these attributes of a VM are normally provided in a command.
+When the metric is sampled, these tokens will be substituted with the
+actual name, ID, or UUID of the vm currently being sampled by vhostmd.
+
+If the metric type is xml, action is expected to return valid metric
+XML as defined below in "XML Format of Content".

.SH Metrics Disk Format

diff --git a/vhostmd.xml b/vhostmd.xml
index c97e838..6f56b58 100644
--- a/vhostmd.xml
+++ b/vhostmd.xml
@@ -35,7 +35,7 @@ within the vm element.
<metrics>
<metric type="string" context="host">
<name>HostName</name>
- <action>virsh hostname | tr -d '[:space:]'</action>
+ <action>virsh CONNECT hostname | tr -d '[:space:]'</action>
</metric>
<metric type="string" context="host">
<name>VirtualizationVendor</name>
@@ -83,12 +83,12 @@ within the vm element.
</metric>
<metric type="real64" context="host">
<name>TotalCPUTime</name>
- <action>virsh dominfo 0 | sed 's/: */:/' | \
+ <action>virsh CONNECT dominfo 0 | sed 's/: */:/' | \
gawk -F: '/CPU time/ {print $2;}'</action>
</metric>
<metric type="real64" context="vm">
<name>TotalCPUTime</name>
- <action>virsh dominfo NAME | sed 's/: */:/' | \
+ <action>virsh CONNECT dominfo NAME | sed 's/: */:/' | \
gawk -F: '/CPU time/ {print $2;}'</action>
</metric>
</metrics>
diff --git a/vhostmd/metric.c b/vhostmd/metric.c
index 9407e0c..84b1678 100644
--- a/vhostmd/metric.c
+++ b/vhostmd/metric.c
@@ -30,31 +30,68 @@
#include "util.h"
#include "metric.h"

+/* If this returns NULL (for error), then 'str' has been freed. */
+static char *replace (char *str, const char *patt, const char *format, ...)
+{
+ char *repl = NULL;
+ char *old_str;
+ va_list args;
+ int r;
+
+ va_start (args, format);
+ r = vasprintf (&repl, format, args);
+ va_end (args);
+ if (r == -1) {
+ vu_log (VHOSTMD_ERR, "vasprintf failed: %m");
+ free (repl);
+ free (str);
+ return NULL;
+ }
+
+ old_str = str;
+ str = vu_str_replace (old_str, patt, repl);
+ free (repl);
+ if (str == NULL) {
+ vu_log (VHOSTMD_ERR, "vu_str_replace failed: %m");
+ free (old_str);
+ return NULL;
+ }
+ free (old_str);
+
+ return str;
+}
+
static int metric_action_subst(metric *m, char **action)
{
- char *str;
- char *temp;
- char *domid;
+ char *temp;

- if ((str = vu_str_replace(m->action, "NAME", m->vm->name)) == NULL)
- return -1;
+ temp = strdup (m->action);
+ if (temp == NULL) {
+ vu_log (VHOSTMD_ERR, "strdup: %m");
+ return -1;
+ }

- temp = str;
- if (asprintf(&domid, "%d", m->vm->id) == -1)
- return -1;
- if ((str = vu_str_replace(temp, "VMID", domid)) == NULL)
- return -1;
- free(domid);
- free(temp);
+ if (libvirt_uri) {
+ temp = replace (temp, "CONNECT", "--connect '%s'", libvirt_uri);
+ if (temp == NULL) return -1;
+ } else {
+ temp = replace (temp, "CONNECT", "");
+ if (temp == NULL) return -1;
+ }

- temp = str;
- if ((str = vu_str_replace(temp, "UUID", m->vm->uuid)) == NULL)
- return -1;
- free(temp);
+ if (m->ctx == METRIC_CONTEXT_VM) {
+ temp = replace (temp, "NAME", "%s", m->vm->name);
+ if (temp == NULL) return -1;

- *action = str;
+ temp = replace (temp, "VMID", "%d", m->vm->id);
+ if (temp == NULL) return -1;

- return 0;
+ temp = replace (temp, "UUID", "%s", m->vm->uuid);
+ if (temp == NULL) return -1;
+ }
+
+ *action = temp;
+ return 0;
}

/*
@@ -208,29 +245,27 @@ int metric_value_get(metric *m)
FILE *fp = NULL;
int ret = -1;
size_t len;
+ char *cmd = NULL;

if (m->pf) {
ret = m->pf(m);
return ret;
}

- if (m->ctx == METRIC_CONTEXT_VM) {
- char *cmd = NULL;
+ if (metric_action_subst(m, &cmd)) {
+ vu_log(VHOSTMD_ERR, "Failed action 'KEYWORD' substitution");
+ return ret;
+ }
+
+ fp = popen (cmd, "r");

- if (metric_action_subst(m, &cmd)) {
- vu_log(VHOSTMD_ERR, "Failed action 'KEYWORD' substitution");
- return ret;
- }
- fp = popen(cmd, "r");
+ if (fp == NULL) {
+ vu_log(VHOSTMD_ERR, "Command failed: %s", cmd);
free(cmd);
+ return ret;
}
- else {
- fp = popen(m->action, "r");
- }
+ free(cmd);

- if (fp == NULL)
- return ret;
-
if (m->type == M_XML)
len = 2048;
else
--
1.6.5.rc2

< Previous Next >
Follow Ups