Add a new qemuMonitorJSONGetObjectProperty() method to support invocation of the 'qom-get' JSON monitor command with a provided path, property, and expected data type return. The qemuMonitorJSONObjectProperty is similar to virTypedParameter; however, a future patch will extend it a bit to include a void pointer to balloon driver statistic data. The provided test will execute a qom-get on "/machine/i440fx" which will return a property "realized". --- src/qemu/qemu_monitor_json.c | 86 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 34 ++++++++++++++++++ tests/qemumonitorjsontest.c | 48 +++++++++++++++++++++++++ 3 files changed, 168 insertions(+) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 216a58c..62f2bf0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4605,6 +4605,92 @@ void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths) VIR_FREE(paths); } + +int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon, + const char *path, + const char *property, + qemuMonitorJSONObjectPropertyPtr prop) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + const char *tmp; + + if (!(cmd = qemuMonitorJSONMakeCommand("qom-get", + "s:path", path, + "s:property", property, + NULL))) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret < 0) + goto cleanup; + + ret = -1; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qom-get reply was missing return data")); + goto cleanup; + } + + switch ((qemuMonitorJSONObjectPropertyType) prop->type) { + /* Simple cases of boolean, int, long, uint, ulong, double, and string + * will receive return value as part of {"return": xxx} statement + */ + case QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN: + ret = virJSONValueGetBoolean(data, &prop->val.b); + break; + case QEMU_MONITOR_OBJECT_PROPERTY_INT: + ret = virJSONValueGetNumberInt(data, &prop->val.iv); + 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: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("qom-get invalid object property type %d"), + prop->type); + goto cleanup; + break; + } + + if (ret == -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qom-get reply was missing return data")); + goto cleanup; + } + + ret = 0; +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + + return ret; +} + + int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon, const char *type, char ***props) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index e7ce145..322e3fd 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -348,6 +348,40 @@ int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon, void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths); +/* Flags for the 'type' field in _qemuMonitorJSONObjectProperty */ +typedef enum { + QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN=1, + QEMU_MONITOR_OBJECT_PROPERTY_INT, + QEMU_MONITOR_OBJECT_PROPERTY_LONG, + QEMU_MONITOR_OBJECT_PROPERTY_UINT, + QEMU_MONITOR_OBJECT_PROPERTY_ULONG, + QEMU_MONITOR_OBJECT_PROPERTY_DOUBLE, + QEMU_MONITOR_OBJECT_PROPERTY_STRING, + + QEMU_MONITOR_OBJECT_PROPERTY_LAST +} qemuMonitorJSONObjectPropertyType; + +typedef struct _qemuMonitorJSONObjectProperty qemuMonitorJSONObjectProperty; +typedef qemuMonitorJSONObjectProperty *qemuMonitorJSONObjectPropertyPtr; +struct _qemuMonitorJSONObjectProperty { + int type; /* qemuMonitorJSONObjectPropertyType */ + union { + bool b; + int iv; + long long l; + unsigned int ui; + unsigned long long ul; + double d; + char *str; + } val; +}; + +int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon, + const char *path, + const char *property, + qemuMonitorJSONObjectPropertyPtr prop) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); + int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon, const char *type, char ***props) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 7841658..660f251 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -673,6 +673,53 @@ cleanup: } +/* + * This test will use a path to /machine/i440fx which should exist in order + * to ensure that the qom-get property fetch works properly. The following + * is the execution and expected return: + * + * + * { "execute": "qom-get","arguments": \ + * { "path": "/machine/i440fx","property": "realized"}} + * {"return": true} + */ +static int +testQemuMonitorJSONGetObjectProperty(const void *data) +{ + const virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNew(true, xmlopt); + int ret = -1; + qemuMonitorJSONObjectProperty prop; + + if (!test) + return -1; + + if (qemuMonitorTestAddItem(test, "qom-get", + "{ \"return\": true }") < 0) + goto cleanup; + + /* Present with path and property */ + memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty)); + prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN; + if (qemuMonitorJSONGetObjectProperty(qemuMonitorTestGetMonitor(test), + "/machine/i440fx", + "realized", + &prop) < 0) + goto cleanup; + + if (!prop.val.b) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "expected true, but false returned"); + goto cleanup; + } + + ret = 0; +cleanup: + qemuMonitorTestFree(test); + return ret; +} + + static int mymain(void) { @@ -702,6 +749,7 @@ mymain(void) DO_TEST(GetTPMModels); DO_TEST(GetCommandLineOptionParameters); DO_TEST(GetListPaths); + DO_TEST(GetObjectProperty); virObjectUnref(xmlopt); -- 1.8.1.4

Add a new qemuMonitorJSONSetObjectProperty() method to support invocation of the 'qom-set' JSON monitor command with a provided path, property, and expected data type to set. The test code uses the same "/machine/i440fx" property as the get test and attempts to set the "realized" property to "true" (which it should be set at anyway). --- src/qemu/qemu_monitor_json.c | 62 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 6 +++++ tests/qemumonitorjsontest.c | 59 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 62f2bf0..92d458c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4691,6 +4691,68 @@ cleanup: } +#define MAKE_SET_CMD(STRING, VALUE) \ + cmd = qemuMonitorJSONMakeCommand("qom-set", \ + "s:path", path, \ + "s:property", property, \ + STRING, VALUE, \ + NULL) +int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon, + const char *path, + const char *property, + qemuMonitorJSONObjectPropertyPtr prop) +{ + int ret = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + + switch ((qemuMonitorJSONObjectPropertyType) prop->type) { + /* Simple cases of boolean, int, long, uint, ulong, double, and string + * will receive return value as part of {"return": xxx} statement + */ + case QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN: + MAKE_SET_CMD("b:value", prop->val.b); + break; + case QEMU_MONITOR_OBJECT_PROPERTY_INT: + MAKE_SET_CMD("i:value", prop->val.iv); + 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: + 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; +} +#undef MAKE_SET_CMD + + 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 322e3fd..a857d86 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -382,6 +382,12 @@ int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon, qemuMonitorJSONObjectPropertyPtr prop) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); +int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon, + const char *path, + const char *property, + qemuMonitorJSONObjectPropertyPtr prop) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); + int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon, const char *type, char ***props) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 660f251..edabca8 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -720,6 +720,64 @@ cleanup: } +/* + * This test will use a path to /machine/i440fx which should exist in order + * to ensure that the qom-set property set works properly. The test will + * set a true property to true just as a proof of concept. Setting it to + * false is not a good idea... + */ +static int +testQemuMonitorJSONSetObjectProperty(const void *data) +{ + const virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNew(true, xmlopt); + int ret = -1; + qemuMonitorJSONObjectProperty prop; + + if (!test) + return -1; + + if (qemuMonitorTestAddItem(test, "qom-set", + "{ \"return\": {} }") < 0) + goto cleanup; + if (qemuMonitorTestAddItem(test, "qom-get", + "{ \"return\": true }") < 0) + goto cleanup; + + /* Let's attempt the setting */ + memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty)); + prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN; + prop.val.b = true; + if (qemuMonitorJSONSetObjectProperty(qemuMonitorTestGetMonitor(test), + "/machine/i440fx", + "realized", + &prop) < 0) + goto cleanup; + + /* To make sure it worked, fetch the property - if this succeeds then + * we didn't hose things + */ + memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty)); + prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN; + if (qemuMonitorJSONGetObjectProperty(qemuMonitorTestGetMonitor(test), + "/machine/i440fx", + "realized", + &prop) < 0) + goto cleanup; + + if (!prop.val.b) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "expected true, but false returned"); + goto cleanup; + } + + ret = 0; +cleanup: + qemuMonitorTestFree(test); + return ret; +} + + static int mymain(void) { @@ -750,6 +808,7 @@ mymain(void) DO_TEST(GetCommandLineOptionParameters); DO_TEST(GetListPaths); DO_TEST(GetObjectProperty); + DO_TEST(SetObjectProperty); virObjectUnref(xmlopt); -- 1.8.1.4

