2:20 p.m.
This patchset replaces:
https://www.redhat.com/archives/libvir-list/2013-July/msg00108.html
Changes since V1:
* Remove the external (eg, qemu_monitor.{c,h}) API's to the qom-list,
qom-get, qom-set interfaces
* Handle errors in virXPathInt() return checking domain_conf.c
* Move the 'period' from _qemuDomainObjPrivate into _qemuMonitor
* Move/rename qemuProcessFindBalloonObjectPath from qemu_process.c
into qemu_monitor.c as qemuMonitorFindBalloonObjectPath()
* Create qemuMonitorJSONSetMemoryStatsPeriod() in qemu_monitor_json.c from
parts of prior change to add qemuProcessUpdateBalloonStatsPeriod()
in qemu_process.c and hav
* Add qemuMonitorSetMemoryStatsPeriod() in qemu_monitor.c to set
the collection period. Adjust the code to take parts of prior change
and make check for balloon path in this path (set if necessary).
* Adjust code in qemuProcessReconnect(), qemuProcessStart(), and
qemuProcessAttach() to call qemuMonitorSetMemoryStatsPeriod()
* Adjust qemuMonitorJSONGetMemoryStats() to get the balloon stats once
the "actual" stat is collected from qemuMonitorJSONGetBalloonInfo().
Reviewer note: The prior code to get "actual" was a duplicate of the
BalloonInfo code. Since "balloon-info" is where the actual memory is
still stored it still must be called as that data is not present in
the balloon stats qom-get "stats" fetch implemented as the new API
qemuMonitorJSONGetBalloonStats().
* Avoid overloading virDomainSetMemoryFlags()...
* Create/use virDomainSetMemoryStatsPeriodFlags() to set the collection
period dynamically from "virsh dommemstats" using the --period qualifier.
John Ferlan (10):
Add qemuMonitorJSONGetObjectListPaths() method for QMP qom-list
command
Add qemuMonitorJSONGetObjectProperty() method for QMP qom-get command
Add qemuMonitorJSONSetObjectProperty() method for QMP qom-set command
Add 'period' for Memballoon statistics gathering capability
Determine whether to start balloon memory stats gathering.
Add capability to fetch balloon stats
Add new public API virDomainSetMemoryStatsPeriodFlags
Specify remote protocol for virDomainSetMemoryStatsPeriodFlags
Implement the virDomainSetMemoryStatsPeriodFlags for QEMU driver
Allow balloon driver collection to be adjusted dynamically
docs/formatdomain.html.in | 19 ++
docs/schemas/domaincommon.rng | 7 +
include/libvirt/libvirt.h.in | 3 +
src/conf/domain_conf.c | 44 +++-
src/conf/domain_conf.h | 1 +
src/driver.h | 6 +
src/libvirt.c | 65 ++++++
src/libvirt_public.syms | 5 +
src/qemu/qemu_driver.c | 66 ++++++
src/qemu/qemu_monitor.c | 133 ++++++++++-
src/qemu/qemu_monitor.h | 2 +
src/qemu/qemu_monitor_json.c | 498 ++++++++++++++++++++++++++++++++----------
src/qemu/qemu_monitor_json.h | 64 ++++++
src/qemu/qemu_process.c | 21 +-
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 15 +-
src/remote_protocol-structs | 6 +
tests/qemumonitorjsontest.c | 186 ++++++++++++++++
tools/virsh-domain-monitor.c | 66 +++++-
19 files changed, 1085 insertions(+), 123 deletions(-)
--
1.8.1.4
2:20 p.m.
New subject: [libvirt] [PATCH v2 01/10] Add qemuMonitorJSONGetObjectListPaths() method for QMP qom-list command
Add a new qemuMonitorJSONGetObjectListPaths() method to support invocation
of the 'qom-list' JSON monitor command with a provided path.
The returned list of paired data fields of "name" and "type" that can
be used to peruse QOM configuration data and eventually utilize for the
balloon statistics.
The test does a "{"execute":"qom-list", "arguments": {
"path": "/"}}" which
returns "{"return": [{"name": "machine",
"type": "child<container>"},
{"name": "type", "type": "string"}]}"
resulting in a return of an array
of 2 elements with [0].name="machine",
[0].type="child<container>". The [1]
entry appears to be a header that could be used some day via a command such
as "virsh qemuobject --list" to format output.
---
src/qemu/qemu_monitor_json.c | 102 +++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 16 +++++++
tests/qemumonitorjsontest.c | 79 +++++++++++++++++++++++++++++++++
3 files changed, 197 insertions(+)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 3383c88..fc2b65f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4540,6 +4540,108 @@ cleanup:
}
+int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
+ const char *path,
+ qemuMonitorJSONListPathPtr **paths)
+{
+ int ret;
+ virJSONValuePtr cmd;
+ virJSONValuePtr reply = NULL;
+ virJSONValuePtr data;
+ qemuMonitorJSONListPathPtr *pathlist = NULL;
+ int n = 0;
+ size_t i;
+
+ *paths = NULL;
+
+ if (!(cmd = qemuMonitorJSONMakeCommand("qom-list",
+ "s:path", path,
+ NULL)))
+ return -1;
+
+ ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+ if (ret == 0)
+ ret = qemuMonitorJSONCheckError(cmd, reply);
+
+ if (ret < 0)
+ goto cleanup;
+
+ ret = -1;
+
+ if (!(data = virJSONValueObjectGet(reply, "return"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("qom-list reply was missing return data"));
+ goto cleanup;
+ }
+
+ if ((n = virJSONValueArraySize(data)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("qom-list reply data was not an array"));
+ goto cleanup;
+ }
+
+ /* null-terminated list */
+ if (VIR_ALLOC_N(pathlist, n + 1) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ for (i = 0; i < n; i++) {
+ virJSONValuePtr child = virJSONValueArrayGet(data, i);
+ const char *tmp;
+ qemuMonitorJSONListPathPtr info;
+
+ if (VIR_ALLOC(info) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ pathlist[i] = info;
+
+ if (!(tmp = virJSONValueObjectGetString(child, "name"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("qom-list reply data was missing
'name'"));
+ goto cleanup;
+ }
+
+ if (VIR_STRDUP(info->name, tmp) < 0)
+ goto cleanup;
+
+ if (virJSONValueObjectHasKey(child, "type")) {
+ if (!(tmp = virJSONValueObjectGetString(child, "type"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("qom-list reply has malformed 'type'
data"));
+ goto cleanup;
+ }
+ if (VIR_STRDUP(info->type, tmp) < 0)
+ goto cleanup;
+ }
+ }
+
+ ret = n;
+ *paths = pathlist;
+
+cleanup:
+ if (ret < 0 && pathlist) {
+ for (i = 0; i < n; i++)
+ qemuMonitorJSONListPathFree(pathlist[i]);
+ VIR_FREE(pathlist);
+ }
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ return ret;
+}
+
+void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths)
+{
+ if (!paths)
+ return;
+ VIR_FREE(paths->name);
+ VIR_FREE(paths->type);
+ VIR_FREE(paths);
+}
+
int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index d79b86b..e7ce145 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -332,6 +332,22 @@ int qemuMonitorJSONGetKVMState(qemuMonitorPtr mon,
int qemuMonitorJSONGetObjectTypes(qemuMonitorPtr mon,
char ***types)
ATTRIBUTE_NONNULL(2);
+
+typedef struct _qemuMonitorJSONListPath qemuMonitorJSONListPath;
+typedef qemuMonitorJSONListPath *qemuMonitorJSONListPathPtr;
+
+struct _qemuMonitorJSONListPath {
+ char *name;
+ char *type;
+};
+
+int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
+ const char *path,
+ qemuMonitorJSONListPathPtr **paths)
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+
+void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths);
+
int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index acc94ca..380d377 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -23,6 +23,7 @@
#include "testutilsqemu.h"
#include "qemumonitortestutils.h"
#include "qemu/qemu_conf.h"
+#include "qemu/qemu_monitor_json.h"
#include "virthread.h"
#include "virerror.h"
#include "virstring.h"
@@ -595,6 +596,83 @@ cleanup:
}
+/*
+ * This test will request to return a list of paths for "/". It should be
+ * a simple list of 1 real element that being the "machine". The following
+ * is the execution and expected return:
+ *
+ * {"execute":"qom-list", "arguments": { "path":
"/"}}"
+ * {"return": [{"name": "machine", "type":
"child<container>"}, \
+ * {"name": "type", "type":
"string"}]}
+ */
+static int
+testQemuMonitorJSONGetListPaths(const void *data)
+{
+ const virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+ qemuMonitorTestPtr test = qemuMonitorTestNew(true, xmlopt);
+ int ret = -1;
+ qemuMonitorJSONListPathPtr *paths;
+ int npaths = 0;
+ int i;
+
+ if (!test)
+ return -1;
+
+ if (qemuMonitorTestAddItem(test, "qom-list",
+ "{ "
+ " \"return\": [ "
+ " {\"name\": \"machine\",
"
+ " \"type\":
\"child<container>\"}, "
+ " {\"name\": \"type\", "
+ " \"type\": \"string\"} "
+ " ]"
+ "}") < 0)
+ goto cleanup;
+
+ /* present with path */
+ if ((npaths = qemuMonitorJSONGetObjectListPaths(
+ qemuMonitorTestGetMonitor(test),
+ "/",
+ &paths)) < 0)
+ goto cleanup;
+
+ if (npaths != 2) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "npaths was %d, expected 1", npaths);
+ goto cleanup;
+ }
+
+#define CHECK(i, wantname, wanttype) \
+ do { \
+ if (STRNEQ(paths[i]->name, (wantname))) { \
+ virReportError(VIR_ERR_INTERNAL_ERROR, \
+ "name was %s, expected %s", \
+ paths[i]->name, (wantname)); \
+ goto cleanup; \
+ } \
+ if (STRNEQ_NULLABLE(paths[i]->type, (wanttype))) { \
+ virReportError(VIR_ERR_INTERNAL_ERROR, \
+ "type was %s, expected %s", \
+ NULLSTR(paths[i]->type), (wanttype)); \
+ goto cleanup; \
+ } \
+ } while (0)
+
+ CHECK(0, "machine", "child<container>");
+
+#undef CHECK
+
+ ret = 0;
+
+cleanup:
+ qemuMonitorTestFree(test);
+ for (i = 0; i < npaths; i++)
+ qemuMonitorJSONListPathFree(paths[i]);
+ VIR_FREE(paths);
+ return ret;
+}
+
+
static int
mymain(void)
{
@@ -623,6 +701,7 @@ mymain(void)
DO_TEST(GetCommands);
DO_TEST(GetTPMModels);
DO_TEST(GetCommandLineOptionParameters);
+ DO_TEST(GetListPaths);
virObjectUnref(xmlopt);
--
1.8.1.4
9:06 a.m.
New subject: [libvirt] [PATCH v2 01/10] Add qemuMonitorJSONGetObjectListPaths() method for QMP qom-list command
On 08.07.2013 21:20, John Ferlan wrote:
Add a new qemuMonitorJSONGetObjectListPaths() method to support
invocation
of the 'qom-list' JSON monitor command with a provided path.
The returned list of paired data fields of "name" and "type" that
can
be used to peruse QOM configuration data and eventually utilize for the
balloon statistics.
The test does a "{"execute":"qom-list", "arguments": {
"path": "/"}}" which
returns "{"return": [{"name": "machine",
"type": "child<container>"},
{"name": "type", "type": "string"}]}"
resulting in a return of an array
of 2 elements with [0].name="machine",
[0].type="child<container>". The [1]
entry appears to be a header that could be used some day via a command such
as "virsh qemuobject --list" to format output.
---
src/qemu/qemu_monitor_json.c | 102 +++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 16 +++++++
tests/qemumonitorjsontest.c | 79 +++++++++++++++++++++++++++++++++
3 files changed, 197 insertions(+)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 3383c88..fc2b65f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4540,6 +4540,108 @@ cleanup:
}
+int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
+ const char *path,
+ qemuMonitorJSONListPathPtr **paths)
+{
+ int ret;
+ virJSONValuePtr cmd;
+ virJSONValuePtr reply = NULL;
+ virJSONValuePtr data;
+ qemuMonitorJSONListPathPtr *pathlist = NULL;
+ int n = 0;
+ size_t i;
+
+ *paths = NULL;
+
+ if (!(cmd = qemuMonitorJSONMakeCommand("qom-list",
+ "s:path", path,
+ NULL)))
+ return -1;
+
+ ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+ if (ret == 0)
+ ret = qemuMonitorJSONCheckError(cmd, reply);
+
+ if (ret < 0)
+ goto cleanup;
+
+ ret = -1;
+
+ if (!(data = virJSONValueObjectGet(reply, "return"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("qom-list reply was missing return data"));
+ goto cleanup;
+ }
+
+ if ((n = virJSONValueArraySize(data)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("qom-list reply data was not an array"));
+ goto cleanup;
+ }
+
+ /* null-terminated list */
Do we need NULL terminated list ... [1]
+ if (VIR_ALLOC_N(pathlist, n + 1) < 0) {
+ virReportOOMError();
This can be dropped now. But as you've said in one of previous e-mails
of your, you already did that.
+ goto cleanup;
+ }
+
+ for (i = 0; i < n; i++) {
+ virJSONValuePtr child = virJSONValueArrayGet(data, i);
+ const char *tmp;
+ qemuMonitorJSONListPathPtr info;
+
+ if (VIR_ALLOC(info) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ pathlist[i] = info;
+
+ if (!(tmp = virJSONValueObjectGetString(child, "name"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("qom-list reply data was missing
'name'"));
+ goto cleanup;
+ }
+
+ if (VIR_STRDUP(info->name, tmp) < 0)
+ goto cleanup;
+
+ if (virJSONValueObjectHasKey(child, "type")) {
+ if (!(tmp = virJSONValueObjectGetString(child, "type"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("qom-list reply has malformed 'type'
data"));
+ goto cleanup;
+ }
+ if (VIR_STRDUP(info->type, tmp) < 0)
+ goto cleanup;
+ }
+ }
+
+ ret = n;
1: ... if we are returning number of items in array? I'm not saying it's
something wrong, just curious.
+ *paths = pathlist;
+
+cleanup:
+ if (ret < 0 && pathlist) {
+ for (i = 0; i < n; i++)
+ qemuMonitorJSONListPathFree(pathlist[i]);
+ VIR_FREE(pathlist);
+ }
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ return ret;
+}
+
+void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths)
+{
+ if (!paths)
+ return;
+ VIR_FREE(paths->name);
+ VIR_FREE(paths->type);
+ VIR_FREE(paths);
+}
+
int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index d79b86b..e7ce145 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -332,6 +332,22 @@ int qemuMonitorJSONGetKVMState(qemuMonitorPtr mon,
int qemuMonitorJSONGetObjectTypes(qemuMonitorPtr mon,
char ***types)
ATTRIBUTE_NONNULL(2);
+
+typedef struct _qemuMonitorJSONListPath qemuMonitorJSONListPath;
+typedef qemuMonitorJSONListPath *qemuMonitorJSONListPathPtr;
+
+struct _qemuMonitorJSONListPath {
+ char *name;
+ char *type;
+};
+
+int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
+ const char *path,
+ qemuMonitorJSONListPathPtr **paths)
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+
+void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths);
+
int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index acc94ca..380d377 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -23,6 +23,7 @@
#include "testutilsqemu.h"
#include "qemumonitortestutils.h"
#include "qemu/qemu_conf.h"
+#include "qemu/qemu_monitor_json.h"
#include "virthread.h"
#include "virerror.h"
#include "virstring.h"
@@ -595,6 +596,83 @@ cleanup:
}
+/*
+ * This test will request to return a list of paths for "/". It should be
+ * a simple list of 1 real element that being the "machine". The following
+ * is the execution and expected return:
+ *
+ * {"execute":"qom-list", "arguments": {
"path": "/"}}"
+ * {"return": [{"name": "machine", "type":
"child<container>"}, \
+ * {"name": "type", "type":
"string"}]}
+ */
+static int
+testQemuMonitorJSONGetListPaths(const void *data)
+{
+ const virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+ qemuMonitorTestPtr test = qemuMonitorTestNew(true, xmlopt);
+ int ret = -1;
+ qemuMonitorJSONListPathPtr *paths;
+ int npaths = 0;
+ int i;
And after Daniel's patches, this needs to be size_t i;
+
+ if (!test)
+ return -1;
+
+ if (qemuMonitorTestAddItem(test, "qom-list",
+ "{ "
+ " \"return\": [ "
+ " {\"name\": \"machine\",
"
+ " \"type\":
\"child<container>\"}, "
+ " {\"name\": \"type\", "
+ " \"type\": \"string\"}
"
+ " ]"
+ "}") < 0)
+ goto cleanup;
+
+ /* present with path */
+ if ((npaths = qemuMonitorJSONGetObjectListPaths(
+ qemuMonitorTestGetMonitor(test),
+ "/",
+ &paths)) < 0)
+ goto cleanup;
+
+ if (npaths != 2) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "npaths was %d, expected 1", npaths);
+ goto cleanup;
+ }
+
+#define CHECK(i, wantname, wanttype) \
+ do { \
+ if (STRNEQ(paths[i]->name, (wantname))) { \
+ virReportError(VIR_ERR_INTERNAL_ERROR, \
+ "name was %s, expected %s", \
+ paths[i]->name, (wantname)); \
+ goto cleanup; \
+ } \
+ if (STRNEQ_NULLABLE(paths[i]->type, (wanttype))) { \
+ virReportError(VIR_ERR_INTERNAL_ERROR, \
+ "type was %s, expected %s", \
+ NULLSTR(paths[i]->type), (wanttype)); \
+ goto cleanup; \
+ } \
+ } while (0)
+
+ CHECK(0, "machine", "child<container>");
+
+#undef CHECK
+
+ ret = 0;
+
+cleanup:
+ qemuMonitorTestFree(test);
+ for (i = 0; i < npaths; i++)
+ qemuMonitorJSONListPathFree(paths[i]);
+ VIR_FREE(paths);
+ return ret;
+}
+
+
static int
mymain(void)
{
@@ -623,6 +701,7 @@ mymain(void)
DO_TEST(GetCommands);
DO_TEST(GetTPMModels);
DO_TEST(GetCommandLineOptionParameters);
+ DO_TEST(GetListPaths);
virObjectUnref(xmlopt);
Michal
10:24 a.m.
New subject: [libvirt] [PATCH v2 01/10] Add qemuMonitorJSONGetObjectListPaths() method for QMP qom-list command
On 07/11/2013 10:06 AM, Michal Privoznik wrote:
On 08.07.2013 21:20, John Ferlan wrote:
<...snip...>
> diff --git a/src/qemu/qemu_monitor_json.c
b/src/qemu/qemu_monitor_json.c
> index 3383c88..fc2b65f 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -4540,6 +4540,108 @@ cleanup:
> }
>
>
> +int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
> + const char *path,
> + qemuMonitorJSONListPathPtr **paths)
> +{
> + int ret;
> + virJSONValuePtr cmd;
> + virJSONValuePtr reply = NULL;
> + virJSONValuePtr data;
> + qemuMonitorJSONListPathPtr *pathlist = NULL;
> + int n = 0;
> + size_t i;
> +
> + *paths = NULL;
> +
> + if (!(cmd = qemuMonitorJSONMakeCommand("qom-list",
> + "s:path", path,
> + NULL)))
> + return -1;
> +
> + ret = qemuMonitorJSONCommand(mon, cmd, &reply);
> +
> + if (ret == 0)
> + ret = qemuMonitorJSONCheckError(cmd, reply);
> +
> + if (ret < 0)
> + goto cleanup;
> +
> + ret = -1;
> +
> + if (!(data = virJSONValueObjectGet(reply, "return"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("qom-list reply was missing return data"));
> + goto cleanup;
> + }
> +
> + if ((n = virJSONValueArraySize(data)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("qom-list reply data was not an array"));
> + goto cleanup;
> + }
> +
> + /* null-terminated list */
Do we need NULL terminated list ... [1]
This same comment and sequence exists in many other places - this was a
lot of cut-n-paste
> + if (VIR_ALLOC_N(pathlist, n + 1) < 0) {
> + virReportOOMError();
This can be dropped now. But as you've said in one of previous e-mails
of your, you already did that.
Right - it was dropped as were the {}
> + goto cleanup;
> + }
> +
> + for (i = 0; i < n; i++) {
> + virJSONValuePtr child = virJSONValueArrayGet(data, i);
> + const char *tmp;
> + qemuMonitorJSONListPathPtr info;
> +
> + if (VIR_ALLOC(info) < 0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + pathlist[i] = info;
> +
> + if (!(tmp = virJSONValueObjectGetString(child, "name"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("qom-list reply data was missing
'name'"));
> + goto cleanup;
> + }
> +
> + if (VIR_STRDUP(info->name, tmp) < 0)
> + goto cleanup;
> +
> + if (virJSONValueObjectHasKey(child, "type")) {
> + if (!(tmp = virJSONValueObjectGetString(child, "type"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("qom-list reply has malformed 'type'
data"));
> + goto cleanup;
> + }
> + if (VIR_STRDUP(info->type, tmp) < 0)
> + goto cleanup;
> + }
> + }
> +
> + ret = n;
1: ... if we are returning number of items in array? I'm not saying it's
something wrong, just curious.
Yes, we are returning the number of items in the array the callers then
use that to walk through the results.
> + *paths = pathlist;
> +
> +cleanup:
> + if (ret < 0 && pathlist) {
> + for (i = 0; i < n; i++)
> + qemuMonitorJSONListPathFree(pathlist[i]);
> + VIR_FREE(pathlist);
> + }
> + virJSONValueFree(cmd);
> + virJSONValueFree(reply);
> + return ret;
> +}
<...snip...>
John
9:48 a.m.
New subject: [libvirt] [PATCH v2 01/10] Add qemuMonitorJSONGetObjectListPaths() method for QMP qom-list command
On Mon, Jul 08, 2013 at 03:20:27PM -0400, John Ferlan wrote:
Add a new qemuMonitorJSONGetObjectListPaths() method to support
invocation
of the 'qom-list' JSON monitor command with a provided path.
The returned list of paired data fields of "name" and "type" that
can
be used to peruse QOM configuration data and eventually utilize for the
balloon statistics.
The test does a "{"execute":"qom-list", "arguments": {
"path": "/"}}" which
returns "{"return": [{"name": "machine",
"type": "child<container>"},
{"name": "type", "type": "string"}]}"
resulting in a return of an array
of 2 elements with [0].name="machine",
[0].type="child<container>". The [1]
entry appears to be a header that could be used some day via a command such
as "virsh qemuobject --list" to format output.
---
src/qemu/qemu_monitor_json.c | 102 +++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 16 +++++++
tests/qemumonitorjsontest.c | 79 +++++++++++++++++++++++++++++++++
3 files changed, 197 insertions(+)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:20 p.m.
New subject: [libvirt] [PATCH v2 02/10] Add qemuMonitorJSONGetObjectProperty() method for QMP qom-get command
Add a new qemuMonitorJSONGetObjectProperty() method to support invocation
of the 'qom-get' JSON monitor command with a provided path, property, and
expected data type return. The qemuMonitorJSONObjectProperty is similar to
virTypedParameter; however, a future patch will extend it a bit to include
a void pointer to balloon driver statistic data.
The provided test will execute a qom-get on "/machine/i440fx" which will
return a property "realized".
---
src/qemu/qemu_monitor_json.c | 86 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 34 ++++++++++++++++++
tests/qemumonitorjsontest.c | 48 +++++++++++++++++++++++++
3 files changed, 168 insertions(+)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index fc2b65f..db107f1 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4642,6 +4642,92 @@ void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths)
VIR_FREE(paths);
}
+
+int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorJSONObjectPropertyPtr prop)
+{
+ int ret;
+ virJSONValuePtr cmd;
+ virJSONValuePtr reply = NULL;
+ virJSONValuePtr data;
+ const char *tmp;
+
+ if (!(cmd = qemuMonitorJSONMakeCommand("qom-get",
+ "s:path", path,
+ "s:property", property,
+ NULL)))
+ return -1;
+
+ ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+ if (ret == 0)
+ ret = qemuMonitorJSONCheckError(cmd, reply);
+
+ if (ret < 0)
+ goto cleanup;
+
+ ret = -1;
+
+ if (!(data = virJSONValueObjectGet(reply, "return"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("qom-get reply was missing return data"));
+ goto cleanup;
+ }
+
+ switch (prop->type) {
+ /* Simple cases of boolean, int, long, uint, ulong, double, and string
+ * will receive return value as part of {"return": xxx} statement
+ */
+ case QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN:
+ ret = virJSONValueGetBoolean(data, &prop->val.b);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_INT:
+ ret = virJSONValueGetNumberInt(data, &prop->val.i);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_LONG:
+ ret = virJSONValueGetNumberLong(data, &prop->val.l);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_UINT:
+ ret = virJSONValueGetNumberUint(data, &prop->val.ui);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_ULONG:
+ ret = virJSONValueGetNumberUlong(data, &prop->val.ul);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_DOUBLE:
+ ret = virJSONValueGetNumberDouble(data, &prop->val.d);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_STRING:
+ tmp = virJSONValueGetString(data);
+ if (tmp && VIR_STRDUP(prop->val.str, tmp) < 0)
+ goto cleanup;
+ if (tmp)
+ ret = 0;
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_LAST:
+ default:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("qom-get invalid object property type %d"),
+ prop->type);
+ goto cleanup;
+ }
+
+ if (ret == -1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("qom-get reply was missing return data"));
+ goto cleanup;
+ }
+
+ ret = 0;
+cleanup:
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+
+ return ret;
+}
+
+
int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index e7ce145..e6bbd62 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -348,6 +348,40 @@ int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths);
+/* Flags for the 'type' field in _qemuMonitorJSONObjectProperty */
+typedef enum {
+ QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN=1,
+ QEMU_MONITOR_OBJECT_PROPERTY_INT,
+ QEMU_MONITOR_OBJECT_PROPERTY_LONG,
+ QEMU_MONITOR_OBJECT_PROPERTY_UINT,
+ QEMU_MONITOR_OBJECT_PROPERTY_ULONG,
+ QEMU_MONITOR_OBJECT_PROPERTY_DOUBLE,
+ QEMU_MONITOR_OBJECT_PROPERTY_STRING,
+
+ QEMU_MONITOR_OBJECT_PROPERTY_LAST
+} qemuMonitorJSONObjectPropertyType;
+
+typedef struct _qemuMonitorJSONObjectProperty qemuMonitorJSONObjectProperty;
+typedef qemuMonitorJSONObjectProperty *qemuMonitorJSONObjectPropertyPtr;
+struct _qemuMonitorJSONObjectProperty {
+ int type; /* qemuMonitorJSONObjectPropertyType */
+ union {
+ bool b;
+ int i;
+ long long l;
+ unsigned int ui;
+ unsigned long long ul;
+ double d;
+ char *str;
+ } val;
+};
+
+int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorJSONObjectPropertyPtr prop)
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+
int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 380d377..2c57211 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -673,6 +673,53 @@ cleanup:
}
+/*
+ * This test will use a path to /machine/i440fx which should exist in order
+ * to ensure that the qom-get property fetch works properly. The following
+ * is the execution and expected return:
+ *
+ *
+ * { "execute": "qom-get","arguments": \
+ * { "path": "/machine/i440fx","property":
"realized"}}
+ * {"return": true}
+ */
+static int
+testQemuMonitorJSONGetObjectProperty(const void *data)
+{
+ const virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+ qemuMonitorTestPtr test = qemuMonitorTestNew(true, xmlopt);
+ int ret = -1;
+ qemuMonitorJSONObjectProperty prop;
+
+ if (!test)
+ return -1;
+
+ if (qemuMonitorTestAddItem(test, "qom-get",
+ "{ \"return\": true }") < 0)
+ goto cleanup;
+
+ /* Present with path and property */
+ memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN;
+ if (qemuMonitorJSONGetObjectProperty(qemuMonitorTestGetMonitor(test),
+ "/machine/i440fx",
+ "realized",
+ &prop) < 0)
+ goto cleanup;
+
+ if (!prop.val.b) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ "expected true, but false returned");
+ goto cleanup;
+ }
+
+ ret = 0;
+cleanup:
+ qemuMonitorTestFree(test);
+ return ret;
+}
+
+
static int
mymain(void)
{
@@ -702,6 +749,7 @@ mymain(void)
DO_TEST(GetTPMModels);
DO_TEST(GetCommandLineOptionParameters);
DO_TEST(GetListPaths);
+ DO_TEST(GetObjectProperty);
virObjectUnref(xmlopt);
--
1.8.1.4
9:05 a.m.
New subject: [libvirt] [PATCH v2 02/10] Add qemuMonitorJSONGetObjectProperty() method for QMP qom-get command
On 08.07.2013 21:20, John Ferlan wrote:
Add a new qemuMonitorJSONGetObjectProperty() method to support
invocation
of the 'qom-get' JSON monitor command with a provided path, property, and
expected data type return. The qemuMonitorJSONObjectProperty is similar to
virTypedParameter; however, a future patch will extend it a bit to include
a void pointer to balloon driver statistic data.
The provided test will execute a qom-get on "/machine/i440fx" which will
return a property "realized".
---
src/qemu/qemu_monitor_json.c | 86 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 34 ++++++++++++++++++
tests/qemumonitorjsontest.c | 48 +++++++++++++++++++++++++
3 files changed, 168 insertions(+)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index fc2b65f..db107f1 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4642,6 +4642,92 @@ void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr
paths)
VIR_FREE(paths);
}
+
+int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorJSONObjectPropertyPtr prop)
+{
+ int ret;
+ virJSONValuePtr cmd;
+ virJSONValuePtr reply = NULL;
+ virJSONValuePtr data;
+ const char *tmp;
+
+ if (!(cmd = qemuMonitorJSONMakeCommand("qom-get",
+ "s:path", path,
+ "s:property", property,
+ NULL)))
+ return -1;
+
+ ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+ if (ret == 0)
+ ret = qemuMonitorJSONCheckError(cmd, reply);
+
+ if (ret < 0)
+ goto cleanup;
+
+ ret = -1;
+
+ if (!(data = virJSONValueObjectGet(reply, "return"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("qom-get reply was missing return data"));
+ goto cleanup;
+ }
+
+ switch (prop->type) {
switch ((qemuMonitorJSONObjectPropertyType) prop->type) {
+ /* Simple cases of boolean, int, long, uint, ulong, double, and
string
+ * will receive return value as part of {"return": xxx} statement
+ */
+ case QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN:
+ ret = virJSONValueGetBoolean(data, &prop->val.b);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_INT:
+ ret = virJSONValueGetNumberInt(data, &prop->val.i);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_LONG:
+ ret = virJSONValueGetNumberLong(data, &prop->val.l);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_UINT:
+ ret = virJSONValueGetNumberUint(data, &prop->val.ui);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_ULONG:
+ ret = virJSONValueGetNumberUlong(data, &prop->val.ul);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_DOUBLE:
+ ret = virJSONValueGetNumberDouble(data, &prop->val.d);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_STRING:
+ tmp = virJSONValueGetString(data);
+ if (tmp && VIR_STRDUP(prop->val.str, tmp) < 0)
+ goto cleanup;
+ if (tmp)
+ ret = 0;
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_LAST:
+ default:
Drop 'default' as it prevents compiler to check completeness coverage of enum in
this switch(). The _LAST can be dropped as well among with virReportError() and subsequent
cleanup then.
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("qom-get invalid object property type %d"),
+ prop->type);
+ goto cleanup;
+ }
+
+ if (ret == -1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("qom-get reply was missing return data"));
+ goto cleanup;
+ }
+
+ ret = 0;
+cleanup:
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+
+ return ret;
+}
+
+
int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index e7ce145..e6bbd62 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -348,6 +348,40 @@ int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths);
+/* Flags for the 'type' field in _qemuMonitorJSONObjectProperty */
+typedef enum {
+ QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN=1,
+ QEMU_MONITOR_OBJECT_PROPERTY_INT,
+ QEMU_MONITOR_OBJECT_PROPERTY_LONG,
+ QEMU_MONITOR_OBJECT_PROPERTY_UINT,
+ QEMU_MONITOR_OBJECT_PROPERTY_ULONG,
+ QEMU_MONITOR_OBJECT_PROPERTY_DOUBLE,
+ QEMU_MONITOR_OBJECT_PROPERTY_STRING,
+
+ QEMU_MONITOR_OBJECT_PROPERTY_LAST
+} qemuMonitorJSONObjectPropertyType;
+
+typedef struct _qemuMonitorJSONObjectProperty qemuMonitorJSONObjectProperty;
+typedef qemuMonitorJSONObjectProperty *qemuMonitorJSONObjectPropertyPtr;
+struct _qemuMonitorJSONObjectProperty {
+ int type; /* qemuMonitorJSONObjectPropertyType */
+ union {
+ bool b;
+ int i;
Huh, syntax-check fails at this ^^^ line thinking @i is a loop variable. I guess
you'll need to add an exception to cfg.mk:
diff --git a/cfg.mk b/cfg.mk
index c6a097e..2936280 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -985,4 +985,4 @@ exclude_file_name_regexp--sc_prohibit_include_public_headers_brackets
= \
^(python/|tools/|examples/|include/libvirt/(virterror|libvirt-(qemu|lxc))\.h$$)
exclude_file_name_regexp--sc_prohibit_int_ijk = \
- ^(src/remote_protocol-structs|src/remote/remote_protocol.x|cfg.mk|include/)$
+
^(src/remote_protocol-structs|src/remote/remote_protocol.x|cfg.mk|include/|src/qemu/qemu_monitor_json.h)$
+ long long l;
+ unsigned int ui;
+ unsigned long long ul;
+ double d;
+ char *str;
+ } val;
+};
+
+int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorJSONObjectPropertyPtr prop)
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+
int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
Michal
10:35 a.m.
New subject: [libvirt] [PATCH v2 02/10] Add qemuMonitorJSONGetObjectProperty() method for QMP qom-get command
On 07/11/2013 10:05 AM, Michal Privoznik wrote:
On 08.07.2013 21:20, John Ferlan wrote:
> Add a new qemuMonitorJSONGetObjectProperty() method to support invocation
> of the 'qom-get' JSON monitor command with a provided path, property, and
> expected data type return. The qemuMonitorJSONObjectProperty is similar to
> virTypedParameter; however, a future patch will extend it a bit to include
> a void pointer to balloon driver statistic data.
>
> The provided test will execute a qom-get on "/machine/i440fx" which will
> return a property "realized".
> ---
> src/qemu/qemu_monitor_json.c | 86 ++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor_json.h | 34 ++++++++++++++++++
> tests/qemumonitorjsontest.c | 48 +++++++++++++++++++++++++
> 3 files changed, 168 insertions(+)
>
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index fc2b65f..db107f1 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -4642,6 +4642,92 @@ void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr
paths)
> VIR_FREE(paths);
> }
>
> +
> +int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
> + const char *path,
> + const char *property,
> + qemuMonitorJSONObjectPropertyPtr prop)
> +{
> + int ret;
> + virJSONValuePtr cmd;
> + virJSONValuePtr reply = NULL;
> + virJSONValuePtr data;
> + const char *tmp;
> +
> + if (!(cmd = qemuMonitorJSONMakeCommand("qom-get",
> + "s:path", path,
> + "s:property", property,
> + NULL)))
> + return -1;
> +
> + ret = qemuMonitorJSONCommand(mon, cmd, &reply);
> +
> + if (ret == 0)
> + ret = qemuMonitorJSONCheckError(cmd, reply);
> +
> + if (ret < 0)
> + goto cleanup;
> +
> + ret = -1;
> +
> + if (!(data = virJSONValueObjectGet(reply, "return"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("qom-get reply was missing return data"));
> + goto cleanup;
> + }
> +
> + switch (prop->type) {
switch ((qemuMonitorJSONObjectPropertyType) prop->type) {
> + /* Simple cases of boolean, int, long, uint, ulong, double, and string
> + * will receive return value as part of {"return": xxx} statement
> + */
> + case QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN:
> + ret = virJSONValueGetBoolean(data, &prop->val.b);
> + break;
> + case QEMU_MONITOR_OBJECT_PROPERTY_INT:
> + ret = virJSONValueGetNumberInt(data, &prop->val.i);
> + break;
> + case QEMU_MONITOR_OBJECT_PROPERTY_LONG:
> + ret = virJSONValueGetNumberLong(data, &prop->val.l);
> + break;
> + case QEMU_MONITOR_OBJECT_PROPERTY_UINT:
> + ret = virJSONValueGetNumberUint(data, &prop->val.ui);
> + break;
> + case QEMU_MONITOR_OBJECT_PROPERTY_ULONG:
> + ret = virJSONValueGetNumberUlong(data, &prop->val.ul);
> + break;
> + case QEMU_MONITOR_OBJECT_PROPERTY_DOUBLE:
> + ret = virJSONValueGetNumberDouble(data, &prop->val.d);
> + break;
> + case QEMU_MONITOR_OBJECT_PROPERTY_STRING:
> + tmp = virJSONValueGetString(data);
> + if (tmp && VIR_STRDUP(prop->val.str, tmp) < 0)
> + goto cleanup;
> + if (tmp)
> + ret = 0;
> + break;
> + case QEMU_MONITOR_OBJECT_PROPERTY_LAST:
> + default:
Drop 'default' as it prevents compiler to check completeness coverage of enum in
this switch(). The _LAST can be dropped as well among with virReportError() and subsequent
cleanup then.
Dropping *_LAST and error report/cleanup caused build failure:
CC libvirt_driver_qemu_impl_la-qemu_monitor_json.lo
qemu/qemu_monitor_json.c: In function 'qemuMonitorJSONGetObjectProperty':
qemu/qemu_monitor_json.c:4642:5: error: enumeration value
'QEMU_MONITOR_OBJECT_PROPERTY_LAST' not handled in switch [-Werror=switch]
qemu/qemu_monitor_json.c: At top level:
cc1: error: unrecognized command line option
"-Wno-unused-command-line-argument" [-Werror]
cc1: all warnings being treated as errors
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("qom-get invalid object property type %d"),
> + prop->type);
> + goto cleanup;
> + }
> +
> + if (ret == -1) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("qom-get reply was missing return data"));
> + goto cleanup;
> + }
> +
> + ret = 0;
> +cleanup:
> + virJSONValueFree(cmd);
> + virJSONValueFree(reply);
> +
> + return ret;
> +}
> +
> +
> int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
> const char *type,
> char ***props)
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index e7ce145..e6bbd62 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -348,6 +348,40 @@ int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
>
> void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths);
>
> +/* Flags for the 'type' field in _qemuMonitorJSONObjectProperty */
> +typedef enum {
> + QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN=1,
> + QEMU_MONITOR_OBJECT_PROPERTY_INT,
> + QEMU_MONITOR_OBJECT_PROPERTY_LONG,
> + QEMU_MONITOR_OBJECT_PROPERTY_UINT,
> + QEMU_MONITOR_OBJECT_PROPERTY_ULONG,
> + QEMU_MONITOR_OBJECT_PROPERTY_DOUBLE,
> + QEMU_MONITOR_OBJECT_PROPERTY_STRING,
> +
> + QEMU_MONITOR_OBJECT_PROPERTY_LAST
> +} qemuMonitorJSONObjectPropertyType;
> +
> +typedef struct _qemuMonitorJSONObjectProperty qemuMonitorJSONObjectProperty;
> +typedef qemuMonitorJSONObjectProperty *qemuMonitorJSONObjectPropertyPtr;
> +struct _qemuMonitorJSONObjectProperty {
> + int type; /* qemuMonitorJSONObjectPropertyType */
> + union {
> + bool b;
> + int i;
Huh, syntax-check fails at this ^^^ line thinking @i is a loop variable. I guess
you'll need to add an exception to cfg.mk:
Yep, saw that - made the update today to follow another example to
change "int i;" to "int iv;" rather than messing with the rule.
Other
places with use "val.i" were changed to "val.iv".
See "_virLockManagerParam" in src/locking/lock_driver.h
diff --git a/cfg.mk b/cfg.mk
index c6a097e..2936280 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -985,4 +985,4 @@ exclude_file_name_regexp--sc_prohibit_include_public_headers_brackets
= \
^(python/|tools/|examples/|include/libvirt/(virterror|libvirt-(qemu|lxc))\.h$$)
exclude_file_name_regexp--sc_prohibit_int_ijk = \
- ^(src/remote_protocol-structs|src/remote/remote_protocol.x|cfg.mk|include/)$
+
^(src/remote_protocol-structs|src/remote/remote_protocol.x|cfg.mk|include/|src/qemu/qemu_monitor_json.h)$
> + long long l;
> + unsigned int ui;
> + unsigned long long ul;
> + double d;
> + char *str;
> + } val;
> +};
> +
> +int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
> + const char *path,
> + const char *property,
> + qemuMonitorJSONObjectPropertyPtr prop)
> + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
> +
> int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
> const char *type,
> char ***props)
Michal
10:39 a.m.
New subject: [libvirt] [PATCH v2 02/10] Add qemuMonitorJSONGetObjectProperty() method for QMP qom-get command
On 07/11/2013 08:05 AM, Michal Privoznik wrote:
On 08.07.2013 21:20, John Ferlan wrote:
> Add a new qemuMonitorJSONGetObjectProperty() method to support invocation
> of the 'qom-get' JSON monitor command with a provided path, property, and
> expected data type return. The qemuMonitorJSONObjectProperty is similar to
> virTypedParameter; however, a future patch will extend it a bit to include
> a void pointer to balloon driver statistic data.
>
> The provided test will execute a qom-get on "/machine/i440fx" which will
> return a property "realized".
> ---
> src/qemu/qemu_monitor_json.c | 86 ++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor_json.h | 34 ++++++++++++++++++
Does the new type need to live in the .h file, or can it be internal to
the .c file? That is, I think qemuMonitorJSONGetObjectProperty can be
marked static, since it is only used within the same file, and then all
the types that it uses moved into the .c file as well.
> +struct _qemuMonitorJSONObjectProperty {
> + int type; /* qemuMonitorJSONObjectPropertyType */
> + union {
> + bool b;
> + int i;
Huh, syntax-check fails at this ^^^ line thinking @i is a loop variable. I guess
you'll need to add an exception to cfg.mk:
Or rename the variable 'si'.
diff --git a/cfg.mk b/cfg.mk
index c6a097e..2936280 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -985,4 +985,4 @@ exclude_file_name_regexp--sc_prohibit_include_public_headers_brackets
= \
^(python/|tools/|examples/|include/libvirt/(virterror|libvirt-(qemu|lxc))\.h$$)
exclude_file_name_regexp--sc_prohibit_int_ijk = \
- ^(src/remote_protocol-structs|src/remote/remote_protocol.x|cfg.mk|include/)$
+
^(src/remote_protocol-structs|src/remote/remote_protocol.x|cfg.mk|include/|src/qemu/qemu_monitor_json.h)$
Exempting entire files because of one line seems a bit overkill; maybe
we can instead tweak the cfg.mk rule to have an
exclude='exempt from syntax-check'
line, then place a magic comment on this use (see how sc_prohibit_strtol
uses that mechanism for one-line exemptions instead of entire-file
exemptions).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:48 a.m.
New subject: [libvirt] [PATCH v2 02/10] Add qemuMonitorJSONGetObjectProperty() method for QMP qom-get command
On Mon, Jul 08, 2013 at 03:20:28PM -0400, John Ferlan wrote:
Add a new qemuMonitorJSONGetObjectProperty() method to support
invocation
of the 'qom-get' JSON monitor command with a provided path, property, and
expected data type return. The qemuMonitorJSONObjectProperty is similar to
virTypedParameter; however, a future patch will extend it a bit to include
a void pointer to balloon driver statistic data.
The provided test will execute a qom-get on "/machine/i440fx" which will
return a property "realized".
---
src/qemu/qemu_monitor_json.c | 86 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 34 ++++++++++++++++++
tests/qemumonitorjsontest.c | 48 +++++++++++++++++++++++++
3 files changed, 168 insertions(+)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:20 p.m.
New subject: [libvirt] [PATCH v2 03/10] Add qemuMonitorJSONSetObjectProperty() method for QMP qom-set command
Add a new qemuMonitorJSONSetObjectProperty() method to support invocation
of the 'qom-set' JSON monitor command with a provided path, property, and
expected data type to set.
The test code uses the same "/machine/i440fx" property as the get test and
attempts to set the "realized" property to "true" (which it should be
set
at anyway).
---
src/qemu/qemu_monitor_json.c | 62 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 6 +++++
tests/qemumonitorjsontest.c | 59 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 127 insertions(+)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index db107f1..9487955 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4728,6 +4728,68 @@ cleanup:
}
+#define MAKE_SET_CMD(STRING, VALUE) \
+ cmd = qemuMonitorJSONMakeCommand("qom-set", \
+ "s:path", path, \
+ "s:property", property, \
+ STRING, VALUE, \
+ NULL)
+int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorJSONObjectPropertyPtr prop)
+{
+ int ret = -1;
+ virJSONValuePtr cmd = NULL;
+ virJSONValuePtr reply = NULL;
+
+ switch (prop->type) {
+ /* Simple cases of boolean, int, long, uint, ulong, double, and string
+ * will receive return value as part of {"return": xxx} statement
+ */
+ case QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN:
+ MAKE_SET_CMD("b:value", prop->val.b);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_INT:
+ MAKE_SET_CMD("i:value", prop->val.i);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_LONG:
+ MAKE_SET_CMD("I:value", prop->val.l);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_UINT:
+ MAKE_SET_CMD("u:value", prop->val.ui);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_ULONG:
+ MAKE_SET_CMD("U:value", prop->val.ul);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_DOUBLE:
+ MAKE_SET_CMD("d:value", prop->val.d);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_STRING:
+ MAKE_SET_CMD("s:value", prop->val.str);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_LAST:
+ default:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("qom-set invalid object property type %d"),
+ prop->type);
+ goto cleanup;
+
+ }
+ if (!cmd)
+ return -1;
+
+ if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) == 0)
+ ret = qemuMonitorJSONCheckError(cmd, reply);
+
+cleanup:
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+
+ return ret;
+}
+
+
int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index e6bbd62..fd0dedd 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -382,6 +382,12 @@ int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
qemuMonitorJSONObjectPropertyPtr prop)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorJSONObjectPropertyPtr prop)
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+
int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 2c57211..3e1b720 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -720,6 +720,64 @@ cleanup:
}
+/*
+ * This test will use a path to /machine/i440fx which should exist in order
+ * to ensure that the qom-set property set works properly. The test will
+ * set a true property to true just as a proof of concept. Setting it to
+ * false is not a good idea...
+ */
+static int
+testQemuMonitorJSONSetObjectProperty(const void *data)
+{
+ const virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+ qemuMonitorTestPtr test = qemuMonitorTestNew(true, xmlopt);
+ int ret = -1;
+ qemuMonitorJSONObjectProperty prop;
+
+ if (!test)
+ return -1;
+
+ if (qemuMonitorTestAddItem(test, "qom-set",
+ "{ \"return\": {} }") < 0)
+ goto cleanup;
+ if (qemuMonitorTestAddItem(test, "qom-get",
+ "{ \"return\": true }") < 0)
+ goto cleanup;
+
+ /* Let's attempt the setting */
+ memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN;
+ prop.val.b = true;
+ if (qemuMonitorJSONSetObjectProperty(qemuMonitorTestGetMonitor(test),
+ "/machine/i440fx",
+ "realized",
+ &prop) < 0)
+ goto cleanup;
+
+ /* To make sure it worked, fetch the property - if this succeeds then
+ * we didn't hose things
+ */
+ memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN;
+ if (qemuMonitorJSONGetObjectProperty(qemuMonitorTestGetMonitor(test),
+ "/machine/i440fx",
+ "realized",
+ &prop) < 0)
+ goto cleanup;
+
+ if (!prop.val.b) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ "expected true, but false returned");
+ goto cleanup;
+ }
+
+ ret = 0;
+cleanup:
+ qemuMonitorTestFree(test);
+ return ret;
+}
+
+
static int
mymain(void)
{
@@ -750,6 +808,7 @@ mymain(void)
DO_TEST(GetCommandLineOptionParameters);
DO_TEST(GetListPaths);
DO_TEST(GetObjectProperty);
+ DO_TEST(SetObjectProperty);
virObjectUnref(xmlopt);
--
1.8.1.4
9:05 a.m.
New subject: [libvirt] [PATCH v2 03/10] Add qemuMonitorJSONSetObjectProperty() method for QMP qom-set command
On 08.07.2013 21:20, John Ferlan wrote:
Add a new qemuMonitorJSONSetObjectProperty() method to support
invocation
of the 'qom-set' JSON monitor command with a provided path, property, and
expected data type to set.
The test code uses the same "/machine/i440fx" property as the get test and
attempts to set the "realized" property to "true" (which it should be
set
at anyway).
---
src/qemu/qemu_monitor_json.c | 62 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 6 +++++
tests/qemumonitorjsontest.c | 59 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 127 insertions(+)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index db107f1..9487955 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4728,6 +4728,68 @@ cleanup:
}
+#define MAKE_SET_CMD(STRING, VALUE) \
+ cmd = qemuMonitorJSONMakeCommand("qom-set", \
+ "s:path", path, \
+ "s:property", property, \
+ STRING, VALUE, \
+ NULL)
+int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorJSONObjectPropertyPtr prop)
+{
+ int ret = -1;
+ virJSONValuePtr cmd = NULL;
+ virJSONValuePtr reply = NULL;
+
+ switch (prop->type) {
Again, this should be:
switch ((qemuMonitorJSONObjectPropertyType) prop->type) {
+ /* Simple cases of boolean, int, long, uint, ulong, double, and
string
+ * will receive return value as part of {"return": xxx} statement
+ */
+ case QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN:
+ MAKE_SET_CMD("b:value", prop->val.b);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_INT:
+ MAKE_SET_CMD("i:value", prop->val.i);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_LONG:
+ MAKE_SET_CMD("I:value", prop->val.l);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_UINT:
+ MAKE_SET_CMD("u:value", prop->val.ui);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_ULONG:
+ MAKE_SET_CMD("U:value", prop->val.ul);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_DOUBLE:
+ MAKE_SET_CMD("d:value", prop->val.d);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_STRING:
+ MAKE_SET_CMD("s:value", prop->val.str);
+ break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_LAST:
+ default:
With these removed.
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("qom-set invalid object property type %d"),
+ prop->type);
+ goto cleanup;
+
+ }
+ if (!cmd)
+ return -1;
+
+ if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) == 0)
+ ret = qemuMonitorJSONCheckError(cmd, reply);
+
+cleanup:
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+
+ return ret;
+}
+
+
int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
Michal
11:15 a.m.
New subject: [libvirt] [PATCH v2 03/10] Add qemuMonitorJSONSetObjectProperty() method for QMP qom-set command
On 07/11/2013 08:05 AM, Michal Privoznik wrote:
On 08.07.2013 21:20, John Ferlan wrote:
> Add a new qemuMonitorJSONSetObjectProperty() method to support invocation
> of the 'qom-set' JSON monitor command with a provided path, property, and
> expected data type to set.
>
> The test code uses the same "/machine/i440fx" property as the get test and
> attempts to set the "realized" property to "true" (which it
should be set
> at anyway).
> ---
> + switch (prop->type) {
Again, this should be:
switch ((qemuMonitorJSONObjectPropertyType) prop->type) {
> + case QEMU_MONITOR_OBJECT_PROPERTY_STRING:
> + MAKE_SET_CMD("s:value", prop->val.str);
> + break;
> + case QEMU_MONITOR_OBJECT_PROPERTY_LAST:
> + default:
With these removed.
For the gcc enforcement of complete coverage of the enum, you have to
list the _LAST but drop the default:.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:49 a.m.
New subject: [libvirt] [PATCH v2 03/10] Add qemuMonitorJSONSetObjectProperty() method for QMP qom-set command
On Mon, Jul 08, 2013 at 03:20:29PM -0400, John Ferlan wrote:
Add a new qemuMonitorJSONSetObjectProperty() method to support
invocation
of the 'qom-set' JSON monitor command with a provided path, property, and
expected data type to set.
The test code uses the same "/machine/i440fx" property as the get test and
attempts to set the "realized" property to "true" (which it should be
set
at anyway).
---
src/qemu/qemu_monitor_json.c | 62 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 6 +++++
tests/qemumonitorjsontest.c | 59 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 127 insertions(+)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:20 p.m.
New subject: [libvirt] [PATCH v2 04/10] Add 'period' for Memballoon statistics gathering capability
Add a period in seconds to allow/enable statistics gathering from the
Balloon driver for 'virsh dommemstat <domain>'.
---
docs/formatdomain.html.in | 10 ++++++++++
docs/schemas/domaincommon.rng | 7 +++++++
src/conf/domain_conf.c | 44 +++++++++++++++++++++++++++++++++++++------
src/conf/domain_conf.h | 1 +
4 files changed, 56 insertions(+), 6 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 52a6353..93d2416 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4637,6 +4637,7 @@ qemu-kvm -net nic,model=? /dev/null
<devices>
<memballoon model='virtio'>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x02' function='0x0'/>
+ <stats period='10'/>
</memballoon>
</devices>
</domain></pre>
@@ -4654,6 +4655,15 @@ qemu-kvm -net nic,model=? /dev/null
<li>'xen' — default with Xen</li>
</ul>
</dd>
+ <dt><code>period</code></dt>
+ <dd>
+ <p>
+ The optional <code>period</code> allows the QEMU virtio memory
+ balloon driver to provide statistics through the <code>virsh
+ dommemstat [domain]</code> command.
+ <span class='since'>Since 1.1.1, requires QEMU 1.5</span>
+ </p>
+ </dd>
</dl>
<h4><a name="elementsRng">Random number generator
device</a></h4>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index c135530..7a6852b 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2946,6 +2946,13 @@
<optional>
<ref name="address"/>
</optional>
+ <optional>
+ <element name="stats">
+ <attribute name="period">
+ <ref name="positiveInteger"/>
+ </attribute>
+ </element>
+ </optional>
</element>
</define>
<define name="parallel">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 402e6e9..d63b735 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8595,10 +8595,14 @@ error:
static virDomainMemballoonDefPtr
virDomainMemballoonDefParseXML(const xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
unsigned int flags)
{
char *model;
virDomainMemballoonDefPtr def;
+ xmlNodePtr save = ctxt->node;
+ int period = -1;
+ int ret;
if (VIR_ALLOC(def) < 0) {
virReportOOMError();
@@ -8607,22 +8611,39 @@ virDomainMemballoonDefParseXML(const xmlNodePtr node,
model = virXMLPropString(node, "model");
if (model == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ virReportError(VIR_ERR_XML_ERROR, "%s",
_("balloon memory must contain model name"));
goto error;
}
+
if ((def->model = virDomainMemballoonModelTypeFromString(model)) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
+ virReportError(VIR_ERR_XML_ERROR,
_("unknown memory balloon model '%s'"), model);
goto error;
}
+ ctxt->node = node;
+ ret = virXPathInt("string(./stats/@period)", ctxt, &period);
+ if (ret == -2) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("period value not a valid integer"));
+ goto error;
+ } else if (ret == 0) {
+ if (period < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("period value must be 0 or more"));
+ goto error;
+ }
+ def->period = period;
+ }
+
if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
goto error;
cleanup:
VIR_FREE(model);
+ ctxt->node = save;
return def;
error:
@@ -11991,7 +12012,7 @@ virDomainDefParseXML(xmlDocPtr xml,
}
if (n > 0) {
virDomainMemballoonDefPtr memballoon =
- virDomainMemballoonDefParseXML(nodes[0], flags);
+ virDomainMemballoonDefParseXML(nodes[0], ctxt, flags);
if (!memballoon)
goto error;
@@ -15192,6 +15213,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
unsigned int flags)
{
const char *model = virDomainMemballoonModelTypeToString(def->model);
+ bool noopts = true;
if (!model) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -15205,11 +15227,21 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
virBufferAddLit(buf, ">\n");
if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
return -1;
- virBufferAddLit(buf, " </memballoon>\n");
- } else {
- virBufferAddLit(buf, "/>\n");
+ noopts = false;
}
+ if (def->period) {
+ if (noopts)
+ virBufferAddLit(buf, ">\n");
+ virBufferAsprintf(buf, " <stats period='%d'/>\n",
def->period);
+ noopts = false;
+ }
+
+ if (noopts)
+ virBufferAddLit(buf, "/>\n");
+ else
+ virBufferAddLit(buf, " </memballoon>\n");
+
return 0;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index da83eb6..a8eb1fe 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1533,6 +1533,7 @@ enum {
struct _virDomainMemballoonDef {
int model;
virDomainDeviceInfo info;
+ int period; /* seconds between collections */
};
struct _virDomainNVRAMDef {
--
1.8.1.4
9:29 a.m.
New subject: [libvirt] [PATCH v2 04/10] Add 'period' for Memballoon statistics gathering capability
On Mon, Jul 08, 2013 at 03:20:30PM -0400, John Ferlan wrote:
Add a period in seconds to allow/enable statistics gathering from
the
Balloon driver for 'virsh dommemstat <domain>'.
---
docs/formatdomain.html.in | 10 ++++++++++
docs/schemas/domaincommon.rng | 7 +++++++
src/conf/domain_conf.c | 44 +++++++++++++++++++++++++++++++++++++------
src/conf/domain_conf.h | 1 +
4 files changed, 56 insertions(+), 6 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index c135530..7a6852b 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2946,6 +2946,13 @@
<optional>
<ref name="address"/>
</optional>
+ <optional>
+ <element name="stats">
+ <attribute name="period">
+ <ref name="positiveInteger"/>
This says 'unsigned int'
+ </attribute>
+ </element>
+ </optional>
</element>
</define>
<define name="parallel">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 402e6e9..d63b735 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
+ if (period < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("period value must be 0 or more"));
+ goto error;
+ }
And this says unsigned int.
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index da83eb6..a8eb1fe 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1533,6 +1533,7 @@ enum {
struct _virDomainMemballoonDef {
int model;
virDomainDeviceInfo info;
+ int period; /* seconds between collections */
But this is signed. Please change to be unsigned.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:20 p.m.
New subject: [libvirt] [PATCH v2 05/10] Determine whether to start balloon memory stats gathering.
At vm startup, reconnect, and attach - check for the presence of the balloon
driver and save the path in the private area of the driver. This path will
remain constant throughout the life of the domain and can then be used rather
than attempting to find the path each time balloon driver statistics are
fetched or the collection period changes. The qom object model model requires
setting object properties after device startup. That is, it's not possible
to pass the period along via the startup code as it won't be recognized.
If a balloon driver path is found a check of the existing collection period
will be made against the saved domain value in order to determine if an
adjustment needs to be made to the period to start or stop collecting stats
---
src/qemu/qemu_monitor.c | 130 +++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor.h | 2 +
src/qemu/qemu_monitor_json.c | 42 ++++++++++++++
src/qemu/qemu_monitor_json.h | 3 +
src/qemu/qemu_process.c | 21 ++++++-
5 files changed, 196 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index fe5ab0a..038c9e8 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -83,6 +83,9 @@ struct _qemuMonitor {
/* cache of query-command-line-options results */
virJSONValuePtr options;
+
+ /* If found, path to the virtio memballoon driver */
+ char *balloonpath;
};
static virClassPtr qemuMonitorClass;
@@ -248,6 +251,7 @@ static void qemuMonitorDispose(void *obj)
virCondDestroy(&mon->notify);
VIR_FREE(mon->buffer);
virJSONValueFree(mon->options);
+ VIR_FREE(mon->balloonpath);
}
@@ -929,6 +933,107 @@ qemuMonitorSetOptions(qemuMonitorPtr mon, virJSONValuePtr options)
mon->options = options;
}
+/* Search the qom objects for the balloon driver object by it's known name
+ * of "virtio-balloon-pci". The entry for the driver will be found in the
+ * returned 'type' field using the syntax
"child<virtio-balloon-pci>".
+ *
+ * Once found, check the entry to ensure it has the correct property listed.
+ * If it does not, then obtaining statistics from qemu will not be possible.
+ * This feature was added to qemu 1.5.
+ *
+ * This procedure will be call recursively until found or the qom-list is
+ * exhausted.
+ *
+ * Returns:
+ *
+ * 1 - Found
+ * 0 - Not found still looking
+ * -1 - Error bail out
+ *
+ * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
+ */
+static int
+qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
+ virDomainObjPtr vm,
+ const char *curpath)
+{
+ int i,j;
+ int npaths = 0;
+ int nprops = 0;
+ int ret = 0;
+ char *nextpath = NULL;
+ qemuMonitorJSONListPathPtr *paths = NULL;
+ qemuMonitorJSONListPathPtr *bprops = NULL;
+
+ /* Not supported */
+ if (vm->def->memballoon &&
+ vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
+ VIR_DEBUG("Model must be virtio to get memballoon path");
+ return -1;
+ }
+
+ /* Already set and won't change */
+ if (mon->balloonpath)
+ return 1;
+
+ VIR_DEBUG("Searching for Balloon Object Path starting at %s", curpath);
+
+ npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, &paths);
+
+ for (i = 0; i < npaths && ret == 0; i++) {
+
+ if (STREQ_NULLABLE(paths[i]->type,
"link<virtio-balloon-pci>")) {
+ VIR_DEBUG("Path to <virtio-balloon-pci> is '%s/%s'",
+ curpath, paths[i]->name);
+ if (virAsprintf(&nextpath, "%s/%s", curpath, paths[i]->name)
< 0) {
+ ret = -1;
+ goto cleanup;
+ }
+
+ /* Now look at the each of the property entries to determine
+ * whether "guest-stats-polling-interval" exists. If not,
+ * then this version of qemu/kvm does not support the feature.
+ */
+ nprops = qemuMonitorJSONGetObjectListPaths(mon, nextpath, &bprops);
+ for (j = 0; j < nprops; j++) {
+ if (STREQ(bprops[j]->name, "guest-stats-polling-interval"))
{
+ mon->balloonpath = nextpath;
+ nextpath = NULL;
+ ret = 1;
+ goto cleanup;
+ }
+ }
+
+ /* If we get here, we found the path, but not the property */
+ VIR_DEBUG("Property 'guest-stats-polling-interval' not
found");
+ ret = -1;
+ goto cleanup;
+ }
+
+ /* Type entries that begin with "child<" are a branch that can be
+ * traversed looking for more entries
+ */
+ if (paths[i]->type && STRPREFIX(paths[i]->type,
"child<")) {
+ if (virAsprintf(&nextpath, "%s/%s", curpath, paths[i]->name)
< 0) {
+ virReportOOMError();
+ ret = -1;
+ goto cleanup;
+ }
+ ret = qemuMonitorFindBalloonObjectPath(mon, vm, nextpath);
+ }
+ }
+
+cleanup:
+ for (i = 0; i < npaths; i++)
+ qemuMonitorJSONListPathFree(paths[i]);
+ VIR_FREE(paths);
+ for (j = 0; j < nprops; j++)
+ qemuMonitorJSONListPathFree(bprops[j]);
+ VIR_FREE(bprops);
+ VIR_FREE(nextpath);
+ return ret;
+}
+
int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon,
const char *cmd,
int scm_fd,
@@ -1390,6 +1495,31 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
return ret;
}
+int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
+ int period)
+{
+ int ret = -1;
+ VIR_DEBUG("mon=%p period=%d", mon, period);
+
+ if (!mon) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
+
+ if (!mon->json) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("JSON monitor is required"));
+ return -1;
+ }
+
+ if (qemuMonitorFindBalloonObjectPath(mon, mon->vm, "/") == 1) {
+ ret = qemuMonitorJSONSetMemoryStatsPeriod(mon, mon->balloonpath,
+ period);
+ }
+ return ret;
+}
+
int
qemuMonitorBlockIOStatusToError(const char *status)
{
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 78011ee..12b730a 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -265,6 +265,8 @@ int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon,
int qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
virDomainMemoryStatPtr stats,
unsigned int nr_stats);
+int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
+ int period);
int qemuMonitorBlockIOStatusToError(const char *status);
virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 9487955..2d7f9b6 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1495,6 +1495,48 @@ cleanup:
}
+/*
+ * Using the provided balloonpath, determine if we need to set the
+ * collection interval property to enable statistics gathering.
+ */
+int
+qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon,
+ char *balloonpath,
+ int period)
+{
+ qemuMonitorJSONObjectProperty prop;
+
+ /* Get the current value of the stats polling interval */
+ memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
+ if (qemuMonitorJSONGetObjectProperty(mon, balloonpath,
+ "guest-stats-polling-interval",
+ &prop) < 0) {
+ VIR_DEBUG("Failed to get polling interval for balloon driver");
+ return 0;
+ }
+
+ VIR_DEBUG("memballoon period=%d
'guest-stats-polling-interval'=%d",
+ period, prop.val.i);
+
+ /* Same value - no need to set */
+ if (period == prop.val.i)
+ return 0;
+
+ /* Set to the value in memballoon (could enable or disable) */
+ memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
+ prop.val.i = period;
+ if (qemuMonitorJSONSetObjectProperty(mon, balloonpath,
+ "guest-stats-polling-interval",
+ &prop) < 0) {
+ VIR_DEBUG("Failed to set polling interval for balloon driver");
+ return -1;
+ }
+ return 0;
+}
+
+
int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
virHashTablePtr table)
{
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index fd0dedd..b0068ff 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -61,6 +61,9 @@ int qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon,
int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
virDomainMemoryStatPtr stats,
unsigned int nr_stats);
+int qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon,
+ char *balloonpath,
+ int period);
int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
virHashTablePtr table);
int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f8c652f..fd3b7a8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3091,6 +3091,13 @@ qemuProcessReconnect(void *opaque)
if (qemuProcessRecoverJob(driver, obj, conn, &oldjob) < 0)
goto error;
+ if (obj->def->memballoon) {
+ qemuDomainObjEnterMonitor(driver, obj);
+ qemuMonitorSetMemoryStatsPeriod(priv->mon,
+ obj->def->memballoon->period);
+ qemuDomainObjExitMonitor(driver, obj);
+ }
+
/* update domain state XML with possibly updated state in virDomainObj */
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, obj) < 0)
goto error;
@@ -3910,6 +3917,9 @@ int qemuProcessStart(virConnectPtr conn,
goto cleanup;
}
qemuDomainObjEnterMonitor(driver, vm);
+ if (vm->def->memballoon)
+ qemuMonitorSetMemoryStatsPeriod(priv->mon,
+ vm->def->memballoon->period);
if (qemuMonitorSetBalloon(priv->mon, cur_balloon) < 0) {
qemuDomainObjExitMonitor(driver, vm);
goto cleanup;
@@ -4439,11 +4449,18 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
if (!virDomainObjIsActive(vm))
goto cleanup;
- if (running)
+ if (running) {
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
VIR_DOMAIN_RUNNING_UNPAUSED);
- else
+ if (vm->def->memballoon) {
+ qemuDomainObjEnterMonitor(driver, vm);
+ qemuMonitorSetMemoryStatsPeriod(priv->mon,
+ vm->def->memballoon->period);
+ qemuDomainObjExitMonitor(driver, vm);
+ }
+ } else {
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, reason);
+ }
VIR_DEBUG("Writing domain status to disk");
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
--
1.8.1.4
9:05 a.m.
New subject: [libvirt] [PATCH v2 05/10] Determine whether to start balloon memory stats gathering.
On 08.07.2013 21:20, John Ferlan wrote:
At vm startup, reconnect, and attach - check for the presence of the
balloon
driver and save the path in the private area of the driver. This path will
remain constant throughout the life of the domain and can then be used rather
than attempting to find the path each time balloon driver statistics are
fetched or the collection period changes. The qom object model model requires
setting object properties after device startup. That is, it's not possible
to pass the period along via the startup code as it won't be recognized.
If a balloon driver path is found a check of the existing collection period
will be made against the saved domain value in order to determine if an
adjustment needs to be made to the period to start or stop collecting stats
---
src/qemu/qemu_monitor.c | 130 +++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor.h | 2 +
src/qemu/qemu_monitor_json.c | 42 ++++++++++++++
src/qemu/qemu_monitor_json.h | 3 +
src/qemu/qemu_process.c | 21 ++++++-
5 files changed, 196 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index fe5ab0a..038c9e8 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -83,6 +83,9 @@ struct _qemuMonitor {
/* cache of query-command-line-options results */
virJSONValuePtr options;
+
+ /* If found, path to the virtio memballoon driver */
+ char *balloonpath;
};
static virClassPtr qemuMonitorClass;
@@ -248,6 +251,7 @@ static void qemuMonitorDispose(void *obj)
virCondDestroy(&mon->notify);
VIR_FREE(mon->buffer);
virJSONValueFree(mon->options);
+ VIR_FREE(mon->balloonpath);
}
@@ -929,6 +933,107 @@ qemuMonitorSetOptions(qemuMonitorPtr mon, virJSONValuePtr options)
mon->options = options;
}
+/* Search the qom objects for the balloon driver object by it's known name
+ * of "virtio-balloon-pci". The entry for the driver will be found in the
+ * returned 'type' field using the syntax
"child<virtio-balloon-pci>".
+ *
+ * Once found, check the entry to ensure it has the correct property listed.
+ * If it does not, then obtaining statistics from qemu will not be possible.
+ * This feature was added to qemu 1.5.
+ *
+ * This procedure will be call recursively until found or the qom-list is
+ * exhausted.
+ *
+ * Returns:
+ *
+ * 1 - Found
+ * 0 - Not found still looking
+ * -1 - Error bail out
+ *
+ * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
+ */
+static int
+qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
+ virDomainObjPtr vm,
+ const char *curpath)
+{
+ int i,j;
size_t i,j, npaths = 0, nprops = 0;
+ int npaths = 0;
+ int nprops = 0;
+ int ret = 0;
+ char *nextpath = NULL;
+ qemuMonitorJSONListPathPtr *paths = NULL;
+ qemuMonitorJSONListPathPtr *bprops = NULL;
+
+ /* Not supported */
+ if (vm->def->memballoon &&
+ vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
+ VIR_DEBUG("Model must be virtio to get memballoon path");
+ return -1;
+ }
+
+ /* Already set and won't change */
+ if (mon->balloonpath)
+ return 1;
+
+ VIR_DEBUG("Searching for Balloon Object Path starting at %s", curpath);
+
+ npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, &paths);
+
+ for (i = 0; i < npaths && ret == 0; i++) {
+
+ if (STREQ_NULLABLE(paths[i]->type,
"link<virtio-balloon-pci>")) {
+ VIR_DEBUG("Path to <virtio-balloon-pci> is
'%s/%s'",
+ curpath, paths[i]->name);
+ if (virAsprintf(&nextpath, "%s/%s", curpath,
paths[i]->name) < 0) {
+ ret = -1;
+ goto cleanup;
+ }
+
+ /* Now look at the each of the property entries to determine
+ * whether "guest-stats-polling-interval" exists. If not,
+ * then this version of qemu/kvm does not support the feature.
+ */
+ nprops = qemuMonitorJSONGetObjectListPaths(mon, nextpath, &bprops);
+ for (j = 0; j < nprops; j++) {
+ if (STREQ(bprops[j]->name, "guest-stats-polling-interval"))
{
+ mon->balloonpath = nextpath;
+ nextpath = NULL;
+ ret = 1;
+ goto cleanup;
Maybe I'd add VIR_DEBUG() here to log the path.
+ }
+ }
+
+ /* If we get here, we found the path, but not the property */
+ VIR_DEBUG("Property 'guest-stats-polling-interval' not
found");
+ ret = -1;
+ goto cleanup;
+ }
+
+ /* Type entries that begin with "child<" are a branch that can be
+ * traversed looking for more entries
+ */
+ if (paths[i]->type && STRPREFIX(paths[i]->type,
"child<")) {
+ if (virAsprintf(&nextpath, "%s/%s", curpath,
paths[i]->name) < 0) {
+ virReportOOMError();
+ ret = -1;
+ goto cleanup;
+ }
+ ret = qemuMonitorFindBalloonObjectPath(mon, vm, nextpath);
+ }
+ }
+
+cleanup:
+ for (i = 0; i < npaths; i++)
+ qemuMonitorJSONListPathFree(paths[i]);
+ VIR_FREE(paths);
+ for (j = 0; j < nprops; j++)
+ qemuMonitorJSONListPathFree(bprops[j]);
+ VIR_FREE(bprops);
+ VIR_FREE(nextpath);
+ return ret;
+}
+
int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon,
const char *cmd,
int scm_fd,
@@ -1390,6 +1495,31 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
return ret;
}
+int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
+ int period)
+{
+ int ret = -1;
+ VIR_DEBUG("mon=%p period=%d", mon, period);
+
+ if (!mon) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+ return -1;
+ }
+
+ if (!mon->json) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("JSON monitor is required"));
+ return -1;
+ }
+
+ if (qemuMonitorFindBalloonObjectPath(mon, mon->vm, "/") == 1) {
+ ret = qemuMonitorJSONSetMemoryStatsPeriod(mon, mon->balloonpath,
+ period);
+ }
+ return ret;
+}
+
int
qemuMonitorBlockIOStatusToError(const char *status)
{
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 78011ee..12b730a 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -265,6 +265,8 @@ int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon,
int qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
virDomainMemoryStatPtr stats,
unsigned int nr_stats);
+int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
+ int period);
int qemuMonitorBlockIOStatusToError(const char *status);
virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 9487955..2d7f9b6 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1495,6 +1495,48 @@ cleanup:
}
+/*
+ * Using the provided balloonpath, determine if we need to set the
s/balloonpath/balloon path/
+ * collection interval property to enable statistics gathering.
+ */
+int
+qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon,
+ char *balloonpath,
+ int period)
+{
+ qemuMonitorJSONObjectProperty prop;
+
+ /* Get the current value of the stats polling interval */
+ memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
+ if (qemuMonitorJSONGetObjectProperty(mon, balloonpath,
+ "guest-stats-polling-interval",
+ &prop) < 0) {
+ VIR_DEBUG("Failed to get polling interval for balloon driver");
+ return 0;
Shouldn't we return -1 here? I mean, if caller requires us to set an
interval, we should error out if we fail. Why are we querying for the
interval prior setting it anyway?
+ }
+
+ VIR_DEBUG("memballoon period=%d
'guest-stats-polling-interval'=%d",
+ period, prop.val.i);
+
+ /* Same value - no need to set */
+ if (period == prop.val.i)
+ return 0;
+
+ /* Set to the value in memballoon (could enable or disable) */
+ memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
+ prop.val.i = period;
+ if (qemuMonitorJSONSetObjectProperty(mon, balloonpath,
+ "guest-stats-polling-interval",
+ &prop) < 0) {
+ VIR_DEBUG("Failed to set polling interval for balloon driver");
This is unnecessary as long as qemuMonitorJSONSetObjectProperty reports
error on retval < 0, which it does.
+ return -1;
+ }
+ return 0;
+}
+
+
int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
virHashTablePtr table)
{
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index fd0dedd..b0068ff 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -61,6 +61,9 @@ int qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon,
int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
virDomainMemoryStatPtr stats,
unsigned int nr_stats);
+int qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon,
+ char *balloonpath,
+ int period);
int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
virHashTablePtr table);
int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f8c652f..fd3b7a8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3091,6 +3091,13 @@ qemuProcessReconnect(void *opaque)
if (qemuProcessRecoverJob(driver, obj, conn, &oldjob) < 0)
goto error;
+ if (obj->def->memballoon) {
+ qemuDomainObjEnterMonitor(driver, obj);
+ qemuMonitorSetMemoryStatsPeriod(priv->mon,
+ obj->def->memballoon->period);
+ qemuDomainObjExitMonitor(driver, obj);
+ }
+
Why do we want to enable this on ProcessReconnect?
/* update domain state XML with possibly updated state in
virDomainObj */
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, obj) < 0)
goto error;
@@ -3910,6 +3917,9 @@ int qemuProcessStart(virConnectPtr conn,
goto cleanup;
}
qemuDomainObjEnterMonitor(driver, vm);
+ if (vm->def->memballoon)
+ qemuMonitorSetMemoryStatsPeriod(priv->mon,
+ vm->def->memballoon->period);
It makes sense here.
if (qemuMonitorSetBalloon(priv->mon, cur_balloon) < 0) {
qemuDomainObjExitMonitor(driver, vm);
goto cleanup;
@@ -4439,11 +4449,18 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
if (!virDomainObjIsActive(vm))
goto cleanup;
- if (running)
+ if (running) {
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
VIR_DOMAIN_RUNNING_UNPAUSED);
- else
+ if (vm->def->memballoon) {
+ qemuDomainObjEnterMonitor(driver, vm);
+ qemuMonitorSetMemoryStatsPeriod(priv->mon,
+ vm->def->memballoon->period);
+ qemuDomainObjExitMonitor(driver, vm);
+ }
+ } else {
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, reason);
+ }
VIR_DEBUG("Writing domain status to disk");
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
Michal
11:28 a.m.
New subject: [libvirt] [PATCH v2 05/10] Determine whether to start balloon memory stats gathering.
On 07/11/2013 10:05 AM, Michal Privoznik wrote:
On 08.07.2013 21:20, John Ferlan wrote:
> At vm startup, reconnect, and attach - check for the presence of the balloon
> driver and save the path in the private area of the driver. This path will
> remain constant throughout the life of the domain and can then be used rather
> than attempting to find the path each time balloon driver statistics are
> fetched or the collection period changes. The qom object model model requires
> setting object properties after device startup. That is, it's not possible
> to pass the period along via the startup code as it won't be recognized.
> If a balloon driver path is found a check of the existing collection period
> will be made against the saved domain value in order to determine if an
> adjustment needs to be made to the period to start or stop collecting stats
> ---
> src/qemu/qemu_monitor.c | 130 +++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor.h | 2 +
> src/qemu/qemu_monitor_json.c | 42 ++++++++++++++
> src/qemu/qemu_monitor_json.h | 3 +
> src/qemu/qemu_process.c | 21 ++++++-
> 5 files changed, 196 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index fe5ab0a..038c9e8 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -83,6 +83,9 @@ struct _qemuMonitor {
>
> /* cache of query-command-line-options results */
> virJSONValuePtr options;
> +
> + /* If found, path to the virtio memballoon driver */
> + char *balloonpath;
> };
>
> static virClassPtr qemuMonitorClass;
> @@ -248,6 +251,7 @@ static void qemuMonitorDispose(void *obj)
> virCondDestroy(&mon->notify);
> VIR_FREE(mon->buffer);
> virJSONValueFree(mon->options);
> + VIR_FREE(mon->balloonpath);
> }
>
>
> @@ -929,6 +933,107 @@ qemuMonitorSetOptions(qemuMonitorPtr mon, virJSONValuePtr
options)
> mon->options = options;
> }
>
> +/* Search the qom objects for the balloon driver object by it's known name
> + * of "virtio-balloon-pci". The entry for the driver will be found in
the
> + * returned 'type' field using the syntax
"child<virtio-balloon-pci>".
> + *
> + * Once found, check the entry to ensure it has the correct property listed.
> + * If it does not, then obtaining statistics from qemu will not be possible.
> + * This feature was added to qemu 1.5.
> + *
> + * This procedure will be call recursively until found or the qom-list is
> + * exhausted.
> + *
> + * Returns:
> + *
> + * 1 - Found
> + * 0 - Not found still looking
> + * -1 - Error bail out
> + *
> + * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
> + */
> +static int
> +qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
> + virDomainObjPtr vm,
> + const char *curpath)
> +{
> + int i,j;
size_t i,j, npaths = 0, nprops = 0;
yep...
> + int npaths = 0;
> + int nprops = 0;
> + int ret = 0;
> + char *nextpath = NULL;
> + qemuMonitorJSONListPathPtr *paths = NULL;
> + qemuMonitorJSONListPathPtr *bprops = NULL;
> +
> + /* Not supported */
> + if (vm->def->memballoon &&
> + vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO)
{
> + VIR_DEBUG("Model must be virtio to get memballoon path");
> + return -1;
> + }
> +
> + /* Already set and won't change */
> + if (mon->balloonpath)
> + return 1;
> +
> + VIR_DEBUG("Searching for Balloon Object Path starting at %s",
curpath);
> +
> + npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, &paths);
> +
> + for (i = 0; i < npaths && ret == 0; i++) {
> +
> + if (STREQ_NULLABLE(paths[i]->type,
"link<virtio-balloon-pci>")) {
> + VIR_DEBUG("Path to <virtio-balloon-pci> is
'%s/%s'",
> + curpath, paths[i]->name);
> + if (virAsprintf(&nextpath, "%s/%s", curpath,
paths[i]->name) < 0) {
> + ret = -1;
> + goto cleanup;
> + }
> +
> + /* Now look at the each of the property entries to determine
> + * whether "guest-stats-polling-interval" exists. If not,
> + * then this version of qemu/kvm does not support the feature.
> + */
> + nprops = qemuMonitorJSONGetObjectListPaths(mon, nextpath, &bprops);
> + for (j = 0; j < nprops; j++) {
> + if (STREQ(bprops[j]->name,
"guest-stats-polling-interval")) {
> + mon->balloonpath = nextpath;
> + nextpath = NULL;
> + ret = 1;
> + goto cleanup;
Maybe I'd add VIR_DEBUG() here to log the path.
Funny - I did have one at one time along with a boatload more to print
each return along the way. It got lost while I was stripping out the
extraneous debug code.
> + }
> + }
> +
> + /* If we get here, we found the path, but not the property */
> + VIR_DEBUG("Property 'guest-stats-polling-interval' not
found");
> + ret = -1;
> + goto cleanup;
> + }
> +
> + /* Type entries that begin with "child<" are a branch that can
be
> + * traversed looking for more entries
> + */
> + if (paths[i]->type && STRPREFIX(paths[i]->type,
"child<")) {
> + if (virAsprintf(&nextpath, "%s/%s", curpath,
paths[i]->name) < 0) {
> + virReportOOMError();
> + ret = -1;
> + goto cleanup;
> + }
> + ret = qemuMonitorFindBalloonObjectPath(mon, vm, nextpath);
> + }
> + }
> +
> +cleanup:
> + for (i = 0; i < npaths; i++)
> + qemuMonitorJSONListPathFree(paths[i]);
> + VIR_FREE(paths);
> + for (j = 0; j < nprops; j++)
> + qemuMonitorJSONListPathFree(bprops[j]);
> + VIR_FREE(bprops);
> + VIR_FREE(nextpath);
> + return ret;
> +}
> +
> int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon,
> const char *cmd,
> int scm_fd,
> @@ -1390,6 +1495,31 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
> return ret;
> }
>
> +int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
> + int period)
> +{
> + int ret = -1;
> + VIR_DEBUG("mon=%p period=%d", mon, period);
> +
> + if (!mon) {
> + virReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("monitor must not be NULL"));
> + return -1;
> + }
> +
> + if (!mon->json) {
> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> + _("JSON monitor is required"));
> + return -1;
> + }
> +
> + if (qemuMonitorFindBalloonObjectPath(mon, mon->vm, "/") == 1) {
> + ret = qemuMonitorJSONSetMemoryStatsPeriod(mon, mon->balloonpath,
> + period);
> + }
> + return ret;
> +}
> +
> int
> qemuMonitorBlockIOStatusToError(const char *status)
> {
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 78011ee..12b730a 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -265,6 +265,8 @@ int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon,
> int qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
> virDomainMemoryStatPtr stats,
> unsigned int nr_stats);
> +int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
> + int period);
>
> int qemuMonitorBlockIOStatusToError(const char *status);
> virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon);
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 9487955..2d7f9b6 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -1495,6 +1495,48 @@ cleanup:
> }
>
>
> +/*
> + * Using the provided balloonpath, determine if we need to set the
s/balloonpath/balloon path/
balloonpath is the variable name...
> + * collection interval property to enable statistics gathering.
> + */
> +int
> +qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon,
> + char *balloonpath,
> + int period)
> +{
> + qemuMonitorJSONObjectProperty prop;
> +
> + /* Get the current value of the stats polling interval */
> + memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
> + prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
> + if (qemuMonitorJSONGetObjectProperty(mon, balloonpath,
> + "guest-stats-polling-interval",
> + &prop) < 0) {
> + VIR_DEBUG("Failed to get polling interval for balloon driver");
> + return 0;
Shouldn't we return -1 here? I mean, if caller requires us to set an
interval, we should error out if we fail. Why are we querying for the
interval prior setting it anyway?
I struggled with this return as well when I first added the code - I
think this was added before saving the path and before checking in the
path code that we our path had a polling-interval property.
So, considering the "current" design it's duplicitous to get the value
and check it against the current setting before setting, so I've removed it.
> + }
> +
> + VIR_DEBUG("memballoon period=%d
'guest-stats-polling-interval'=%d",
> + period, prop.val.i);
> +
> + /* Same value - no need to set */
> + if (period == prop.val.i)
> + return 0;
> +
> + /* Set to the value in memballoon (could enable or disable) */
> + memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
> + prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
> + prop.val.i = period;
> + if (qemuMonitorJSONSetObjectProperty(mon, balloonpath,
> + "guest-stats-polling-interval",
> + &prop) < 0) {
> + VIR_DEBUG("Failed to set polling interval for balloon driver");
This is unnecessary as long as qemuMonitorJSONSetObjectProperty reports
error on retval < 0, which it does.
OK gone.
> + return -1;
> + }
> + return 0;
> +}
> +
> +
> int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
> virHashTablePtr table)
> {
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index fd0dedd..b0068ff 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -61,6 +61,9 @@ int qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon,
> int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
> virDomainMemoryStatPtr stats,
> unsigned int nr_stats);
> +int qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon,
> + char *balloonpath,
> + int period);
> int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
> virHashTablePtr table);
> int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index f8c652f..fd3b7a8 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3091,6 +3091,13 @@ qemuProcessReconnect(void *opaque)
> if (qemuProcessRecoverJob(driver, obj, conn, &oldjob) < 0)
> goto error;
>
> + if (obj->def->memballoon) {
> + qemuDomainObjEnterMonitor(driver, obj);
> + qemuMonitorSetMemoryStatsPeriod(priv->mon,
> + obj->def->memballoon->period);
> + qemuDomainObjExitMonitor(driver, obj);
> + }
> +
Why do we want to enable this on ProcessReconnect?
Before I had a way to dynamically set the period I only had the period
in the XML file, thus this was the only way to "get" the stats to show
up for a running guest when I installed the new code and restarted
libvirtd.
Tks,
John
> /* update domain state XML with possibly updated state in
virDomainObj */
> if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, obj) < 0)
> goto error;
> @@ -3910,6 +3917,9 @@ int qemuProcessStart(virConnectPtr conn,
> goto cleanup;
> }
> qemuDomainObjEnterMonitor(driver, vm);
> + if (vm->def->memballoon)
> + qemuMonitorSetMemoryStatsPeriod(priv->mon,
> + vm->def->memballoon->period);
It makes sense here.
> if (qemuMonitorSetBalloon(priv->mon, cur_balloon) < 0) {
> qemuDomainObjExitMonitor(driver, vm);
> goto cleanup;
> @@ -4439,11 +4449,18 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
> if (!virDomainObjIsActive(vm))
> goto cleanup;
>
> - if (running)
> + if (running) {
> virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
> VIR_DOMAIN_RUNNING_UNPAUSED);
> - else
> + if (vm->def->memballoon) {
> + qemuDomainObjEnterMonitor(driver, vm);
> + qemuMonitorSetMemoryStatsPeriod(priv->mon,
> + vm->def->memballoon->period);
> + qemuDomainObjExitMonitor(driver, vm);
> + }
> + } else {
> virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, reason);
> + }
>
> VIR_DEBUG("Writing domain status to disk");
> if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
>
Michal
9:33 a.m.
New subject: [libvirt] [PATCH v2 05/10] Determine whether to start balloon memory stats gathering.
On Mon, Jul 08, 2013 at 03:20:31PM -0400, John Ferlan wrote:
At vm startup, reconnect, and attach - check for the presence of the
balloon
driver and save the path in the private area of the driver. This path will
remain constant throughout the life of the domain and can then be used rather
than attempting to find the path each time balloon driver statistics are
fetched or the collection period changes. The qom object model model requires
setting object properties after device startup. That is, it's not possible
to pass the period along via the startup code as it won't be recognized.
If a balloon driver path is found a check of the existing collection period
will be made against the saved domain value in order to determine if an
adjustment needs to be made to the period to start or stop collecting stats
---
src/qemu/qemu_monitor.c | 130 +++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor.h | 2 +
src/qemu/qemu_monitor_json.c | 42 ++++++++++++++
src/qemu/qemu_monitor_json.h | 3 +
src/qemu/qemu_process.c | 21 ++++++-
5 files changed, 196 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index fe5ab0a..038c9e8 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -83,6 +83,9 @@ struct _qemuMonitor {
/* cache of query-command-line-options results */
virJSONValuePtr options;
+
+ /* If found, path to the virtio memballoon driver */
+ char *balloonpath;
Hmm, we can't distinguish between "not searched yet" and
"searched, but didn't find one". This means in the latter
case we'll repeatedly search every time. Not too bad, but
a little wasteful. Perhaps add a 'bool ballooninit' to
track whether we've searched yet.
+ * Returns:
+ *
+ * 1 - Found
+ * 0 - Not found still looking
+ * -1 - Error bail out
+ *
+ * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
+ */
+static int
+qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
+ virDomainObjPtr vm,
+ const char *curpath)
+{
+ int i,j;
size_t now
+ int npaths = 0;
+ int nprops = 0;
+ int ret = 0;
+ char *nextpath = NULL;
+ qemuMonitorJSONListPathPtr *paths = NULL;
+ qemuMonitorJSONListPathPtr *bprops = NULL;
+
+ /* Not supported */
+ if (vm->def->memballoon &&
+ vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
+ VIR_DEBUG("Model must be virtio to get memballoon path");
+ return -1;
+ }
What about if 'vm->def->memballoon == NULL' ? Shouldn't we return -1 in
that case too ? eg this condition should actually be
if (vm->def->memballoon == NULL ||
vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:07 a.m.
New subject: [libvirt] [PATCH v2 05/10] Determine whether to start balloon memory stats gathering.
On 07/11/2013 10:33 AM, Daniel P. Berrange wrote:
On Mon, Jul 08, 2013 at 03:20:31PM -0400, John Ferlan wrote:
> At vm startup, reconnect, and attach - check for the presence of the balloon
> driver and save the path in the private area of the driver. This path will
> remain constant throughout the life of the domain and can then be used rather
> than attempting to find the path each time balloon driver statistics are
> fetched or the collection period changes. The qom object model model requires
> setting object properties after device startup. That is, it's not possible
> to pass the period along via the startup code as it won't be recognized.
> If a balloon driver path is found a check of the existing collection period
> will be made against the saved domain value in order to determine if an
> adjustment needs to be made to the period to start or stop collecting stats
> ---
> src/qemu/qemu_monitor.c | 130 +++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor.h | 2 +
> src/qemu/qemu_monitor_json.c | 42 ++++++++++++++
> src/qemu/qemu_monitor_json.h | 3 +
> src/qemu/qemu_process.c | 21 ++++++-
> 5 files changed, 196 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index fe5ab0a..038c9e8 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -83,6 +83,9 @@ struct _qemuMonitor {
>
> /* cache of query-command-line-options results */
> virJSONValuePtr options;
> +
> + /* If found, path to the virtio memballoon driver */
> + char *balloonpath;
Hmm, we can't distinguish between "not searched yet" and
"searched, but didn't find one". This means in the latter
case we'll repeatedly search every time. Not too bad, but
a little wasteful. Perhaps add a 'bool ballooninit' to
track whether we've searched yet.
Oh yeah - added that, thanks.
> + * Returns:
> + *
> + * 1 - Found
> + * 0 - Not found still looking
> + * -1 - Error bail out
> + *
> + * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
> + */
> +static int
> +qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
> + virDomainObjPtr vm,
> + const char *curpath)
> +{
> + int i,j;
size_t now
Yep
> + int npaths = 0;
> + int nprops = 0;
> + int ret = 0;
> + char *nextpath = NULL;
> + qemuMonitorJSONListPathPtr *paths = NULL;
> + qemuMonitorJSONListPathPtr *bprops = NULL;
> +
> + /* Not supported */
> + if (vm->def->memballoon &&
> + vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO)
{
> + VIR_DEBUG("Model must be virtio to get memballoon path");
> + return -1;
> + }
What about if 'vm->def->memballoon == NULL' ? Shouldn't we return -1
in
that case too ? eg this condition should actually be
if (vm->def->memballoon == NULL ||
vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
Ah - right! Must've been sleeping when I used the && and not a if
(!vm->def->memballoon...)
John
2:20 p.m.
New subject: [libvirt] [PATCH v2 06/10] Add capability to fetch balloon stats
This patch will add the QEMU_MONITOR_OBJECT_PROPERTY_BALLOON_STATS type and
a mechanism in the qemuMonitorObjectProperty to fetch and store an opaque
data array assuming that we are provided a count of current elements,
a count of maximum elements, and the address of the array store the data.
Use the mechanism to fetch balloon driver statistics.
---
src/qemu/qemu_monitor.c | 3 +-
src/qemu/qemu_monitor_json.c | 206 ++++++++++++++++++++-----------------------
src/qemu/qemu_monitor_json.h | 5 ++
3 files changed, 103 insertions(+), 111 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 038c9e8..aac2692 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1489,7 +1489,8 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
}
if (mon->json)
- ret = qemuMonitorJSONGetMemoryStats(mon, stats, nr_stats);
+ ret = qemuMonitorJSONGetMemoryStats(mon, mon->balloonpath,
+ stats, nr_stats);
else
ret = qemuMonitorTextGetMemoryStats(mon, stats, nr_stats);
return ret;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 2d7f9b6..1bc9b71 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1368,129 +1368,111 @@ cleanup:
}
-int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
- virDomainMemoryStatPtr stats,
- unsigned int nr_stats)
-{
- int ret;
- int got = 0;
- virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-balloon",
- NULL);
- virJSONValuePtr reply = NULL;
+/* Process the balloon driver statistics. The request and data returned
+ * will be as follows (although the 'child[#]' entry will differ based on
+ * where it's run).
+ *
+ * { "execute": "qom-get","arguments": \
+ * { "path":
"/machine/i440fx/pci.0/child[7]","property": "guest-stats"}
}
+ *
+ * {"return": {"stats": \
+ * {"stat-swap-out": 0,
+ * "stat-free-memory": 686350336,
+ * "stat-minor-faults": 697283,
+ * "stat-major-faults": 951,
+ * "stat-total-memory": 1019924480,
+ * "stat-swap-in": 0},
+ * "last-update": 1371221540}}
+ *
+ * A value in "stats" can be -1 indicating it's never been
collected/stored.
+ * The 'last-update' value could be used in the future in order to determine
+ * rates and/or whether data has been collected since a previous cycle.
+ * It's currently unused.
+ */
+#define GET_BALLOON_STATS(FIELD, TAG, DIVISOR) \
+ if (virJSONValueObjectHasKey(statsdata, FIELD) && \
+ (prop->curelems < prop->maxelems)) { \
+ if (virJSONValueObjectGetNumberUlong(statsdata, FIELD, &mem) < 0) { \
+ VIR_DEBUG("Failed to get '%s' value", FIELD);
\
+ } else { \
+ /* Not being collected? No point in providing bad data */ \
+ if (mem != -1UL) { \
+ stat[prop->curelems].tag = TAG; \
+ stat[prop->curelems].val = mem / DIVISOR; \
+ prop->curelems++; \
+ } \
+ } \
+ }
- if (!cmd)
- return -1;
+static int
+qemuMonitorJSONGetBalloonStats(virJSONValuePtr data,
+ qemuMonitorJSONObjectPropertyPtr prop)
+{
+ int ret = -1;
+ unsigned long long mem;
+ virJSONValuePtr statsdata;
+ virDomainMemoryStatPtr stat = (virDomainMemoryStatPtr)prop->val.ptr;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+ VIR_DEBUG("Address of found stat = %p", stat);
- if (ret == 0) {
- /* See if balloon soft-failed */
- if (qemuMonitorJSONHasError(reply, "DeviceNotActive") ||
- qemuMonitorJSONHasError(reply, "KVMMissingCap"))
- goto cleanup;
+ if (!(statsdata = virJSONValueObjectGet(data, "stats"))) {
+ VIR_DEBUG("data does not include 'stats'");
+ goto cleanup;
+ }
- /* See if any other fatal error occurred */
- ret = qemuMonitorJSONCheckError(cmd, reply);
+ GET_BALLOON_STATS("stat-swap-in",
+ VIR_DOMAIN_MEMORY_STAT_SWAP_IN, 1024);
+ GET_BALLOON_STATS("stat-swap-out",
+ VIR_DOMAIN_MEMORY_STAT_SWAP_OUT, 1024);
+ GET_BALLOON_STATS("stat-major-faults",
+ VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT, 1);
+ GET_BALLOON_STATS("stat-minor-faults",
+ VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT, 1);
+ GET_BALLOON_STATS("stat-free-memory",
+ VIR_DOMAIN_MEMORY_STAT_UNUSED, 1024);
+ GET_BALLOON_STATS("stat-total-memory",
+ VIR_DOMAIN_MEMORY_STAT_AVAILABLE, 1024);
- /* Success */
- if (ret == 0) {
- virJSONValuePtr data;
- unsigned long long mem;
+ ret = 0;
- if (!(data = virJSONValueObjectGet(reply, "return"))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("info balloon reply was missing return
data"));
- ret = -1;
- goto cleanup;
- }
+cleanup:
+ virJSONValueFree(statsdata);
+ return ret;
+}
+#undef GET_BALLOON_STATS
- if (virJSONValueObjectHasKey(data, "actual") && (got <
nr_stats)) {
- if (virJSONValueObjectGetNumberUlong(data, "actual", &mem)
< 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("info balloon reply was missing balloon
actual"));
- ret = -1;
- goto cleanup;
- }
- stats[got].tag = VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON;
- stats[got].val = (mem/1024);
- got++;
- }
+int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
+ char *balloonpath,
+ virDomainMemoryStatPtr stats,
+ unsigned int nr_stats)
+{
+ int ret;
+ unsigned long long actualmem;
+ int got = 0;
+ qemuMonitorJSONObjectProperty prop;
- if (virJSONValueObjectHasKey(data, "mem_swapped_in") &&
(got < nr_stats)) {
- if (virJSONValueObjectGetNumberUlong(data, "mem_swapped_in",
&mem) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("info balloon reply was missing balloon
mem_swapped_in"));
- ret = -1;
- goto cleanup;
- }
- stats[got].tag = VIR_DOMAIN_MEMORY_STAT_SWAP_IN;
- stats[got].val = (mem/1024);
- got++;
- }
- if (virJSONValueObjectHasKey(data, "mem_swapped_out") &&
(got < nr_stats)) {
- if (virJSONValueObjectGetNumberUlong(data, "mem_swapped_out",
&mem) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("info balloon reply was missing balloon
mem_swapped_out"));
- ret = -1;
- goto cleanup;
- }
- stats[got].tag = VIR_DOMAIN_MEMORY_STAT_SWAP_OUT;
- stats[got].val = (mem/1024);
- got++;
- }
- if (virJSONValueObjectHasKey(data, "major_page_faults") &&
(got < nr_stats)) {
- if (virJSONValueObjectGetNumberUlong(data, "major_page_faults",
&mem) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("info balloon reply was missing balloon
major_page_faults"));
- ret = -1;
- goto cleanup;
- }
- stats[got].tag = VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT;
- stats[got].val = mem;
- got++;
- }
- if (virJSONValueObjectHasKey(data, "minor_page_faults") &&
(got < nr_stats)) {
- if (virJSONValueObjectGetNumberUlong(data, "minor_page_faults",
&mem) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("info balloon reply was missing balloon
minor_page_faults"));
- ret = -1;
- goto cleanup;
- }
- stats[got].tag = VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT;
- stats[got].val = mem;
- got++;
- }
- if (virJSONValueObjectHasKey(data, "free_mem") && (got <
nr_stats)) {
- if (virJSONValueObjectGetNumberUlong(data, "free_mem",
&mem) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("info balloon reply was missing balloon
free_mem"));
- ret = -1;
- goto cleanup;
- }
- stats[got].tag = VIR_DOMAIN_MEMORY_STAT_UNUSED;
- stats[got].val = (mem/1024);
- got++;
- }
- if (virJSONValueObjectHasKey(data, "total_mem") && (got
< nr_stats)) {
- if (virJSONValueObjectGetNumberUlong(data, "total_mem",
&mem) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("info balloon reply was missing balloon
total_mem"));
- ret = -1;
- goto cleanup;
- }
- stats[got].tag = VIR_DOMAIN_MEMORY_STAT_AVAILABLE;
- stats[got].val = (mem/1024);
- got++;
- }
+ ret = qemuMonitorJSONGetBalloonInfo(mon, &actualmem);
+ if (ret == 1 && (got < nr_stats)) {
+ stats[got].tag = VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON;
+ stats[got].val = actualmem;
+ got++;
+ }
+
+ if (got == 1 && balloonpath) {
+ memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BALLOON_STATS;
+ prop.curelems = got;
+ prop.maxelems = nr_stats;
+ prop.val.ptr = (void **)stats;
+ if (qemuMonitorJSONGetObjectProperty(mon, balloonpath,
+ "guest-stats", &prop) == 0) {
+ got = prop.curelems;
}
}
if (got > 0)
ret = got;
-cleanup:
- virJSONValueFree(cmd);
- virJSONValueFree(reply);
return ret;
}
@@ -4747,6 +4729,9 @@ int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
if (tmp)
ret = 0;
break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_BALLOON_STATS:
+ ret = qemuMonitorJSONGetBalloonStats(data, prop);
+ break;
case QEMU_MONITOR_OBJECT_PROPERTY_LAST:
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -4810,6 +4795,7 @@ int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
case QEMU_MONITOR_OBJECT_PROPERTY_STRING:
MAKE_SET_CMD("s:value", prop->val.str);
break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_BALLOON_STATS:
case QEMU_MONITOR_OBJECT_PROPERTY_LAST:
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -4830,7 +4816,7 @@ cleanup:
return ret;
}
-
+#undef MAKE_SET_CMD
int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
const char *type,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index b0068ff..b3945b2 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -59,6 +59,7 @@ int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon,
int qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon,
unsigned long long *currmem);
int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
+ char *balloonpath,
virDomainMemoryStatPtr stats,
unsigned int nr_stats);
int qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon,
@@ -360,6 +361,7 @@ typedef enum {
QEMU_MONITOR_OBJECT_PROPERTY_ULONG,
QEMU_MONITOR_OBJECT_PROPERTY_DOUBLE,
QEMU_MONITOR_OBJECT_PROPERTY_STRING,
+ QEMU_MONITOR_OBJECT_PROPERTY_BALLOON_STATS,
QEMU_MONITOR_OBJECT_PROPERTY_LAST
} qemuMonitorJSONObjectPropertyType;
@@ -368,6 +370,8 @@ typedef struct _qemuMonitorJSONObjectProperty
qemuMonitorJSONObjectProperty;
typedef qemuMonitorJSONObjectProperty *qemuMonitorJSONObjectPropertyPtr;
struct _qemuMonitorJSONObjectProperty {
int type; /* qemuMonitorJSONObjectPropertyType */
+ int curelems; /* Current number elements in **ptr array */
+ int maxelems; /* Maximum number elements allowed in any **ptr array */
union {
bool b;
int i;
@@ -376,6 +380,7 @@ struct _qemuMonitorJSONObjectProperty {
unsigned long long ul;
double d;
char *str;
+ void **ptr;
} val;
};
--
1.8.1.4
9:39 a.m.
New subject: [libvirt] [PATCH v2 06/10] Add capability to fetch balloon stats
On Mon, Jul 08, 2013 at 03:20:32PM -0400, John Ferlan wrote:
This patch will add the QEMU_MONITOR_OBJECT_PROPERTY_BALLOON_STATS
type and
a mechanism in the qemuMonitorObjectProperty to fetch and store an opaque
data array assuming that we are provided a count of current elements,
a count of maximum elements, and the address of the array store the data.
Use the mechanism to fetch balloon driver statistics.
---
src/qemu/qemu_monitor.c | 3 +-
src/qemu/qemu_monitor_json.c | 206 ++++++++++++++++++++-----------------------
src/qemu/qemu_monitor_json.h | 5 ++
3 files changed, 103 insertions(+), 111 deletions(-)
@@ -4747,6 +4729,9 @@ int
qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
if (tmp)
ret = 0;
break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_BALLOON_STATS:
+ ret = qemuMonitorJSONGetBalloonStats(data, prop);
+ break;
case QEMU_MONITOR_OBJECT_PROPERTY_LAST:
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -4810,6 +4795,7 @@ int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
case QEMU_MONITOR_OBJECT_PROPERTY_STRING:
MAKE_SET_CMD("s:value", prop->val.str);
break;
+ case QEMU_MONITOR_OBJECT_PROPERTY_BALLOON_STATS:
case QEMU_MONITOR_OBJECT_PROPERTY_LAST:
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -4830,7 +4816,7 @@ cleanup:
return ret;
}
-
[snip]
@@ -360,6 +361,7 @@ typedef enum {
QEMU_MONITOR_OBJECT_PROPERTY_ULONG,
QEMU_MONITOR_OBJECT_PROPERTY_DOUBLE,
QEMU_MONITOR_OBJECT_PROPERTY_STRING,
+ QEMU_MONITOR_OBJECT_PROPERTY_BALLOON_STATS,
QEMU_MONITOR_OBJECT_PROPERTY_LAST
} qemuMonitorJSONObjectPropertyType;
@@ -368,6 +370,8 @@ typedef struct _qemuMonitorJSONObjectProperty
qemuMonitorJSONObjectProperty;
typedef qemuMonitorJSONObjectProperty *qemuMonitorJSONObjectPropertyPtr;
struct _qemuMonitorJSONObjectProperty {
int type; /* qemuMonitorJSONObjectPropertyType */
+ int curelems; /* Current number elements in **ptr array */
+ int maxelems; /* Maximum number elements allowed in any **ptr array */
union {
bool b;
int i;
@@ -376,6 +380,7 @@ struct _qemuMonitorJSONObjectProperty {
unsigned long long ul;
double d;
char *str;
+ void **ptr;
} val;
I still don't really like that you're taking a function which is
generic (qemuMonitorJSONGetObjectProperty) and adding in code which
is specific to a single monitor command, and storing command specific
data structures in a 'void **'. IMHO qemuMonitorJSONGetObjectProperty
should only ever return generic data structures, and the conversion
to the memory stats struct should be done by the caller.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
9:41 a.m.
New subject: [libvirt] [PATCH v2 06/10] Add capability to fetch balloon stats
On Mon, Jul 08, 2013 at 03:20:32PM -0400, John Ferlan wrote:
This patch will add the QEMU_MONITOR_OBJECT_PROPERTY_BALLOON_STATS
type and
a mechanism in the qemuMonitorObjectProperty to fetch and store an opaque
data array assuming that we are provided a count of current elements,
a count of maximum elements, and the address of the array store the data.
Use the mechanism to fetch balloon driver statistics.
---
src/qemu/qemu_monitor.c | 3 +-
src/qemu/qemu_monitor_json.c | 206 ++++++++++++++++++++-----------------------
src/qemu/qemu_monitor_json.h | 5 ++
3 files changed, 103 insertions(+), 111 deletions(-)
+int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
+ char *balloonpath,
+ virDomainMemoryStatPtr stats,
+ unsigned int nr_stats)
Given the level of changes in this function, can you also add a
test case for it in qemumonitorjsontest.c. It is hard to tell from
the code that it is actually doing what is is intended to do.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:20 p.m.
New subject: [libvirt] [PATCH v2 07/10] Add new public API virDomainSetMemoryStatsPeriodFlags
Add new API in order to set the balloon memory driver statistics collection
period in order to allow dynamic period adjustment for the virsh dommemstats to
display balloon stats data
---
include/libvirt/libvirt.h.in | 3 ++
src/driver.h | 6 ++++
src/libvirt.c | 65 ++++++++++++++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 5 ++++
4 files changed, 79 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index b87255a..069cac8 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1909,6 +1909,9 @@ int virDomainSetMemory (virDomainPtr
domain,
int virDomainSetMemoryFlags (virDomainPtr domain,
unsigned long memory,
unsigned int flags);
+int virDomainSetMemoryStatsPeriodFlags (virDomainPtr domain,
+ int period,
+ unsigned int flags);
int virDomainGetMaxVcpus (virDomainPtr domain);
int virDomainGetSecurityLabel (virDomainPtr domain,
virSecurityLabelPtr seclabel);
diff --git a/src/driver.h b/src/driver.h
index 31851cb..a8964e9 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -207,6 +207,11 @@ typedef int
unsigned int flags);
typedef int
+(*virDrvDomainSetMemoryStatsPeriodFlags)(virDomainPtr domain,
+ int period,
+ unsigned int flags);
+
+typedef int
(*virDrvDomainSetMemoryParameters)(virDomainPtr domain,
virTypedParameterPtr params,
int nparams,
@@ -1158,6 +1163,7 @@ struct _virDriver {
virDrvDomainSetMaxMemory domainSetMaxMemory;
virDrvDomainSetMemory domainSetMemory;
virDrvDomainSetMemoryFlags domainSetMemoryFlags;
+ virDrvDomainSetMemoryStatsPeriodFlags domainSetMemoryStatsPeriodFlags;
virDrvDomainSetMemoryParameters domainSetMemoryParameters;
virDrvDomainGetMemoryParameters domainGetMemoryParameters;
virDrvDomainSetNumaParameters domainSetNumaParameters;
diff --git a/src/libvirt.c b/src/libvirt.c
index 8e19c64..e02ef09 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -3782,6 +3782,71 @@ error:
return -1;
}
+/**
+ * virDomainSetMemoryStatsPeriodFlags:
+ * @domain: a domain object or NULL
+ * @period: the period in seconds for stats collection
+ * @flags: bitwise-OR of virDomainMemoryModFlags
+ *
+ * Dynamically change the domain memory balloon driver statistics collection
+ * period. Use 0 to disable and a positive value to enable.
+ *
+ * @flags may include VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG.
+ * Both flags may be set. If VIR_DOMAIN_AFFECT_LIVE is set, the change affects
+ * a running domain and will fail if domain is not active.
+ * If VIR_DOMAIN_AFFECT_CONFIG is set, the change affects persistent state,
+ * and will fail for transient domains. If neither flag is specified
+ * (that is, @flags is VIR_DOMAIN_AFFECT_CURRENT), then an inactive domain
+ * modifies persistent setup, while an active domain is hypervisor-dependent
+ * on whether just live or both live and persistent state is changed.
+ *
+ * Not all hypervisors can support all flag combinations.
+ *
+ * Returns 0 in case of success, -1 in case of failure.
+ */
+
+int
+virDomainSetMemoryStatsPeriodFlags(virDomainPtr domain, int period,
+ unsigned int flags)
+{
+ virConnectPtr conn;
+
+ VIR_DOMAIN_DEBUG(domain, "peroid=%d, flags=%x", period, flags);
+
+ virResetLastError();
+
+ if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
+ virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+ virDispatchError(NULL);
+ return -1;
+ }
+
+ if (domain->conn->flags & VIR_CONNECT_RO) {
+ virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ goto error;
+ }
+
+ /* This must be positive to set the balloon collection period */
+ virCheckNonNegativeArgGoto(period, error);
+
+ conn = domain->conn;
+
+ if (conn->driver->domainSetMemoryStatsPeriodFlags) {
+ int ret;
+ ret = conn->driver->domainSetMemoryStatsPeriodFlags(domain, period,
+ flags);
+ if (ret < 0)
+ goto error;
+ return ret;
+ }
+
+ virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+ virDispatchError(domain->conn);
+ return -1;
+}
+
/* Helper function called to validate incoming client array on any
* interface that sets typed parameters in the hypervisor. */
static int
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 7c6edf6..d4e4834 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -627,4 +627,9 @@ LIBVIRT_1.1.0 {
virDomainMigrateToURI3;
} LIBVIRT_1.0.6;
+LIBVIRT_1.1.1 {
+ global:
+ virDomainSetMemoryStatsPeriodFlags;
+} LIBVIRT_1.1.0;
+
# .... define new API here using predicted next version number ....
--
1.8.1.4
9:06 a.m.
New subject: [libvirt] [PATCH v2 07/10] Add new public API virDomainSetMemoryStatsPeriodFlags
On 08.07.2013 21:20, John Ferlan wrote:
Add new API in order to set the balloon memory driver statistics
collection
period in order to allow dynamic period adjustment for the virsh dommemstats to
display balloon stats data
---
include/libvirt/libvirt.h.in | 3 ++
src/driver.h | 6 ++++
src/libvirt.c | 65 ++++++++++++++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 5 ++++
4 files changed, 79 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index b87255a..069cac8 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1909,6 +1909,9 @@ int virDomainSetMemory (virDomainPtr
domain,
int virDomainSetMemoryFlags (virDomainPtr domain,
unsigned long memory,
unsigned int flags);
+int virDomainSetMemoryStatsPeriodFlags (virDomainPtr domain,
+ int period,
+ unsigned int flags);
Lose Flags from the function name. This is completely new API, not an
improved version of pre-existing one, where we forgot to add @flags.
Michal
9:45 a.m.
New subject: [libvirt] [PATCH v2 07/10] Add new public API virDomainSetMemoryStatsPeriodFlags
On Mon, Jul 08, 2013 at 03:20:33PM -0400, John Ferlan wrote:
Add new API in order to set the balloon memory driver statistics
collection
period in order to allow dynamic period adjustment for the virsh dommemstats to
display balloon stats data
---
include/libvirt/libvirt.h.in | 3 ++
src/driver.h | 6 ++++
src/libvirt.c | 65 ++++++++++++++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 5 ++++
4 files changed, 79 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index b87255a..069cac8 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1909,6 +1909,9 @@ int virDomainSetMemory (virDomainPtr
domain,
int virDomainSetMemoryFlags (virDomainPtr domain,
unsigned long memory,
unsigned int flags);
+int virDomainSetMemoryStatsPeriodFlags (virDomainPtr domain,
+ int period,
+ unsigned int flags);
s/Flags// in the name - we only do that if we're replacing an
existing API.
diff --git a/src/driver.h b/src/driver.h
index 31851cb..a8964e9 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -207,6 +207,11 @@ typedef int
unsigned int flags);
typedef int
+(*virDrvDomainSetMemoryStatsPeriodFlags)(virDomainPtr domain,
+ int period,
+ unsigned int flags);
s/Flags//
@@ -1158,6 +1163,7 @@ struct _virDriver {
virDrvDomainSetMaxMemory domainSetMaxMemory;
virDrvDomainSetMemory domainSetMemory;
virDrvDomainSetMemoryFlags domainSetMemoryFlags;
+ virDrvDomainSetMemoryStatsPeriodFlags domainSetMemoryStatsPeriodFlags;
s/Flags//
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 7c6edf6..d4e4834 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -627,4 +627,9 @@ LIBVIRT_1.1.0 {
virDomainMigrateToURI3;
} LIBVIRT_1.0.6;
+LIBVIRT_1.1.1 {
+ global:
+ virDomainSetMemoryStatsPeriodFlags;
s/Flags//
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:20 p.m.
New subject: [libvirt] [PATCH v2 08/10] Specify remote protocol for virDomainSetMemoryStatsPeriodFlags
Wire up the remote protocol
---
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 15 ++++++++++++++-
src/remote_protocol-structs | 6 ++++++
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 7f3e833..762e4b3 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -6610,6 +6610,7 @@ static virDriver remote_driver = {
.domainSetMaxMemory = remoteDomainSetMaxMemory, /* 0.3.0 */
.domainSetMemory = remoteDomainSetMemory, /* 0.3.0 */
.domainSetMemoryFlags = remoteDomainSetMemoryFlags, /* 0.9.0 */
+ .domainSetMemoryStatsPeriodFlags = remoteDomainSetMemoryStatsPeriodFlags, /* 1.1.1
*/
.domainSetMemoryParameters = remoteDomainSetMemoryParameters, /* 0.8.5 */
.domainGetMemoryParameters = remoteDomainGetMemoryParameters, /* 0.8.5 */
.domainSetBlkioParameters = remoteDomainSetBlkioParameters, /* 0.9.0 */
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 2e9dc1d..dcdc0f3 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -833,6 +833,12 @@ struct remote_domain_set_memory_flags_args {
unsigned int flags;
};
+struct remote_domain_set_memory_stats_period_flags_args {
+ remote_nonnull_domain dom;
+ int period;
+ unsigned int flags;
+};
+
struct remote_domain_get_info_args {
remote_nonnull_domain dom;
};
@@ -4944,6 +4950,13 @@ enum remote_procedure {
* @generate: none
* @acl: domain:migrate
*/
- REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3_PARAMS = 307
+ REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3_PARAMS = 307,
+ /**
+ * @generate: both
+ * @acl: domain:write
+ * @acl: domain:save:!VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE
+ * @acl: domain:save:VIR_DOMAIN_AFFECT_CONFIG
+ */
+ REMOTE_PROC_DOMAIN_SET_MEMORY_STATS_PERIOD_FLAGS = 308
};
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index e38d24a..53aa8b5 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -500,6 +500,11 @@ struct remote_domain_set_memory_flags_args {
uint64_t memory;
u_int flags;
};
+struct remote_domain_set_memory_stats_period_flags_args {
+ remote_nonnull_domain dom;
+ int period;
+ u_int flags;
+};
struct remote_domain_get_info_args {
remote_nonnull_domain dom;
};
@@ -2601,4 +2606,5 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_MIGRATE_PERFORM3_PARAMS = 305,
REMOTE_PROC_DOMAIN_MIGRATE_FINISH3_PARAMS = 306,
REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3_PARAMS = 307,
+ REMOTE_PROC_DOMAIN_SET_MEMORY_STATS_PERIOD_FLAGS = 308,
};
--
1.8.1.4
9:45 a.m.
New subject: [libvirt] [PATCH v2 08/10] Specify remote protocol for virDomainSetMemoryStatsPeriodFlags
On Mon, Jul 08, 2013 at 03:20:34PM -0400, John Ferlan wrote:
Wire up the remote protocol
---
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 15 ++++++++++++++-
src/remote_protocol-structs | 6 ++++++
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 7f3e833..762e4b3 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -6610,6 +6610,7 @@ static virDriver remote_driver = {
.domainSetMaxMemory = remoteDomainSetMaxMemory, /* 0.3.0 */
.domainSetMemory = remoteDomainSetMemory, /* 0.3.0 */
.domainSetMemoryFlags = remoteDomainSetMemoryFlags, /* 0.9.0 */
+ .domainSetMemoryStatsPeriodFlags = remoteDomainSetMemoryStatsPeriodFlags, /* 1.1.1
*/
.domainSetMemoryParameters = remoteDomainSetMemoryParameters, /* 0.8.5 */
.domainGetMemoryParameters = remoteDomainGetMemoryParameters, /* 0.8.5 */
.domainSetBlkioParameters = remoteDomainSetBlkioParameters, /* 0.9.0 */
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 2e9dc1d..dcdc0f3 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -833,6 +833,12 @@ struct remote_domain_set_memory_flags_args {
unsigned int flags;
};
+struct remote_domain_set_memory_stats_period_flags_args {
s/flags// and in other places below.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:20 p.m.
New subject: [libvirt] [PATCH v2 09/10] Implement the virDomainSetMemoryStatsPeriodFlags for QEMU driver
Implement the new API that will handle setting the balloon driver statistics
collection period in order to enable or disable the collection dynamically.
---
src/qemu/qemu_driver.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1b9ba41..e97a28b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2274,6 +2274,71 @@ static int qemuDomainSetMaxMemory(virDomainPtr dom, unsigned long
memory)
return qemuDomainSetMemoryFlags(dom, memory, VIR_DOMAIN_MEM_MAXIMUM);
}
+static int qemuDomainSetMemoryStatsPeriodFlags(virDomainPtr dom, int period,
+ unsigned int flags)
+{
+ virQEMUDriverPtr driver = dom->conn->privateData;
+ qemuDomainObjPrivatePtr priv;
+ virDomainObjPtr vm;
+ virDomainDefPtr persistentDef = NULL;
+ int ret = -1, r;
+ virQEMUDriverConfigPtr cfg = NULL;
+ virCapsPtr caps = NULL;
+
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+ if (!(vm = qemuDomObjFromDomain(dom)))
+ goto cleanup;
+
+ cfg = virQEMUDriverGetConfig(driver);
+
+ if (virDomainSetMemoryStatsPeriodFlagsEnsureACL(dom->conn, vm->def,
+ flags) < 0)
+ goto cleanup;
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ goto cleanup;
+
+ if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+ goto endjob;
+ if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags,
+ &persistentDef) < 0)
+ goto endjob;
+
+ /* Set the balloon driver collection interval */
+ priv = vm->privateData;
+
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ r = qemuMonitorSetMemoryStatsPeriod(priv->mon, period);
+ qemuDomainObjExitMonitor(driver, vm);
+ if (r < 0)
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("unable to set balloon driver collection
period"));
+ }
+
+ if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ sa_assert(persistentDef);
+ persistentDef->memballoon->period = period;
+ ret = virDomainSaveConfig(cfg->configDir, persistentDef);
+ goto endjob;
+ }
+
+ ret = 0;
+endjob:
+ if (qemuDomainObjEndJob(driver, vm) == 0)
+ vm = NULL;
+
+cleanup:
+ if (vm)
+ virObjectUnlock(vm);
+ virObjectUnref(caps);
+ virObjectUnref(cfg);
+ return ret;
+}
+
static int qemuDomainInjectNMI(virDomainPtr domain, unsigned int flags)
{
virQEMUDriverPtr driver = domain->conn->privateData;
@@ -16086,6 +16151,7 @@ static virDriver qemuDriver = {
.domainSetMemoryFlags = qemuDomainSetMemoryFlags, /* 0.9.0 */
.domainSetMemoryParameters = qemuDomainSetMemoryParameters, /* 0.8.5 */
.domainGetMemoryParameters = qemuDomainGetMemoryParameters, /* 0.8.5 */
+ .domainSetMemoryStatsPeriodFlags = qemuDomainSetMemoryStatsPeriodFlags, /* 1.1.1 */
.domainSetBlkioParameters = qemuDomainSetBlkioParameters, /* 0.9.0 */
.domainGetBlkioParameters = qemuDomainGetBlkioParameters, /* 0.9.0 */
.domainGetInfo = qemuDomainGetInfo, /* 0.2.0 */
--
1.8.1.4
2:20 p.m.
New subject: [libvirt] [PATCH v2 10/10] Allow balloon driver collection to be adjusted dynamically
Use the virDomainSetMemoryStatsPeriodFlags() to pass a period defined by
usage of a new --period option in order to set the collection period for the
balloon driver. This may enable or disable the collection based on the value.
Add the --current, --live, & --config options to dommemstat.
---
docs/formatdomain.html.in | 11 +++++++-
tools/virsh-domain-monitor.c | 66 ++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 73 insertions(+), 4 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 93d2416..df84ed2 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4660,7 +4660,16 @@ qemu-kvm -net nic,model=? /dev/null
<p>
The optional <code>period</code> allows the QEMU virtio memory
balloon driver to provide statistics through the <code>virsh
- dommemstat [domain]</code> command.
+ dommemstat [domain]</code> command. By default, collection is
+ not enabled. In order to enable, use the <code>virsh dommemstat
+ [domain] --period [number]</code> command or <code>virsh
edit</code>
+ command to add the option to the XML definition.
+ The <code>virsh dommemstat</code> will accept the options
+ <code>--live</code>, <code>--current</code>, or
<code>--config</code>.
+ If an option is not provided, the change for a running domain will
+ only be made to the active guest.
+ If the QEMU driver is not at the right
+ revision, the attempt to set the period will fail.
<span class='since'>Since 1.1.1, requires QEMU 1.5</span>
</p>
</dd>
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 58d6d40..d2ed921 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -314,6 +314,23 @@ static const vshCmdOptDef opts_dommemstat[] = {
.flags = VSH_OFLAG_REQ,
.help = N_("domain name, id or uuid")
},
+ {.name = "period",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_EMPTY_OK,
+ .help = N_("period in seconds to set collection")
+ },
+ {.name = "config",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect next boot")
+ },
+ {.name = "live",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect running domain")
+ },
+ {.name = "current",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect current domain")
+ },
{.name = NULL}
};
@@ -324,15 +341,56 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
const char *name;
struct _virDomainMemoryStat stats[VIR_DOMAIN_MEMORY_STAT_NR];
unsigned int nr_stats, i;
+ int ret = false;
+ int rv = 0;
+ int period = -1;
+ bool config = vshCommandOptBool(cmd, "config");
+ bool live = vshCommandOptBool(cmd, "live");
+ bool current = vshCommandOptBool(cmd, "current");
+ unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+ if (config)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
return false;
+ /* If none of the options were specified and we're active
+ * then be sure to allow active modification */
+ if (!current && !live && !config && virDomainIsActive(dom) ==
1)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+ /* Providing a period will adjust the balloon driver collection period.
+ * This is not really an unsigned long, but it
+ */
+ if ((rv = vshCommandOptInt(cmd, "period", &period)) < 0) {
+ vshError(ctl, "%s",
+ _("Unable to parse integer parameter."));
+ goto cleanup;
+ }
+ if (rv > 0) {
+ if (period < 0) {
+ vshError(ctl, _("Invalid collection period value '%d'"),
period);
+ goto cleanup;
+ }
+
+ if (virDomainSetMemoryStatsPeriodFlags(dom, period, flags) < 0) {
+ vshError(ctl, "%s",
+ _("Unable to change balloon collection period."));
+ } else {
+ ret = true;
+ }
+ goto cleanup;
+ }
+
nr_stats = virDomainMemoryStats(dom, stats, VIR_DOMAIN_MEMORY_STAT_NR, 0);
if (nr_stats == -1) {
vshError(ctl, _("Failed to get memory statistics for domain %s"),
name);
- virDomainFree(dom);
- return false;
+ goto cleanup;
}
for (i = 0; i < nr_stats; i++) {
@@ -354,8 +412,10 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "rss %llu\n", stats[i].val);
}
+ ret = true;
+cleanup:
virDomainFree(dom);
- return true;
+ return ret;
}
/*
--
1.8.1.4
9:47 a.m.
New subject: [libvirt] [PATCH v2 10/10] Allow balloon driver collection to be adjusted dynamically
On Mon, Jul 08, 2013 at 03:20:36PM -0400, John Ferlan wrote:
Use the virDomainSetMemoryStatsPeriodFlags() to pass a period defined
by
usage of a new --period option in order to set the collection period for the
balloon driver. This may enable or disable the collection based on the value.
Add the --current, --live, & --config options to dommemstat.
---
docs/formatdomain.html.in | 11 +++++++-
tools/virsh-domain-monitor.c | 66 ++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 73 insertions(+), 4 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 93d2416..df84ed2 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4660,7 +4660,16 @@ qemu-kvm -net nic,model=? /dev/null
<p>
The optional <code>period</code> allows the QEMU virtio memory
balloon driver to provide statistics through the <code>virsh
- dommemstat [domain]</code> command.
+ dommemstat [domain]</code> command. By default, collection is
+ not enabled. In order to enable, use the <code>virsh dommemstat
+ [domain] --period [number]</code> command or <code>virsh
edit</code>
+ command to add the option to the XML definition.
+ The <code>virsh dommemstat</code> will accept the options
+ <code>--live</code>, <code>--current</code>, or
<code>--config</code>.
+ If an option is not provided, the change for a running domain will
+ only be made to the active guest.
+ If the QEMU driver is not at the right
+ revision, the attempt to set the period will fail.
<span class='since'>Since 1.1.1, requires QEMU 1.5</span>
</p>
</dd>
Move this chunk into the patch which changes the domaincommon.rng
file.
diff --git a/tools/virsh-domain-monitor.c
b/tools/virsh-domain-monitor.c
index 58d6d40..d2ed921 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -314,6 +314,23 @@ static const vshCmdOptDef opts_dommemstat[] = {
.flags = VSH_OFLAG_REQ,
.help = N_("domain name, id or uuid")
},
+ {.name = "period",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_EMPTY_OK,
+ .help = N_("period in seconds to set collection")
+ },
+ {.name = "config",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect next boot")
+ },
+ {.name = "live",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect running domain")
+ },
+ {.name = "current",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect current domain")
+ },
{.name = NULL}
};
@@ -324,15 +341,56 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
const char *name;
struct _virDomainMemoryStat stats[VIR_DOMAIN_MEMORY_STAT_NR];
unsigned int nr_stats, i;
+ int ret = false;
+ int rv = 0;
+ int period = -1;
+ bool config = vshCommandOptBool(cmd, "config");
+ bool live = vshCommandOptBool(cmd, "live");
+ bool current = vshCommandOptBool(cmd, "current");
+ unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+ if (config)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
return false;
+ /* If none of the options were specified and we're active
+ * then be sure to allow active modification */
+ if (!current && !live && !config && virDomainIsActive(dom)
== 1)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+ /* Providing a period will adjust the balloon driver collection period.
+ * This is not really an unsigned long, but it
+ */
+ if ((rv = vshCommandOptInt(cmd, "period", &period)) < 0) {
+ vshError(ctl, "%s",
+ _("Unable to parse integer parameter."));
+ goto cleanup;
+ }
+ if (rv > 0) {
+ if (period < 0) {
+ vshError(ctl, _("Invalid collection period value '%d'"),
period);
+ goto cleanup;
+ }
+
+ if (virDomainSetMemoryStatsPeriodFlags(dom, period, flags) < 0) {
+ vshError(ctl, "%s",
+ _("Unable to change balloon collection period."));
+ } else {
+ ret = true;
+ }
+ goto cleanup;
+ }
+
nr_stats = virDomainMemoryStats(dom, stats, VIR_DOMAIN_MEMORY_STAT_NR, 0);
if (nr_stats == -1) {
vshError(ctl, _("Failed to get memory statistics for domain %s"),
name);
- virDomainFree(dom);
- return false;
+ goto cleanup;
}
for (i = 0; i < nr_stats; i++) {
@@ -354,8 +412,10 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "rss %llu\n", stats[i].val);
}
+ ret = true;
+cleanup:
virDomainFree(dom);
- return true;
+ return ret;
}
I think the virsh.pod entry for this command could do with updating.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
9:05 a.m.
On 08.07.2013 21:20, John Ferlan wrote:
This patchset replaces:
https://www.redhat.com/archives/libvir-list/2013-July/msg00108.html
Changes since V1:
* Remove the external (eg, qemu_monitor.{c,h}) API's to the qom-list,
qom-get, qom-set interfaces
* Handle errors in virXPathInt() return checking domain_conf.c
* Move the 'period' from _qemuDomainObjPrivate into _qemuMonitor
* Move/rename qemuProcessFindBalloonObjectPath from qemu_process.c
into qemu_monitor.c as qemuMonitorFindBalloonObjectPath()
* Create qemuMonitorJSONSetMemoryStatsPeriod() in qemu_monitor_json.c from
parts of prior change to add qemuProcessUpdateBalloonStatsPeriod()
in qemu_process.c and hav
* Add qemuMonitorSetMemoryStatsPeriod() in qemu_monitor.c to set
the collection period. Adjust the code to take parts of prior change
and make check for balloon path in this path (set if necessary).
* Adjust code in qemuProcessReconnect(), qemuProcessStart(), and
qemuProcessAttach() to call qemuMonitorSetMemoryStatsPeriod()
* Adjust qemuMonitorJSONGetMemoryStats() to get the balloon stats once
the "actual" stat is collected from qemuMonitorJSONGetBalloonInfo().
Reviewer note: The prior code to get "actual" was a duplicate of the
BalloonInfo code. Since "balloon-info" is where the actual memory is
still stored it still must be called as that data is not present in
the balloon stats qom-get "stats" fetch implemented as the new API
qemuMonitorJSONGetBalloonStats().
* Avoid overloading virDomainSetMemoryFlags()...
* Create/use virDomainSetMemoryStatsPeriodFlags() to set the collection
period dynamically from "virsh dommemstats" using the --period qualifier.
John Ferlan (10):
Add qemuMonitorJSONGetObjectListPaths() method for QMP qom-list
command
Add qemuMonitorJSONGetObjectProperty() method for QMP qom-get command
Add qemuMonitorJSONSetObjectProperty() method for QMP qom-set command
Add 'period' for Memballoon statistics gathering capability
Determine whether to start balloon memory stats gathering.
Add capability to fetch balloon stats
Add new public API virDomainSetMemoryStatsPeriodFlags
Specify remote protocol for virDomainSetMemoryStatsPeriodFlags
Implement the virDomainSetMemoryStatsPeriodFlags for QEMU driver
Allow balloon driver collection to be adjusted dynamically
docs/formatdomain.html.in | 19 ++
docs/schemas/domaincommon.rng | 7 +
include/libvirt/libvirt.h.in | 3 +
src/conf/domain_conf.c | 44 +++-
src/conf/domain_conf.h | 1 +
src/driver.h | 6 +
src/libvirt.c | 65 ++++++
src/libvirt_public.syms | 5 +
src/qemu/qemu_driver.c | 66 ++++++
src/qemu/qemu_monitor.c | 133 ++++++++++-
src/qemu/qemu_monitor.h | 2 +
src/qemu/qemu_monitor_json.c | 498 ++++++++++++++++++++++++++++++++----------
src/qemu/qemu_monitor_json.h | 64 ++++++
src/qemu/qemu_process.c | 21 +-
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 15 +-
src/remote_protocol-structs | 6 +
tests/qemumonitorjsontest.c | 186 ++++++++++++++++
tools/virsh-domain-monitor.c | 66 +++++-
19 files changed, 1085 insertions(+), 123 deletions(-)
ACK series modulo patch 05/10 where three questions need to be answered
first:
- Why do we call qemuMonitorSetMemoryStatsPeriod in qemuProcessReconnect?
- Why do we try to gather polling interval prior setting it?
- I guess we should error out there as well.
Michal
4153
days inactive
4156
days old
32 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Eric Blake -
John Ferlan -
Michal Privoznik