8:39 a.m.
https://bugzilla.redhat.com/show_bug.cgi?id=904160
The following patches will provide the support and functionality in order
to re-enable getting domain memory statistics from the balloon driver based
on the upstream QEMU patch:
https://lists.gnu.org/archive/html/qemu-devel/2013-01/msg04835.html
Statistics gathering requires usage of the QEMU QObject model, specifically
the qom-list command to find the path to the balloon driver and the qom-get/
qom-set commands in order to get/set properties on the object. A future
possible extension would be to allow 'virsh qemuobject [--list|--get|--put]'
where the --list would list a single path and the get/set would allow
viewing/adjusting a specific object path property.
The QObject model requires setting properties after the domain (and device)
has been started. For the balloon driver, in order to enable collection the
property "guest-stats-polling-interval" is used. Since the QObject model has
been in place for a while and the balloon driver has been part of that, the
capability to gather statistics is only discernible if the property is found.
Once found, the property can be changed at any time after startup in order
to exend/shorted the collection interval or disable the collection by setting
the property back to 0 (zero). Setting the property back does not reset
already generated statistics. If statistics haven't been generated at all
they are initialized to -1. Thus if values are set at -1, they will not
be stored in the output. The only possible visual cue to determine that
statistics are disabled is if the polling interval property was 0 (zero).
Since it wasn't clear whether adjusting the dommemstats output to provide
that visual cue, I left that as a future possible exercise - although it is
possible to add it to this patch set where the output would be extended
to indicate the current collection interval period or that collection is
currently disabled, eg one new row "period #".
The collection period interval is saved in a new memballoon stats field
as 'period'. This is saved in the xml as "<stats
period='10'/>". The
capability to dynamically set the period once the domain has started is
controlled by a new '--period <value>' option to the dommemstats command.
The command was extended to support the --live, --current, and --config
options. The implementation uses the existing virDomainSetMemoryFlags with
a new 'VIR_DOMAIN_MEM_PERIOD' flag that the qemuDomainSetMemoryFlags() will
handle. This was methodology was chosen in preference to generating (a)
new driver function(s) to just handle the get/set of the collection period.
John Ferlan (8):
Add qemuMonitorGetObjectListPaths() method for QMP qom-list command
Add qemuMonitorGetObjectProperty() method for QMP qom-get command
Add qemuMonitorSetObjectProperty() 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
If available fetch the balloon driver memory stats
Allow balloon driver collection to be adjusted dynamically
docs/formatdomain.html.in | 14 ++
docs/schemas/domaincommon.rng | 7 +
include/libvirt/libvirt.h.in | 1 +
src/conf/domain_conf.c | 27 +++-
src/conf/domain_conf.h | 1 +
src/libvirt.c | 8 +-
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_domain.h | 2 +
src/qemu/qemu_driver.c | 61 +++++++-
src/qemu/qemu_monitor.c | 79 +++++++++++
src/qemu/qemu_monitor.h | 57 ++++++++
src/qemu/qemu_monitor_json.c | 317 ++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 18 +++
src/qemu/qemu_process.c | 164 +++++++++++++++++++++-
tests/qemumonitorjsontest.c | 184 ++++++++++++++++++++++++
tools/virsh-domain-monitor.c | 70 +++++++++-
16 files changed, 1000 insertions(+), 11 deletions(-)
--
1.8.1.4
8:39 a.m.
New subject: [libvirt] [PATCH 1/8] Add qemuMonitorGetObjectListPaths() method for QMP qom-list command
Add a new qemuMonitorGetObjectListPaths() 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.c | 33 ++++++++++++++++
src/qemu/qemu_monitor.h | 15 +++++++
src/qemu/qemu_monitor_json.c | 93 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 6 +++
tests/qemumonitorjsontest.c | 77 ++++++++++++++++++++++++++++++++++++
5 files changed, 224 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 091e239..f175250 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3435,6 +3435,39 @@ int qemuMonitorGetObjectTypes(qemuMonitorPtr mon,
}
+int qemuMonitorGetObjectListPaths(qemuMonitorPtr mon,
+ const char *path,
+ qemuMonitorListPathPtr **paths)
+{
+ VIR_DEBUG("mon=%p paths=%p",
+ mon, paths);
+
+ 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;
+ }
+
+ return qemuMonitorJSONGetObjectListPaths(mon, path, paths);
+}
+
+
+void qemuMonitorListPathFree(qemuMonitorListPathPtr paths)
+{
+ if (!paths)
+ return;
+ VIR_FREE(paths->name);
+ VIR_FREE(paths->type);
+ VIR_FREE(paths);
+}
+
+
int qemuMonitorGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 86ef635..2e92f8c 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -679,6 +679,21 @@ int qemuMonitorGetKVMState(qemuMonitorPtr mon,
int qemuMonitorGetObjectTypes(qemuMonitorPtr mon,
char ***types);
+
+typedef struct _qemuMonitorListPath qemuMonitorListPath;
+typedef qemuMonitorListPath *qemuMonitorListPathPtr;
+
+struct _qemuMonitorListPath {
+ char *name;
+ char *type;
+};
+
+int qemuMonitorGetObjectListPaths(qemuMonitorPtr mon,
+ const char *path,
+ qemuMonitorListPathPtr **paths);
+
+void qemuMonitorListPathFree(qemuMonitorListPathPtr paths);
+
int qemuMonitorGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index c0d7960..7d0cc85 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4533,6 +4533,99 @@ cleanup:
}
+int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
+ const char *path,
+ qemuMonitorListPathPtr **paths)
+{
+ int ret;
+ virJSONValuePtr cmd;
+ virJSONValuePtr reply = NULL;
+ virJSONValuePtr data;
+ qemuMonitorListPathPtr *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;
+ qemuMonitorListPathPtr 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++)
+ qemuMonitorListPathFree(pathlist[i]);
+ VIR_FREE(pathlist);
+ }
+ 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 d79b86b..20a2364 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -332,6 +332,12 @@ int qemuMonitorJSONGetKVMState(qemuMonitorPtr mon,
int qemuMonitorJSONGetObjectTypes(qemuMonitorPtr mon,
char ***types)
ATTRIBUTE_NONNULL(2);
+
+int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
+ const char *path,
+ qemuMonitorListPathPtr **paths)
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+
int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index acc94ca..ab8a73d 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -595,6 +595,82 @@ 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;
+ qemuMonitorListPathPtr *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 = qemuMonitorGetObjectListPaths(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++)
+ qemuMonitorListPathFree(paths[i]);
+ VIR_FREE(paths);
+ return ret;
+}
+
+
static int
mymain(void)
{
@@ -623,6 +699,7 @@ mymain(void)
DO_TEST(GetCommands);
DO_TEST(GetTPMModels);
DO_TEST(GetCommandLineOptionParameters);
+ DO_TEST(GetListPaths);
virObjectUnref(xmlopt);
--
1.8.1.4
5:56 a.m.
New subject: [libvirt] [PATCH 1/8] Add qemuMonitorGetObjectListPaths() method for QMP qom-list command
On Tue, Jul 02, 2013 at 09:39:19AM -0400, John Ferlan wrote:
Add a new qemuMonitorGetObjectListPaths() 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.
Top marks for adding a test !
---
src/qemu/qemu_monitor.c | 33 ++++++++++++++++
src/qemu/qemu_monitor.h | 15 +++++++
src/qemu/qemu_monitor_json.c | 93 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 6 +++
tests/qemumonitorjsontest.c | 77 ++++++++++++++++++++++++++++++++++++
5 files changed, 224 insertions(+)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 86ef635..2e92f8c 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -679,6 +679,21 @@ int qemuMonitorGetKVMState(qemuMonitorPtr mon,
int qemuMonitorGetObjectTypes(qemuMonitorPtr mon,
char ***types);
+
+typedef struct _qemuMonitorListPath qemuMonitorListPath;
+typedef qemuMonitorListPath *qemuMonitorListPathPtr;
+
+struct _qemuMonitorListPath {
+ char *name;
+ char *type;
+};
+
+int qemuMonitorGetObjectListPaths(qemuMonitorPtr mon,
+ const char *path,
+ qemuMonitorListPathPtr **paths);
+
+void qemuMonitorListPathFree(qemuMonitorListPathPtr paths);
The qom related monitor calls are super generic, which also means
they are a super PITA to deal with in code. To mitigate this, I
think we should try to keep code that uses the qom commands
isolated in the qemu_monitor* files, and then expose higher level
APIs to the rest of the QEMU driver code.
So I think it is ok to expose these APIs in qemu_monitor_json.h,
but we should not expose it in qemu_monitor.h
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 :|
10:30 a.m.
New subject: [libvirt] [PATCH 1/8] Add qemuMonitorGetObjectListPaths() method for QMP qom-list command
On 07/03/2013 06:56 AM, Daniel P. Berrange wrote:
On Tue, Jul 02, 2013 at 09:39:19AM -0400, John Ferlan wrote:
> Add a new qemuMonitorGetObjectListPaths() 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.
Top marks for adding a test !
> ---
> src/qemu/qemu_monitor.c | 33 ++++++++++++++++
> src/qemu/qemu_monitor.h | 15 +++++++
> src/qemu/qemu_monitor_json.c | 93 ++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor_json.h | 6 +++
> tests/qemumonitorjsontest.c | 77 ++++++++++++++++++++++++++++++++++++
> 5 files changed, 224 insertions(+)
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 86ef635..2e92f8c 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -679,6 +679,21 @@ int qemuMonitorGetKVMState(qemuMonitorPtr mon,
>
> int qemuMonitorGetObjectTypes(qemuMonitorPtr mon,
> char ***types);
> +
> +typedef struct _qemuMonitorListPath qemuMonitorListPath;
> +typedef qemuMonitorListPath *qemuMonitorListPathPtr;
> +
> +struct _qemuMonitorListPath {
> + char *name;
> + char *type;
> +};
> +
> +int qemuMonitorGetObjectListPaths(qemuMonitorPtr mon,
> + const char *path,
> + qemuMonitorListPathPtr **paths);
> +
> +void qemuMonitorListPathFree(qemuMonitorListPathPtr paths);
The qom related monitor calls are super generic, which also means
they are a super PITA to deal with in code. To mitigate this, I
think we should try to keep code that uses the qom commands
isolated in the qemu_monitor* files, and then expose higher level
APIs to the rest of the QEMU driver code.
So I think it is ok to expose these APIs in qemu_monitor_json.h,
but we should not expose it in qemu_monitor.h
Daniel
The model I followed was the same model other entry points used through
qemu_monitor and into qemu_monitor_json (and for some into
qemu_monitor_text).
If I read this correctly, for the first 3 patches it seems you are
advocating removing the middle man - that is later rather than calling
qemuMonitorGetObjectListPaths call qemuMonitorJSONGetObjectListPaths
directly. Similarly for the GetObject & PutObject calls.
I cannot think of an issue with the change, but I'm just making sure to
avoid continual rework.
That also seems to mean modifying the test to include
'qemu_monitor_json.h'...
John
10:38 a.m.
New subject: [libvirt] [PATCH 1/8] Add qemuMonitorGetObjectListPaths() method for QMP qom-list command
On Wed, Jul 03, 2013 at 11:30:46AM -0400, John Ferlan wrote:
On 07/03/2013 06:56 AM, Daniel P. Berrange wrote:
> On Tue, Jul 02, 2013 at 09:39:19AM -0400, John Ferlan wrote:
>> Add a new qemuMonitorGetObjectListPaths() 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.
>
> Top marks for adding a test !
>
>> ---
>> src/qemu/qemu_monitor.c | 33 ++++++++++++++++
>> src/qemu/qemu_monitor.h | 15 +++++++
>> src/qemu/qemu_monitor_json.c | 93 ++++++++++++++++++++++++++++++++++++++++++++
>> src/qemu/qemu_monitor_json.h | 6 +++
>> tests/qemumonitorjsontest.c | 77 ++++++++++++++++++++++++++++++++++++
>> 5 files changed, 224 insertions(+)
>
>
>> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
>> index 86ef635..2e92f8c 100644
>> --- a/src/qemu/qemu_monitor.h
>> +++ b/src/qemu/qemu_monitor.h
>> @@ -679,6 +679,21 @@ int qemuMonitorGetKVMState(qemuMonitorPtr mon,
>>
>> int qemuMonitorGetObjectTypes(qemuMonitorPtr mon,
>> char ***types);
>> +
>> +typedef struct _qemuMonitorListPath qemuMonitorListPath;
>> +typedef qemuMonitorListPath *qemuMonitorListPathPtr;
>> +
>> +struct _qemuMonitorListPath {
>> + char *name;
>> + char *type;
>> +};
>> +
>> +int qemuMonitorGetObjectListPaths(qemuMonitorPtr mon,
>> + const char *path,
>> + qemuMonitorListPathPtr **paths);
>> +
>> +void qemuMonitorListPathFree(qemuMonitorListPathPtr paths);
>
> The qom related monitor calls are super generic, which also means
> they are a super PITA to deal with in code. To mitigate this, I
> think we should try to keep code that uses the qom commands
> isolated in the qemu_monitor* files, and then expose higher level
> APIs to the rest of the QEMU driver code.
>
> So I think it is ok to expose these APIs in qemu_monitor_json.h,
> but we should not expose it in qemu_monitor.h
>
> Daniel
>
The model I followed was the same model other entry points used through
qemu_monitor and into qemu_monitor_json (and for some into
qemu_monitor_text).
If I read this correctly, for the first 3 patches it seems you are
advocating removing the middle man - that is later rather than calling
qemuMonitorGetObjectListPaths call qemuMonitorJSONGetObjectListPaths
directly. Similarly for the GetObject & PutObject calls.
No, I'm not recommending that qemu_driver/qemu_process call
into qemu_monitor_json.h
I'm saying that instead of directly exposing the ugly interface
of the QOM commands to callers of qemu_monitor.h, we should have
a higher level API exposed.
eg, qemu_monitor.h should just expose
qemuMonitorGetMemoryStats() (already exists in fact)
qemuMonitorSetMemoryStatsRefresPeriod()
and then the qemu_monitor.c impls of these methods can call
qemuMonitorJSON{GetObjectListPaths,GetObject,PutObject}.
No other code outside of the qemu_monitor.c file then has to
know about qemuMonitorJSON{GetObjectListPaths,GetObject,PutObject}
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 :|
8:39 a.m.
New subject: [libvirt] [PATCH 2/8] Add qemuMonitorGetObjectProperty() method for QMP qom-get command
Add a new qemuMonitorGetObjectProperty() method to support invocation
of the 'qom-get' JSON monitor command with a provided path, property, and
expected data type return. The qemuMonitorObjectProperty 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.c | 23 ++++++++++++
src/qemu/qemu_monitor.h | 33 +++++++++++++++++
src/qemu/qemu_monitor_json.c | 86 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 6 ++++
tests/qemumonitorjsontest.c | 48 +++++++++++++++++++++++++
5 files changed, 196 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index f175250..6737a63 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3468,6 +3468,29 @@ void qemuMonitorListPathFree(qemuMonitorListPathPtr paths)
}
+int qemuMonitorGetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorObjectPropertyPtr prop)
+{
+ VIR_DEBUG("mon=%p path=%s property=%s prop=%p type=%d",
+ mon, path, property, prop, prop->type);
+ 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;
+ }
+
+ return qemuMonitorJSONGetObjectProperty(mon, path, property, prop);
+}
+
+
int qemuMonitorGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 2e92f8c..cc22123 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -694,6 +694,39 @@ int qemuMonitorGetObjectListPaths(qemuMonitorPtr mon,
void qemuMonitorListPathFree(qemuMonitorListPathPtr paths);
+/* Flags for the 'type' field in _qemuMonitorObjectProperty */
+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
+} qemuMonitorObjectPropertyType;
+
+typedef struct _qemuMonitorObjectProperty qemuMonitorObjectProperty;
+typedef qemuMonitorObjectProperty *qemuMonitorObjectPropertyPtr;
+struct _qemuMonitorObjectProperty {
+ int type; /* qemuMonitorObjectPropertyType */
+ union {
+ bool b;
+ int i;
+ long long l;
+ unsigned int ui;
+ unsigned long long ul;
+ double d;
+ char *str;
+ } val;
+};
+
+int qemuMonitorGetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorObjectPropertyPtr prop);
+
int qemuMonitorGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 7d0cc85..a725903 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4626,6 +4626,92 @@ cleanup:
return ret;
}
+
+int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorObjectPropertyPtr 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 20a2364..63807df 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -338,6 +338,12 @@ int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
qemuMonitorListPathPtr **paths)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorObjectPropertyPtr 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 ab8a73d..4544676 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -671,6 +671,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;
+ qemuMonitorObjectProperty prop;
+
+ if (!test)
+ return -1;
+
+ if (qemuMonitorTestAddItem(test, "qom-get",
+ "{ \"return\": true }") < 0)
+ goto cleanup;
+
+ /* Present with path and property */
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN;
+ if (qemuMonitorGetObjectProperty(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)
{
@@ -700,6 +747,7 @@ mymain(void)
DO_TEST(GetTPMModels);
DO_TEST(GetCommandLineOptionParameters);
DO_TEST(GetListPaths);
+ DO_TEST(GetObjectProperty);
virObjectUnref(xmlopt);
--
1.8.1.4
5:57 a.m.
New subject: [libvirt] [PATCH 2/8] Add qemuMonitorGetObjectProperty() method for QMP qom-get command
On Tue, Jul 02, 2013 at 09:39:20AM -0400, John Ferlan wrote:
Add a new qemuMonitorGetObjectProperty() method to support
invocation
of the 'qom-get' JSON monitor command with a provided path, property, and
expected data type return. The qemuMonitorObjectProperty 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.c | 23 ++++++++++++
src/qemu/qemu_monitor.h | 33 +++++++++++++++++
src/qemu/qemu_monitor_json.c | 86 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 6 ++++
tests/qemumonitorjsontest.c | 48 +++++++++++++++++++++++++
5 files changed, 196 insertions(+)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 2e92f8c..cc22123 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -694,6 +694,39 @@ int qemuMonitorGetObjectListPaths(qemuMonitorPtr mon,
void qemuMonitorListPathFree(qemuMonitorListPathPtr paths);
+/* Flags for the 'type' field in _qemuMonitorObjectProperty */
+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
+} qemuMonitorObjectPropertyType;
+
+typedef struct _qemuMonitorObjectProperty qemuMonitorObjectProperty;
+typedef qemuMonitorObjectProperty *qemuMonitorObjectPropertyPtr;
+struct _qemuMonitorObjectProperty {
+ int type; /* qemuMonitorObjectPropertyType */
+ union {
+ bool b;
+ int i;
+ long long l;
+ unsigned int ui;
+ unsigned long long ul;
+ double d;
+ char *str;
+ } val;
+};
I think the struct/enum should be kept in qemu_monitor_json.h
+
+int qemuMonitorGetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorObjectPropertyPtr prop);
and not expose this method in qemu_monitor.h
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 :|
7:03 a.m.
New subject: [libvirt] [PATCH 2/8] Add qemuMonitorGetObjectProperty() method for QMP qom-get command
On 02.07.2013 15:39, John Ferlan wrote:
Add a new qemuMonitorGetObjectProperty() method to support
invocation
of the 'qom-get' JSON monitor command with a provided path, property, and
expected data type return. The qemuMonitorObjectProperty 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.c | 23 ++++++++++++
src/qemu/qemu_monitor.h | 33 +++++++++++++++++
src/qemu/qemu_monitor_json.c | 86 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 6 ++++
tests/qemumonitorjsontest.c | 48 +++++++++++++++++++++++++
5 files changed, 196 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index f175250..6737a63 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3468,6 +3468,29 @@ void qemuMonitorListPathFree(qemuMonitorListPathPtr paths)
}
+int qemuMonitorGetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorObjectPropertyPtr prop)
+{
+ VIR_DEBUG("mon=%p path=%s property=%s prop=%p type=%d",
+ mon, path, property, prop, prop->type);
+ 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;
+ }
+
+ return qemuMonitorJSONGetObjectProperty(mon, path, property, prop);
+}
+
+
int qemuMonitorGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 2e92f8c..cc22123 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -694,6 +694,39 @@ int qemuMonitorGetObjectListPaths(qemuMonitorPtr mon,
void qemuMonitorListPathFree(qemuMonitorListPathPtr paths);
+/* Flags for the 'type' field in _qemuMonitorObjectProperty */
+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
+} qemuMonitorObjectPropertyType;
+
+typedef struct _qemuMonitorObjectProperty qemuMonitorObjectProperty;
+typedef qemuMonitorObjectProperty *qemuMonitorObjectPropertyPtr;
+struct _qemuMonitorObjectProperty {
+ int type; /* qemuMonitorObjectPropertyType */
+ union {
+ bool b;
+ int i;
+ long long l;
+ unsigned int ui;
+ unsigned long long ul;
+ double d;
+ char *str;
+ } val;
+};
+
+int qemuMonitorGetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorObjectPropertyPtr prop);
+
int qemuMonitorGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 7d0cc85..a725903 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4626,6 +4626,92 @@ cleanup:
return ret;
}
+
+int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorObjectPropertyPtr 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)
Lose the 'tmp' here as VIR_STRDUP will check it for NULL anyway. In
fact, these lines can be rewritten into a single line:
ret = VIR_STRDUP(prop->val.str, tmp);
+ 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;
+}
+
+
Michal
7:12 a.m.
New subject: [libvirt] [PATCH 2/8] Add qemuMonitorGetObjectProperty() method for QMP qom-get command
On 07/03/2013 02:03 PM, Michal Privoznik wrote:
On 02.07.2013 15:39, John Ferlan wrote:
> + case QEMU_MONITOR_OBJECT_PROPERTY_STRING:
> + tmp = virJSONValueGetString(data);
> + if (tmp && VIR_STRDUP(prop->val.str, tmp) < 0)
Lose the 'tmp' here as VIR_STRDUP will check it for NULL anyway. In
fact, these lines can be rewritten into a single line:
ret = VIR_STRDUP(prop->val.str, tmp);
> + goto cleanup;
> + if (tmp)
> + ret = 0;
> + break;
Not at all. Before it returns 0 if GetString returned something.
After your change, ret will be 1 in that case, and 0 if GetString returned
nothing.
Jan
7:20 a.m.
New subject: [libvirt] [PATCH 2/8] Add qemuMonitorGetObjectProperty() method for QMP qom-get command
On 03.07.2013 14:12, Ján Tomko wrote:
On 07/03/2013 02:03 PM, Michal Privoznik wrote:
> On 02.07.2013 15:39, John Ferlan wrote:
>> + case QEMU_MONITOR_OBJECT_PROPERTY_STRING:
>> + tmp = virJSONValueGetString(data);
>> + if (tmp && VIR_STRDUP(prop->val.str, tmp) < 0)
>
> Lose the 'tmp' here as VIR_STRDUP will check it for NULL anyway. In
> fact, these lines can be rewritten into a single line:
>
> ret = VIR_STRDUP(prop->val.str, tmp);
>
>> + goto cleanup;
>> + if (tmp)
>> + ret = 0;
>> + break;
Not at all. Before it returns 0 if GetString returned something.
After your change, ret will be 1 in that case, and 0 if GetString returned
nothing.
Jan
But if you take a look a few lines below, we will rewrite ret = 0
anyway. The only exception is if VIR_STRDUP fails. Which I should have
checked, right.
Michal
12:10 p.m.
New subject: [libvirt] [PATCH 2/8] Add qemuMonitorGetObjectProperty() method for QMP qom-get command
On 07/03/2013 08:20 AM, Michal Privoznik wrote:
On 03.07.2013 14:12, Ján Tomko wrote:
> On 07/03/2013 02:03 PM, Michal Privoznik wrote:
>> On 02.07.2013 15:39, John Ferlan wrote:
>>> + case QEMU_MONITOR_OBJECT_PROPERTY_STRING:
>>> + tmp = virJSONValueGetString(data);
>>> + if (tmp && VIR_STRDUP(prop->val.str, tmp) < 0)
>>
>> Lose the 'tmp' here as VIR_STRDUP will check it for NULL anyway. In
>> fact, these lines can be rewritten into a single line:
>>
>> ret = VIR_STRDUP(prop->val.str, tmp);
>>
>>> + goto cleanup;
>>> + if (tmp)
>>> + ret = 0;
>>> + break;
>
> Not at all. Before it returns 0 if GetString returned something.
> After your change, ret will be 1 in that case, and 0 if GetString returned
> nothing.
>
> Jan
>
But if you take a look a few lines below, we will rewrite ret = 0
anyway. The only exception is if VIR_STRDUP fails. Which I should have
checked, right.
Michal
I didn't want VIR_STRDUP() to be called if 'data' wasn't a string. That
is - I don't want an empty "" returned. That allows the error to be
reported that there's invalid data.
Although it seems the qemuMonitorJSONHumanCommandWithFd() does an
extraneous check for VIR_STRDUP():
data = virJSONValueGetString(obj);
if (VIR_STRDUP(*reply_str, data ? data : "") < 0)
goto cleanup;
Not part of this, but I was checking callers for virJSONValueGetString()
John
8:39 a.m.
New subject: [libvirt] [PATCH 3/8] Add qemuMonitorSetObjectProperty() method for QMP qom-set command
Add a new qemuMonitorSetObjectProperty() 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.c | 23 ++++++++++++++++
src/qemu/qemu_monitor.h | 5 ++++
src/qemu/qemu_monitor_json.c | 62 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 6 +++++
tests/qemumonitorjsontest.c | 59 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 155 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 6737a63..c46b1e5 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3491,6 +3491,29 @@ int qemuMonitorGetObjectProperty(qemuMonitorPtr mon,
}
+int qemuMonitorSetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorObjectPropertyPtr prop)
+{
+ VIR_DEBUG("mon=%p path=%s property=%s prop=%p type=%d",
+ mon, path, property, prop, prop->type);
+ 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;
+ }
+
+ return qemuMonitorJSONSetObjectProperty(mon, path, property, prop);
+}
+
+
int qemuMonitorGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index cc22123..b822b97 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -727,6 +727,11 @@ int qemuMonitorGetObjectProperty(qemuMonitorPtr mon,
const char *property,
qemuMonitorObjectPropertyPtr prop);
+int qemuMonitorSetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorObjectPropertyPtr prop);
+
int qemuMonitorGetObjectProps(qemuMonitorPtr mon,
const char *type,
char ***props);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a725903..c599626 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4712,6 +4712,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,
+ qemuMonitorObjectPropertyPtr 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 63807df..1da4c44 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -344,6 +344,12 @@ int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
qemuMonitorObjectPropertyPtr prop)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorObjectPropertyPtr 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 4544676..c0987c6 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -718,6 +718,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;
+ qemuMonitorObjectProperty 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(qemuMonitorObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN;
+ prop.val.b = true;
+ if (qemuMonitorSetObjectProperty(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(qemuMonitorObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN;
+ if (qemuMonitorGetObjectProperty(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)
{
@@ -748,6 +806,7 @@ mymain(void)
DO_TEST(GetCommandLineOptionParameters);
DO_TEST(GetListPaths);
DO_TEST(GetObjectProperty);
+ DO_TEST(SetObjectProperty);
virObjectUnref(xmlopt);
--
1.8.1.4
5:57 a.m.
New subject: [libvirt] [PATCH 3/8] Add qemuMonitorSetObjectProperty() method for QMP qom-set command
On Tue, Jul 02, 2013 at 09:39:21AM -0400, John Ferlan wrote:
Add a new qemuMonitorSetObjectProperty() 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.c | 23 ++++++++++++++++
src/qemu/qemu_monitor.h | 5 ++++
src/qemu/qemu_monitor_json.c | 62 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 6 +++++
tests/qemumonitorjsontest.c | 59 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 155 insertions(+)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index cc22123..b822b97 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -727,6 +727,11 @@ int qemuMonitorGetObjectProperty(qemuMonitorPtr mon,
const char *property,
qemuMonitorObjectPropertyPtr prop);
+int qemuMonitorSetObjectProperty(qemuMonitorPtr mon,
+ const char *path,
+ const char *property,
+ qemuMonitorObjectPropertyPtr prop);
+
Again, I think we should not expose this method in qemu_monitor.h
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 :|
8:39 a.m.
New subject: [libvirt] [PATCH 4/8] 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/schemas/domaincommon.rng | 7 +++++++
src/conf/domain_conf.c | 27 +++++++++++++++++++++++----
src/conf/domain_conf.h | 1 +
3 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index cf82878..53e707c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2915,6 +2915,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 011de71..cd7dbee 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8571,10 +8571,12 @@ error:
static virDomainMemballoonDefPtr
virDomainMemballoonDefParseXML(const xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
unsigned int flags)
{
char *model;
virDomainMemballoonDefPtr def;
+ xmlNodePtr save = ctxt->node;
if (VIR_ALLOC(def) < 0) {
virReportOOMError();
@@ -8587,18 +8589,24 @@ virDomainMemballoonDefParseXML(const xmlNodePtr node,
_("balloon memory must contain model name"));
goto error;
}
+
if ((def->model = virDomainMemballoonModelTypeFromString(model)) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown memory balloon model '%s'"), model);
goto error;
}
+ ctxt->node = node;
+ if (virXPathInt("string(./stats/@period)", ctxt, &def->period) <
0)
+ def->period = 0;
+
if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
goto error;
cleanup:
VIR_FREE(model);
+ ctxt->node = save;
return def;
error:
@@ -11911,7 +11919,7 @@ virDomainDefParseXML(xmlDocPtr xml,
}
if (n > 0) {
virDomainMemballoonDefPtr memballoon =
- virDomainMemballoonDefParseXML(nodes[0], flags);
+ virDomainMemballoonDefParseXML(nodes[0], ctxt, flags);
if (!memballoon)
goto error;
@@ -15080,6 +15088,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
unsigned int flags)
{
const char *model = virDomainMemballoonModelTypeToString(def->model);
+ bool noopts = true;
if (!model) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -15093,11 +15102,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 3817e37..405903f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1527,6 +1527,7 @@ enum {
struct _virDomainMemballoonDef {
int model;
virDomainDeviceInfo info;
+ int period; /* seconds between collections */
};
struct _virDomainNVRAMDef {
--
1.8.1.4
5:58 a.m.
New subject: [libvirt] [PATCH 4/8] Add 'period' for Memballoon statistics gathering capability
On Tue, Jul 02, 2013 at 09:39:22AM -0400, John Ferlan wrote:
Add a period in seconds to allow/enable statistics gathering from
the
Balloon driver for 'virsh dommemstat <domain>'.
---
docs/schemas/domaincommon.rng | 7 +++++++
src/conf/domain_conf.c | 27 +++++++++++++++++++++++----
src/conf/domain_conf.h | 1 +
3 files changed, 31 insertions(+), 4 deletions(-)
It is preferred to include the docs for formatdomain.html.in in the
same patch that introduces the rng/parser. So move the docs out of
patch 8, into here.
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 :|
7:03 a.m.
New subject: [libvirt] [PATCH 4/8] Add 'period' for Memballoon statistics gathering capability
On 02.07.2013 15:39, John Ferlan wrote:
Add a period in seconds to allow/enable statistics gathering from
the
Balloon driver for 'virsh dommemstat <domain>'.
---
docs/schemas/domaincommon.rng | 7 +++++++
src/conf/domain_conf.c | 27 +++++++++++++++++++++++----
src/conf/domain_conf.h | 1 +
3 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index cf82878..53e707c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2915,6 +2915,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 011de71..cd7dbee 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8571,10 +8571,12 @@ error:
static virDomainMemballoonDefPtr
virDomainMemballoonDefParseXML(const xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
unsigned int flags)
{
char *model;
virDomainMemballoonDefPtr def;
+ xmlNodePtr save = ctxt->node;
if (VIR_ALLOC(def) < 0) {
virReportOOMError();
@@ -8587,18 +8589,24 @@ virDomainMemballoonDefParseXML(const xmlNodePtr node,
_("balloon memory must contain model name"));
goto error;
}
+
if ((def->model = virDomainMemballoonModelTypeFromString(model)) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown memory balloon model '%s'"),
model);
goto error;
}
+ ctxt->node = node;
+ if (virXPathInt("string(./stats/@period)", ctxt, &def->period) <
0)
+ def->period = 0;
This is silently ignoring invalid integer value in @period. Moreover,
it's merging two different cases into one: invalid integer and @period
not present. After all, setting def->period to zero is done by
VIR_ALLOC. So I think we must distinguish if virXPathInt returned -1
(@period not present in XML) or -2(@period is not valid integer). And we
must not forget to check if user hasn't entered period < 0.
+
if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
goto error;
cleanup:
VIR_FREE(model);
+ ctxt->node = save;
return def;
error:
@@ -11911,7 +11919,7 @@ virDomainDefParseXML(xmlDocPtr xml,
}
if (n > 0) {
virDomainMemballoonDefPtr memballoon =
- virDomainMemballoonDefParseXML(nodes[0], flags);
+ virDomainMemballoonDefParseXML(nodes[0], ctxt, flags);
if (!memballoon)
goto error;
@@ -15080,6 +15088,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
unsigned int flags)
{
const char *model = virDomainMemballoonModelTypeToString(def->model);
+ bool noopts = true;
if (!model) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -15093,11 +15102,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 3817e37..405903f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1527,6 +1527,7 @@ enum {
struct _virDomainMemballoonDef {
int model;
virDomainDeviceInfo info;
+ int period; /* seconds between collections */
};
struct _virDomainNVRAMDef {
Michal
8:39 a.m.
New subject: [libvirt] [PATCH 5/8] 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_domain.c | 1 +
src/qemu/qemu_domain.h | 2 +
src/qemu/qemu_process.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 165 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8d79066..5aaf1e1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -244,6 +244,7 @@ qemuDomainObjPrivateFree(void *data)
VIR_FREE(priv->vcpupids);
VIR_FREE(priv->lockState);
VIR_FREE(priv->origname);
+ VIR_FREE(priv->balloonpath);
virChrdevFree(priv->devs);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 068a4c3..005fd0f 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -161,6 +161,8 @@ struct _qemuDomainObjPrivate {
char *origname;
int nbdPort; /* Port used for migration with NBD */
+ char *balloonpath;
+
virChrdevsPtr devs;
qemuDomainCleanupCallback *cleanupCallbacks;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ac5ffcf..9a2add1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1564,6 +1564,150 @@ qemuProcessLookupPTYs(virDomainChrDefPtr *devices,
return 0;
}
+/* 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
+qemuProcessFindBalloonObjectPath(qemuMonitorPtr mon,
+ virDomainObjPtr vm,
+ const char *curpath,
+ char **balloonpath)
+{
+ int i,j;
+ int npaths = 0;
+ int nprops = 0;
+ int ret = 0;
+ char *nextpath = NULL;
+ qemuMonitorListPathPtr *paths = NULL;
+ qemuMonitorListPathPtr *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 (*balloonpath)
+ return 1;
+
+ VIR_DEBUG("Searching for Balloon Object Path starting at %s", curpath);
+
+ npaths = qemuMonitorGetObjectListPaths(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 = qemuMonitorGetObjectListPaths(mon, nextpath, &bprops);
+ for (j = 0; j < nprops; j++) {
+ if (STREQ(bprops[j]->name, "guest-stats-polling-interval"))
{
+ *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 = qemuProcessFindBalloonObjectPath(mon, vm, nextpath,
+ balloonpath);
+ }
+ }
+
+cleanup:
+ for (i = 0; i < npaths; i++)
+ qemuMonitorListPathFree(paths[i]);
+ VIR_FREE(paths);
+ for (j = 0; j < nprops; j++)
+ qemuMonitorListPathFree(bprops[j]);
+ VIR_FREE(bprops);
+ VIR_FREE(nextpath);
+ return ret;
+}
+
+/*
+ * Using the provided balloonpath, determine if we need to set the
+ * collection interval property to enable statistics gathering.
+ *
+ * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
+ */
+static void
+qemuProcessUpdateBalloonStatsPeriod(qemuMonitorPtr mon,
+ char *balloonpath,
+ virDomainObjPtr vm)
+{
+ qemuMonitorObjectProperty prop;
+
+ /* Get the current value of the stats polling interval */
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
+ if (qemuMonitorGetObjectProperty(mon, balloonpath,
+ "guest-stats-polling-interval",
+ &prop) < 0) {
+ VIR_DEBUG("Failed to get polling interval for balloon driver");
+ return;
+ }
+
+ VIR_DEBUG("memballoon period=%d
'guest-stats-polling-interval'=%d",
+ vm->def->memballoon->period, prop.val.i);
+
+ /* Same value - no need to set */
+ if (vm->def->memballoon->period == prop.val.i)
+ return;
+
+ /* Set to the value in memballoon (could enable or disable) */
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
+ prop.val.i = vm->def->memballoon->period;
+ if (qemuMonitorSetObjectProperty(mon, balloonpath,
+ "guest-stats-polling-interval",
+ &prop) < 0)
+ VIR_DEBUG("Failed to set polling interval for balloon driver");
+
+}
+
static int
qemuProcessFindCharDevicePTYsMonitor(virDomainObjPtr vm,
virQEMUCapsPtr qemuCaps,
@@ -3047,6 +3191,12 @@ qemuProcessReconnect(void *opaque)
if (qemuProcessRecoverJob(driver, obj, conn, &oldjob) < 0)
goto error;
+ qemuDomainObjEnterMonitor(driver, obj);
+ if (qemuProcessFindBalloonObjectPath(priv->mon, obj, "/",
+ &priv->balloonpath) == 1)
+ qemuProcessUpdateBalloonStatsPeriod(priv->mon, priv->balloonpath, obj);
+ qemuDomainObjExitMonitor(driver, obj);
+
/* update domain state XML with possibly updated state in virDomainObj */
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, obj) < 0)
goto error;
@@ -3867,6 +4017,9 @@ int qemuProcessStart(virConnectPtr conn,
goto cleanup;
}
qemuDomainObjEnterMonitor(driver, vm);
+ if (qemuProcessFindBalloonObjectPath(priv->mon, vm, "/",
+ &priv->balloonpath) == 1)
+ qemuProcessUpdateBalloonStatsPeriod(priv->mon, priv->balloonpath, vm);
if (qemuMonitorSetBalloon(priv->mon, cur_balloon) < 0) {
qemuDomainObjExitMonitor(driver, vm);
goto cleanup;
@@ -4400,11 +4553,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
+ qemuDomainObjEnterMonitor(driver, vm);
+ if (qemuProcessFindBalloonObjectPath(priv->mon, vm, "/",
+ &priv->balloonpath) == 1)
+ qemuProcessUpdateBalloonStatsPeriod(priv->mon,
+ priv->balloonpath, vm);
+ 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
6:02 a.m.
New subject: [libvirt] [PATCH 5/8] Determine whether to start balloon memory stats gathering.
On Tue, Jul 02, 2013 at 09:39:23AM -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_domain.c | 1 +
src/qemu/qemu_domain.h | 2 +
src/qemu/qemu_process.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 165 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8d79066..5aaf1e1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -244,6 +244,7 @@ qemuDomainObjPrivateFree(void *data)
VIR_FREE(priv->vcpupids);
VIR_FREE(priv->lockState);
VIR_FREE(priv->origname);
+ VIR_FREE(priv->balloonpath);
virChrdevFree(priv->devs);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 068a4c3..005fd0f 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -161,6 +161,8 @@ struct _qemuDomainObjPrivate {
char *origname;
int nbdPort; /* Port used for migration with NBD */
+ char *balloonpath;
+
virChrdevsPtr devs;
I think I'd prefer this to be kept as a private property
in the struct qemuMonitor.
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ac5ffcf..9a2add1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1564,6 +1564,150 @@ qemuProcessLookupPTYs(virDomainChrDefPtr *devices,
return 0;
}
+/* 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
+qemuProcessFindBalloonObjectPath(qemuMonitorPtr mon,
+ virDomainObjPtr vm,
+ const char *curpath,
+ char **balloonpath)
+{
+ int i,j;
+ int npaths = 0;
+ int nprops = 0;
+ int ret = 0;
+ char *nextpath = NULL;
+ qemuMonitorListPathPtr *paths = NULL;
+ qemuMonitorListPathPtr *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 (*balloonpath)
+ return 1;
+
+ VIR_DEBUG("Searching for Balloon Object Path starting at %s", curpath);
+
+ npaths = qemuMonitorGetObjectListPaths(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 = qemuMonitorGetObjectListPaths(mon, nextpath, &bprops);
+ for (j = 0; j < nprops; j++) {
+ if (STREQ(bprops[j]->name, "guest-stats-polling-interval"))
{
+ *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 = qemuProcessFindBalloonObjectPath(mon, vm, nextpath,
+ balloonpath);
+ }
+ }
+
+cleanup:
+ for (i = 0; i < npaths; i++)
+ qemuMonitorListPathFree(paths[i]);
+ VIR_FREE(paths);
+ for (j = 0; j < nprops; j++)
+ qemuMonitorListPathFree(bprops[j]);
+ VIR_FREE(bprops);
+ VIR_FREE(nextpath);
+ return ret;
+}
I think this method should be kept in qemu_monitor.c and made static
to that file
We can then populate it on first use.
+/*
+ * Using the provided balloonpath, determine if we need to set the
+ * collection interval property to enable statistics gathering.
+ *
+ * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
+ */
+static void
+qemuProcessUpdateBalloonStatsPeriod(qemuMonitorPtr mon,
+ char *balloonpath,
+ virDomainObjPtr vm)
+{
+ qemuMonitorObjectProperty prop;
+
+ /* Get the current value of the stats polling interval */
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
+ if (qemuMonitorGetObjectProperty(mon, balloonpath,
+ "guest-stats-polling-interval",
+ &prop) < 0) {
+ VIR_DEBUG("Failed to get polling interval for balloon driver");
+ return;
+ }
+
+ VIR_DEBUG("memballoon period=%d
'guest-stats-polling-interval'=%d",
+ vm->def->memballoon->period, prop.val.i);
+
+ /* Same value - no need to set */
+ if (vm->def->memballoon->period == prop.val.i)
+ return;
+
+ /* Set to the value in memballoon (could enable or disable) */
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
+ prop.val.i = vm->def->memballoon->period;
+ if (qemuMonitorSetObjectProperty(mon, balloonpath,
+ "guest-stats-polling-interval",
+ &prop) < 0)
+ VIR_DEBUG("Failed to set polling interval for balloon driver");
+
+}
Since I recommended that we don't expose qemuMonitorGetObjectProperty
or qemuMonitorSetObjectProperty in qemu_monitor.h, what you'll need
todo here is move this code into qemu_monitor.c defining a method
something like
int qemuMonitorSetBalloonStatsPeriod(qemuMonitorPtr mon,
int period);
This method can populate the balloonpath field in qemuMOnitorPtr struct
on first use.
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 :|
8:39 a.m.
New subject: [libvirt] [PATCH 6/8] 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.h | 4 +++
src/qemu/qemu_monitor_json.c | 76 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index b822b97..4199160 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -703,6 +703,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
} qemuMonitorObjectPropertyType;
@@ -711,6 +712,8 @@ typedef struct _qemuMonitorObjectProperty qemuMonitorObjectProperty;
typedef qemuMonitorObjectProperty *qemuMonitorObjectPropertyPtr;
struct _qemuMonitorObjectProperty {
int type; /* qemuMonitorObjectPropertyType */
+ int curelems; /* Current number elements in **ptr array */
+ int maxelems; /* Maximum number elements allowed in any **ptr array */
union {
bool b;
int i;
@@ -719,6 +722,7 @@ struct _qemuMonitorObjectProperty {
unsigned long long ul;
double d;
char *str;
+ void **ptr;
} val;
};
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index c599626..49001a8 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4627,6 +4627,78 @@ cleanup:
}
+/* 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++; \
+ } \
+ } \
+ }
+
+static int
+qemuMonitorJSONGetBalloonStats(virJSONValuePtr data,
+ qemuMonitorObjectPropertyPtr prop)
+{
+ int ret = -1;
+ unsigned long long mem;
+ virJSONValuePtr statsdata;
+ virDomainMemoryStatPtr stat = (virDomainMemoryStatPtr)prop->val.ptr;
+
+ VIR_DEBUG("Address of found stat = %p", stat);
+
+ if (!(statsdata = virJSONValueObjectGet(data, "stats"))) {
+ VIR_DEBUG("data does not include 'stats'");
+ goto cleanup;
+ }
+
+ 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);
+
+ ret = 0;
+
+cleanup:
+ virJSONValueFree(statsdata);
+ return ret;
+}
+
int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
const char *path,
const char *property,
@@ -4689,6 +4761,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,
@@ -4752,6 +4827,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,
--
1.8.1.4
6:06 a.m.
New subject: [libvirt] [PATCH 6/8] Add capability to fetch balloon stats
On Tue, Jul 02, 2013 at 09:39:24AM -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.h | 4 +++
src/qemu/qemu_monitor_json.c | 76 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index b822b97..4199160 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -703,6 +703,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
} qemuMonitorObjectPropertyType;
@@ -711,6 +712,8 @@ typedef struct _qemuMonitorObjectProperty qemuMonitorObjectProperty;
typedef qemuMonitorObjectProperty *qemuMonitorObjectPropertyPtr;
struct _qemuMonitorObjectProperty {
int type; /* qemuMonitorObjectPropertyType */
+ int curelems; /* Current number elements in **ptr array */
+ int maxelems; /* Maximum number elements allowed in any **ptr array */
union {
bool b;
int i;
@@ -719,6 +722,7 @@ struct _qemuMonitorObjectProperty {
unsigned long long ul;
double d;
char *str;
+ void **ptr;
Hmm, not sure I really like the look of this opaque pointer.
} val;
};
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index c599626..49001a8 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4627,6 +4627,78 @@ cleanup:
}
+/* 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++; \
+ } \
+ } \
+ }
+
+static int
+qemuMonitorJSONGetBalloonStats(virJSONValuePtr data,
+ qemuMonitorObjectPropertyPtr prop)
+{
+ int ret = -1;
+ unsigned long long mem;
+ virJSONValuePtr statsdata;
+ virDomainMemoryStatPtr stat = (virDomainMemoryStatPtr)prop->val.ptr;
+
+ VIR_DEBUG("Address of found stat = %p", stat);
+
+ if (!(statsdata = virJSONValueObjectGet(data, "stats"))) {
+ VIR_DEBUG("data does not include 'stats'");
+ goto cleanup;
+ }
+
+ 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);
+
+ ret = 0;
+
+cleanup:
+ virJSONValueFree(statsdata);
+ return ret;
+}
You'll want an '#undef GET_BALLOON_STATS' call.
+
int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
const char *path,
const char *property,
@@ -4689,6 +4761,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,
@@ -4752,6 +4827,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,
We already have a method qemuMonitor{JSON}GetMemoryStats which was using
the legacy 'query-balloon' interface that QEMU has removed now. IMHO we
should just update that existing API to fetch the data using the new
QOM based monitor commands. Then existing code using that API in the
QEMU driver will "just work".
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 :|
8:39 a.m.
New subject: [libvirt] [PATCH 7/8] If available fetch the balloon driver memory stats
In the 'virsh dommemstats <domain>' qemu driver code, if we have a
balloonpath and we hadn't already collected the data (somehow) from the
"query-balloon" command, then use qom-get to attempt to fill in the
domain memory stats data from the balloon driver.
---
src/qemu/qemu_driver.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f51e766..c9a66ff 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9499,6 +9499,7 @@ qemuDomainMemoryStats(virDomainPtr dom,
virQEMUDriverPtr driver = dom->conn->privateData;
virDomainObjPtr vm;
int ret = -1;
+ qemuMonitorObjectProperty prop;
virCheckFlags(0, -1);
@@ -9518,6 +9519,22 @@ qemuDomainMemoryStats(virDomainPtr dom,
qemuDomainObjPrivatePtr priv = vm->privateData;
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorGetMemoryStats(priv->mon, stats, nr_stats);
+
+ /* The above call should only return the 'actual' memory due to
+ * changes that disabled statistics gathering from query-balloon.
+ * If so, let's check if we have a path to the balloon device
+ * and then try to gather more stats from there if possible
+ */
+ if ((ret == 0 || ret == 1) && priv->balloonpath) {
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BALLOON_STATS;
+ prop.curelems = ret;
+ prop.maxelems = nr_stats;
+ prop.val.ptr = (void **)stats;
+ if (qemuMonitorGetObjectProperty(priv->mon, priv->balloonpath,
+ "guest-stats", &prop) == 0)
+ ret = prop.curelems;
+ }
qemuDomainObjExitMonitor(driver, vm);
if (ret >= 0 && ret < nr_stats) {
--
1.8.1.4
6:07 a.m.
New subject: [libvirt] [PATCH 7/8] If available fetch the balloon driver memory stats
On Tue, Jul 02, 2013 at 09:39:25AM -0400, John Ferlan wrote:
In the 'virsh dommemstats <domain>' qemu driver code,
if we have a
balloonpath and we hadn't already collected the data (somehow) from the
"query-balloon" command, then use qom-get to attempt to fill in the
domain memory stats data from the balloon driver.
---
src/qemu/qemu_driver.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f51e766..c9a66ff 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9499,6 +9499,7 @@ qemuDomainMemoryStats(virDomainPtr dom,
virQEMUDriverPtr driver = dom->conn->privateData;
virDomainObjPtr vm;
int ret = -1;
+ qemuMonitorObjectProperty prop;
virCheckFlags(0, -1);
@@ -9518,6 +9519,22 @@ qemuDomainMemoryStats(virDomainPtr dom,
qemuDomainObjPrivatePtr priv = vm->privateData;
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorGetMemoryStats(priv->mon, stats, nr_stats);
+
+ /* The above call should only return the 'actual' memory due to
+ * changes that disabled statistics gathering from query-balloon.
+ * If so, let's check if we have a path to the balloon device
+ * and then try to gather more stats from there if possible
+ */
+ if ((ret == 0 || ret == 1) && priv->balloonpath) {
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BALLOON_STATS;
+ prop.curelems = ret;
+ prop.maxelems = nr_stats;
+ prop.val.ptr = (void **)stats;
+ if (qemuMonitorGetObjectProperty(priv->mon, priv->balloonpath,
+ "guest-stats", &prop) == 0)
+ ret = prop.curelems;
+ }
If we hide all the QOM stuff inside the existing qemuMonitorGetMemoryStats()
API, then this patch just goes away entirely.
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 :|
8:39 a.m.
New subject: [libvirt] [PATCH 8/8] Allow balloon driver collection to be adjusted dynamically
Extend the virDomainSetMemeoryFlags() to accept a 'VIR_DOMAIN_MEM_PERIOD'
which will be used to dynamically set the collection period for the balloon
driver via a 'virsh dommemstat <domain> --period <value>' command.
Add
the --current, --live, & --config options to dommemstat.
---
docs/formatdomain.html.in | 14 +++++++++
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 8 ++++-
src/qemu/qemu_driver.c | 44 +++++++++++++++++++++++++++-
tools/virsh-domain-monitor.c | 70 ++++++++++++++++++++++++++++++++++++++++++--
5 files changed, 132 insertions(+), 5 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index cc4c5ea..0ec256d 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4612,6 +4612,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>
@@ -4629,6 +4630,19 @@ 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. 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. 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>
</dl>
<h4><a name="elementsRng">Random number generator
device</a></h4>
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index b791125..1fc6e82 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1844,6 +1844,7 @@ typedef enum {
/* Additionally, these flags may be bitwise-OR'd in. */
VIR_DOMAIN_MEM_MAXIMUM = (1 << 2), /* affect Max rather than current */
+ VIR_DOMAIN_MEM_PERIOD = (1 << 3), /* set dommemstats period */
} virDomainMemoryModFlags;
diff --git a/src/libvirt.c b/src/libvirt.c
index bc1694a..81745bb 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -3738,6 +3738,9 @@ error:
* on whether just live or both live and persistent state is changed.
* If VIR_DOMAIN_MEM_MAXIMUM is set, the change affects domain's maximum memory
* size rather than current memory size.
+ * If VIR_DOMAIN_MEM_PERIOD is set, the domain memory statistics collection
+ * period for the balloon driver will be adjusted. Use 0 to disable and
+ * a positive value to enable.
* Not all hypervisors can support all flag combinations.
*
* Returns 0 in case of success, -1 in case of failure.
@@ -3763,7 +3766,10 @@ virDomainSetMemoryFlags(virDomainPtr domain, unsigned long memory,
virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
goto error;
}
- virCheckNonZeroArgGoto(memory, error);
+
+ /* This can be non zero only for setting the balloon collection period */
+ if (!(flags & VIR_DOMAIN_MEM_PERIOD))
+ virCheckNonZeroArgGoto(memory, error);
conn = domain->conn;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c9a66ff..b76e634 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2166,7 +2166,8 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG |
- VIR_DOMAIN_MEM_MAXIMUM, -1);
+ VIR_DOMAIN_MEM_MAXIMUM |
+ VIR_DOMAIN_MEM_PERIOD, -1);
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
@@ -2205,6 +2206,47 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
goto endjob;
}
+ } else if (flags & VIR_DOMAIN_MEM_PERIOD) {
+ /* Set the balloon driver collection interval */
+ priv = vm->privateData;
+ if (!priv->balloonpath) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("memory balloon driver not defined or "
+ "not virtio model"));
+ goto endjob;
+ }
+
+ if (newmem != (int)newmem) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("period value=%lu does not match set value=%d"),
+ newmem, (int)newmem);
+ goto endjob;
+ }
+
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ qemuMonitorObjectProperty prop;
+
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
+ prop.val.i = newmem;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ r = qemuMonitorSetObjectProperty(priv->mon, priv->balloonpath,
+ "guest-stats-polling-interval",
+ &prop);
+ 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 = newmem;
+ ret = virDomainSaveConfig(cfg->configDir, persistentDef);
+ goto endjob;
+ }
} else {
/* resize the current memory */
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 3ba829c..f123247 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -306,6 +306,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}
};
@@ -316,15 +333,60 @@ 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;
+ /* none of the options were specified - choose defaults based on state */
+ if (!current && !live && !config) {
+ if (virDomainIsActive(dom) == 1)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+ else
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ }
+
+ /* 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;
+ }
+
+ flags |= VIR_DOMAIN_MEM_PERIOD;
+ if (virDomainSetMemoryFlags(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++) {
@@ -346,8 +408,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
6:09 a.m.
New subject: [libvirt] [PATCH 8/8] Allow balloon driver collection to be adjusted dynamically
On Tue, Jul 02, 2013 at 09:39:26AM -0400, John Ferlan wrote:
Extend the virDomainSetMemeoryFlags() to accept a
'VIR_DOMAIN_MEM_PERIOD'
which will be used to dynamically set the collection period for the balloon
driver via a 'virsh dommemstat <domain> --period <value>' command.
Add
the --current, --live, & --config options to dommemstat.
While what you've done here works from a technical POV, I'm not a
fan of overloading the semantics of the API using a flag. Tuning
memory stat collection period has no semantic overlap with tuning
of memory limits. So I'm inclined to say that we should have a
new dedicated API for tuning the memory stats period.
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 :|
7:03 a.m.
New subject: [libvirt] [PATCH 8/8] Allow balloon driver collection to be adjusted dynamically
On 02.07.2013 15:39, John Ferlan wrote:
Extend the virDomainSetMemeoryFlags() to accept a
'VIR_DOMAIN_MEM_PERIOD'
which will be used to dynamically set the collection period for the balloon
driver via a 'virsh dommemstat <domain> --period <value>' command.
Add
the --current, --live, & --config options to dommemstat.
---
docs/formatdomain.html.in | 14 +++++++++
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 8 ++++-
src/qemu/qemu_driver.c | 44 +++++++++++++++++++++++++++-
tools/virsh-domain-monitor.c | 70 ++++++++++++++++++++++++++++++++++++++++++--
5 files changed, 132 insertions(+), 5 deletions(-)
diff --git a/tools/virsh-domain-monitor.c
b/tools/virsh-domain-monitor.c
index 3ba829c..f123247 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -306,6 +306,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}
};
@@ -316,15 +333,60 @@ 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;
+ /* none of the options were specified - choose defaults based on state */
+ if (!current && !live && !config) {
+ if (virDomainIsActive(dom) == 1)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+ else
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
This should not be needed as qemu driver will the proper option for
VIR_DOMAIN_AFFECT_CURRENT. Client can't make such decision as domain is
not locked and may translate into different state.
+ }
+
+ /* 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;
+ }
+
+ flags |= VIR_DOMAIN_MEM_PERIOD;
+ if (virDomainSetMemoryFlags(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++) {
@@ -346,8 +408,10 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "rss %llu\n", stats[i].val);
}
+ ret = true;
+cleanup:
virDomainFree(dom);
- return true;
+ return ret;
}
/*
Michal
12:37 p.m.
New subject: [libvirt] [PATCH 8/8] Allow balloon driver collection to be adjusted dynamically
On 07/03/2013 08:03 AM, Michal Privoznik wrote:
On 02.07.2013 15:39, John Ferlan wrote:
> Extend the virDomainSetMemeoryFlags() to accept a 'VIR_DOMAIN_MEM_PERIOD'
> which will be used to dynamically set the collection period for the balloon
> driver via a 'virsh dommemstat <domain> --period <value>'
command. Add
> the --current, --live, & --config options to dommemstat.
> ---
> docs/formatdomain.html.in | 14 +++++++++
> include/libvirt/libvirt.h.in | 1 +
> src/libvirt.c | 8 ++++-
> src/qemu/qemu_driver.c | 44 +++++++++++++++++++++++++++-
> tools/virsh-domain-monitor.c | 70 ++++++++++++++++++++++++++++++++++++++++++--
> 5 files changed, 132 insertions(+), 5 deletions(-)
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
...snip...
>
> if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
> return false;
>
> + /* none of the options were specified - choose defaults based on state */
> + if (!current && !live && !config) {
> + if (virDomainIsActive(dom) == 1)
> + flags |= VIR_DOMAIN_AFFECT_LIVE;
> + else
> + flags |= VIR_DOMAIN_AFFECT_CONFIG;
This should not be needed as qemu driver will the proper option for
VIR_DOMAIN_AFFECT_CURRENT. Client can't make such decision as domain is
not locked and may translate into different state.
I removed the 'else' condition and merely was following other examples
for the 'if' option.
John
Michal
9:07 a.m.
On 07/02/2013 09:39 AM, John Ferlan wrote:
<...snip...>
John Ferlan (8):
Add qemuMonitorGetObjectListPaths() method for QMP qom-list command
Add qemuMonitorGetObjectProperty() method for QMP qom-get command
Add qemuMonitorSetObjectProperty() 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
If available fetch the balloon driver memory stats
Allow balloon driver collection to be adjusted dynamically
docs/formatdomain.html.in | 14 ++
docs/schemas/domaincommon.rng | 7 +
include/libvirt/libvirt.h.in | 1 +
src/conf/domain_conf.c | 27 +++-
src/conf/domain_conf.h | 1 +
src/libvirt.c | 8 +-
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_domain.h | 2 +
src/qemu/qemu_driver.c | 61 +++++++-
src/qemu/qemu_monitor.c | 79 +++++++++++
src/qemu/qemu_monitor.h | 57 ++++++++
src/qemu/qemu_monitor_json.c | 317 ++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 18 +++
src/qemu/qemu_process.c | 164 +++++++++++++++++++++-
tests/qemumonitorjsontest.c | 184 ++++++++++++++++++++++++
tools/virsh-domain-monitor.c | 70 +++++++++-
16 files changed, 1000 insertions(+), 11 deletions(-)
FYI: I have rebased to the most recently pushed patches and have removed
the virReportOOMError()'s from patch #1 and #5
John
4154
days inactive
4162
days old
27 comments
4 participants
participants (4)
-
Daniel P. Berrange -
John Ferlan -
Ján Tomko -
Michal Privoznik