Add a period in seconds to allow/enable statistics gathering from the Balloon driver for 'virsh dommemstat <domain>'. --- docs/formatdomain.html.in | 10 ++++++++++ docs/schemas/domaincommon.rng | 7 +++++++ src/conf/domain_conf.c | 38 +++++++++++++++++++++++++++++++------- src/conf/domain_conf.h | 1 + 4 files changed, 49 insertions(+), 7 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 52a6353..93d2416 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4637,6 +4637,7 @@ qemu-kvm -net nic,model=? /dev/null <devices> <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + <stats period='10'/> </memballoon> </devices> </domain></pre> @@ -4654,6 +4655,15 @@ qemu-kvm -net nic,model=? /dev/null <li>'xen' — default with Xen</li> </ul> </dd> + <dt><code>period</code></dt> + <dd> + <p> + The optional <code>period</code> allows the QEMU virtio memory + balloon driver to provide statistics through the <code>virsh + dommemstat [domain]</code> command. + <span class='since'>Since 1.1.1, requires QEMU 1.5</span> + </p> + </dd> </dl> <h4><a name="elementsRng">Random number generator device</a></h4> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c135530..7a6852b 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2946,6 +2946,13 @@ <optional> <ref name="address"/> </optional> + <optional> + <element name="stats"> + <attribute name="period"> + <ref name="positiveInteger"/> + </attribute> + </element> + </optional> </element> </define> <define name="parallel"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 33ae4a7..816b96c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8530,32 +8530,43 @@ 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) return NULL; model = virXMLPropString(node, "model"); if (model == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_XML_ERROR, "%s", _("balloon memory must contain model name")); goto error; } + if ((def->model = virDomainMemballoonModelTypeFromString(model)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_XML_ERROR, _("unknown memory balloon model '%s'"), model); goto error; } + ctxt->node = node; + if (virXPathUInt("string(./stats/@period)", ctxt, &def->period) < -1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid statistics collection period")); + goto error; + } + if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; cleanup: VIR_FREE(model); + ctxt->node = save; return def; error: @@ -9428,7 +9439,9 @@ virDomainDeviceDefParse(const char *xmlStr, goto error; break; case VIR_DOMAIN_DEVICE_MEMBALLOON: - if (!(dev->data.memballoon = virDomainMemballoonDefParseXML(node, flags))) + if (!(dev->data.memballoon = virDomainMemballoonDefParseXML(node, + ctxt, + flags))) goto error; break; case VIR_DOMAIN_DEVICE_NVRAM: @@ -11931,7 +11944,7 @@ virDomainDefParseXML(xmlDocPtr xml, } if (n > 0) { virDomainMemballoonDefPtr memballoon = - virDomainMemballoonDefParseXML(nodes[0], flags); + virDomainMemballoonDefParseXML(nodes[0], ctxt, flags); if (!memballoon) goto error; @@ -15127,6 +15140,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf, unsigned int flags) { const char *model = virDomainMemballoonModelTypeToString(def->model); + bool noopts = true; if (!model) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -15140,11 +15154,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='%u'/>\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 4a7dcd2..ed314d9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1533,6 +1533,7 @@ enum { struct _virDomainMemballoonDef { int model; virDomainDeviceInfo info; + unsigned int period; /* seconds between collections */ }; struct _virDomainNVRAMDef { -- 1.8.1.4

At vm startup and attach attempt to set the balloon driver statistics collection period based on the value found in the domain xml file. This is not done at reconnect since it's possible that a collection period was set on the live guest and making the set period call would reset to whatever value is stored in the config file. Setting the stats collection period has a side effect of searching through the qom-list output for the virtio balloon driver and making sure that it has the right properties in order to allow setting of a collection period and eventually fetching of statistics. The walk through the qom-list is expensive and thus the balloonpath will be saved in the monitor private structure as well as a flag indicating that the initialization has already been attempted (in the event that a path is not found, no sense to keep checking). This processing model conforms to the qom object model model which 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. --- src/qemu/qemu_monitor.c | 130 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_monitor_json.c | 24 ++++++++ src/qemu/qemu_monitor_json.h | 3 + src/qemu/qemu_process.c | 14 ++++- 5 files changed, 171 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6f9a8fc..a3e250c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -83,6 +83,10 @@ struct _qemuMonitor { /* cache of query-command-line-options results */ virJSONValuePtr options; + + /* If found, path to the virtio memballoon driver */ + char *balloonpath; + bool ballooninit; }; static virClassPtr qemuMonitorClass; @@ -248,6 +252,7 @@ static void qemuMonitorDispose(void *obj) virCondDestroy(&mon->notify); VIR_FREE(mon->buffer); virJSONValueFree(mon->options); + VIR_FREE(mon->balloonpath); } @@ -925,6 +930,105 @@ qemuMonitorSetOptions(qemuMonitorPtr mon, virJSONValuePtr options) mon->options = options; } +/* Search the qom objects for the balloon driver object by it's known name + * of "virtio-balloon-pci". The entry for the driver will be found in the + * returned 'type' field using the syntax "child<virtio-balloon-pci>". + * + * Once found, check the entry to ensure it has the correct property listed. + * If it does not, then obtaining statistics from qemu will not be possible. + * This feature was added to qemu 1.5. + * + * This procedure will be call recursively until found or the qom-list is + * exhausted. + * + * Returns: + * + * 1 - Found + * 0 - Not found still looking + * -1 - Error bail out + * + * NOTE: This assumes we have already called qemuDomainObjEnterMonitor() + */ +static int +qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon, + virDomainObjPtr vm, + const char *curpath) +{ + size_t i, j, npaths = 0, nprops = 0; + int ret = 0; + char *nextpath = NULL; + qemuMonitorJSONListPathPtr *paths = NULL; + qemuMonitorJSONListPathPtr *bprops = NULL; + + /* Already set and won't change or we already search and failed to find */ + if (mon->balloonpath || mon->ballooninit) + return 1; + + /* 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; + } + + VIR_DEBUG("Searching for Balloon Object Path starting at %s", curpath); + + npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, &paths); + + for (i = 0; i < npaths && ret == 0; i++) { + + if (STREQ_NULLABLE(paths[i]->type, "link<virtio-balloon-pci>")) { + VIR_DEBUG("Path to <virtio-balloon-pci> is '%s/%s'", + curpath, paths[i]->name); + if (virAsprintf(&nextpath, "%s/%s", curpath, paths[i]->name) < 0) { + ret = -1; + goto cleanup; + } + + /* Now look at the each of the property entries to determine + * whether "guest-stats-polling-interval" exists. If not, + * then this version of qemu/kvm does not support the feature. + */ + nprops = qemuMonitorJSONGetObjectListPaths(mon, nextpath, &bprops); + for (j = 0; j < nprops; j++) { + if (STREQ(bprops[j]->name, "guest-stats-polling-interval")) { + VIR_DEBUG("Found Balloon Object Path %s", nextpath); + mon->balloonpath = nextpath; + nextpath = NULL; + ret = 1; + goto cleanup; + } + } + + /* If we get here, we found the path, but not the property */ + VIR_DEBUG("Property 'guest-stats-polling-interval' not found"); + ret = -1; + goto cleanup; + } + + /* Type entries that begin with "child<" are a branch that can be + * traversed looking for more entries + */ + if (paths[i]->type && STRPREFIX(paths[i]->type, "child<")) { + if (virAsprintf(&nextpath, "%s/%s", curpath, paths[i]->name) < 0) { + ret = -1; + goto cleanup; + } + ret = qemuMonitorFindBalloonObjectPath(mon, vm, nextpath); + } + } + +cleanup: + for (i = 0; i < npaths; i++) + qemuMonitorJSONListPathFree(paths[i]); + VIR_FREE(paths); + for (j = 0; j < nprops; j++) + qemuMonitorJSONListPathFree(bprops[j]); + VIR_FREE(bprops); + VIR_FREE(nextpath); + return ret; +} + int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon, const char *cmd, int scm_fd, @@ -1386,6 +1490,32 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, return ret; } +int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon, + int period) +{ + int ret = -1; + VIR_DEBUG("mon=%p period=%d", mon, period); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + if (qemuMonitorFindBalloonObjectPath(mon, mon->vm, "/") == 1) { + ret = qemuMonitorJSONSetMemoryStatsPeriod(mon, mon->balloonpath, + period); + } + mon->ballooninit = true; + return ret; +} + int qemuMonitorBlockIOStatusToError(const char *status) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 78011ee..12b730a 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -265,6 +265,8 @@ int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon, int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); +int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon, + int period); int qemuMonitorBlockIOStatusToError(const char *status); virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 92d458c..a9e8723 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1488,6 +1488,30 @@ cleanup: } +/* + * Using the provided balloonpath, determine if we need to set the + * collection interval property to enable statistics gathering. + */ +int +qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon, + char *balloonpath, + int period) +{ + qemuMonitorJSONObjectProperty prop; + + /* Set to the value in memballoon (could enable or disable) */ + memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty)); + prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT; + prop.val.iv = period; + if (qemuMonitorJSONSetObjectProperty(mon, balloonpath, + "guest-stats-polling-interval", + &prop) < 0) { + return -1; + } + return 0; +} + + int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, virHashTablePtr table) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index a857d86..e2324f1 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -61,6 +61,9 @@ int qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon, int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); +int qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon, + char *balloonpath, + int period); int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, virHashTablePtr table); int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 574abf2..239c65f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3883,6 +3883,9 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; } qemuDomainObjEnterMonitor(driver, vm); + if (vm->def->memballoon) + qemuMonitorSetMemoryStatsPeriod(priv->mon, + vm->def->memballoon->period); if (qemuMonitorSetBalloon(priv->mon, cur_balloon) < 0) { qemuDomainObjExitMonitor(driver, vm); goto cleanup; @@ -4409,11 +4412,18 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, if (!virDomainObjIsActive(vm)) goto cleanup; - if (running) + if (running) { virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNPAUSED); - else + if (vm->def->memballoon) { + qemuDomainObjEnterMonitor(driver, vm); + qemuMonitorSetMemoryStatsPeriod(priv->mon, + vm->def->memballoon->period); + qemuDomainObjExitMonitor(driver, vm); + } + } else { virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, reason); + } VIR_DEBUG("Writing domain status to disk"); if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) -- 1.8.1.4

This patch will add the qemuMonitorJSONGetMemoryStats() to execute a "guest-stats" on the balloonpath using "get-qom" replacing the former mechanism which looked through the "query-ballon" returned data for the fields. The "query-balloon" code only returns 'actual' memory. Rather than duplicating the existing code, have the JSON API use the GetBalloonInfo API. A check in the qemuMonitorGetMemoryStats() will be made to ensure the balloon driver path has been set. Since the underlying JSON code can return data not associated with the balloon driver, we don't fail on a failure to get the balloonpath. Of course since we've made the check, we can then set the ballooninit flag. Getting the path here is primarily due to the process reconnect path which doesn't attempt to set the collection period. --- src/qemu/qemu_monitor.c | 10 ++- src/qemu/qemu_monitor_json.c | 190 ++++++++++++++++++++----------------------- src/qemu/qemu_monitor_json.h | 1 + 3 files changed, 95 insertions(+), 106 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a3e250c..14b80e3 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1483,10 +1483,14 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, return -1; } - if (mon->json) - ret = qemuMonitorJSONGetMemoryStats(mon, stats, nr_stats); - else + if (mon->json) { + ignore_value(qemuMonitorFindBalloonObjectPath(mon, mon->vm, "/")); + mon->ballooninit = true; + ret = qemuMonitorJSONGetMemoryStats(mon, mon->balloonpath, + stats, nr_stats); + } else { ret = qemuMonitorTextGetMemoryStats(mon, stats, nr_stats); + } return ret; } diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index a9e8723..45edbd3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1361,131 +1361,115 @@ 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) && \ + (got < nr_stats)) { \ + 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) { \ + stats[got].tag = TAG; \ + stats[got].val = mem / DIVISOR; \ + got++; \ + } \ + } \ + } + + int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon, + char *balloonpath, virDomainMemoryStatPtr stats, unsigned int nr_stats) { int ret; - int got = 0; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-balloon", - NULL); + virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; + virJSONValuePtr data; + virJSONValuePtr statsdata; + unsigned long long mem; + int got = 0; - if (!cmd) - return -1; + ret = qemuMonitorJSONGetBalloonInfo(mon, &mem); + if (ret == 1 && (got < nr_stats)) { + stats[got].tag = VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON; + stats[got].val = mem; + got++; + } - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (!balloonpath) + goto cleanup; - if (ret == 0) { - /* See if balloon soft-failed */ - if (qemuMonitorJSONHasError(reply, "DeviceNotActive") || - qemuMonitorJSONHasError(reply, "KVMMissingCap")) - goto cleanup; + if (!(cmd = qemuMonitorJSONMakeCommand("qom-get", + "s:path", balloonpath, + "s:property", "guest-stats", + NULL))) + goto cleanup; - /* See if any other fatal error occurred */ - ret = qemuMonitorJSONCheckError(cmd, reply); + ret = qemuMonitorJSONCommand(mon, cmd, &reply); - /* Success */ - if (ret == 0) { - virJSONValuePtr data; - unsigned long long mem; + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); - if (!(data = virJSONValueObjectGet(reply, "return"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("info balloon reply was missing return data")); - ret = -1; - goto cleanup; - } + if (ret < 0) + goto cleanup; - if (virJSONValueObjectHasKey(data, "actual") && (got < nr_stats)) { - if (virJSONValueObjectGetNumberUlong(data, "actual", &mem) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("info balloon reply was missing balloon actual")); - ret = -1; - goto cleanup; - } - stats[got].tag = VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON; - stats[got].val = (mem/1024); - got++; - } + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qom-get reply was missing return data")); + goto cleanup; + } - if (virJSONValueObjectHasKey(data, "mem_swapped_in") && (got < nr_stats)) { - if (virJSONValueObjectGetNumberUlong(data, "mem_swapped_in", &mem) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("info balloon reply was missing balloon mem_swapped_in")); - ret = -1; - goto cleanup; - } - stats[got].tag = VIR_DOMAIN_MEMORY_STAT_SWAP_IN; - stats[got].val = (mem/1024); - got++; - } - if (virJSONValueObjectHasKey(data, "mem_swapped_out") && (got < nr_stats)) { - if (virJSONValueObjectGetNumberUlong(data, "mem_swapped_out", &mem) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("info balloon reply was missing balloon mem_swapped_out")); - ret = -1; - goto cleanup; - } - stats[got].tag = VIR_DOMAIN_MEMORY_STAT_SWAP_OUT; - stats[got].val = (mem/1024); - got++; - } - if (virJSONValueObjectHasKey(data, "major_page_faults") && (got < nr_stats)) { - if (virJSONValueObjectGetNumberUlong(data, "major_page_faults", &mem) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("info balloon reply was missing balloon major_page_faults")); - ret = -1; - goto cleanup; - } - stats[got].tag = VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT; - stats[got].val = mem; - got++; - } - if (virJSONValueObjectHasKey(data, "minor_page_faults") && (got < nr_stats)) { - if (virJSONValueObjectGetNumberUlong(data, "minor_page_faults", &mem) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("info balloon reply was missing balloon minor_page_faults")); - ret = -1; - goto cleanup; - } - stats[got].tag = VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT; - stats[got].val = mem; - got++; - } - if (virJSONValueObjectHasKey(data, "free_mem") && (got < nr_stats)) { - if (virJSONValueObjectGetNumberUlong(data, "free_mem", &mem) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("info balloon reply was missing balloon free_mem")); - ret = -1; - goto cleanup; - } - stats[got].tag = VIR_DOMAIN_MEMORY_STAT_UNUSED; - stats[got].val = (mem/1024); - got++; - } - if (virJSONValueObjectHasKey(data, "total_mem") && (got < nr_stats)) { - if (virJSONValueObjectGetNumberUlong(data, "total_mem", &mem) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("info balloon reply was missing balloon total_mem")); - ret = -1; - goto cleanup; - } - stats[got].tag = VIR_DOMAIN_MEMORY_STAT_AVAILABLE; - stats[got].val = (mem/1024); - got++; - } - } + if (!(statsdata = virJSONValueObjectGet(data, "stats"))) { + VIR_DEBUG("data does not include 'stats'"); + goto cleanup; } - if (got > 0) - ret = got; + 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); + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); + + if (got > 0) + ret = got; + return ret; } +#undef GET_BALLOON_STATS /* diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index e2324f1..c97cd66 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -59,6 +59,7 @@ int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon, int qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon, unsigned long long *currmem); int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon, + char *balloonpath, virDomainMemoryStatPtr stats, unsigned int nr_stats); int qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon, -- 1.8.1.4