On Thu, Jul 11, 2013 at 07:55:55PM -0400, John Ferlan wrote:
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;
This isn't correct logic. You need
if (mon->balloonpath) {
return 1;
} else if (mon->ballooninit) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s"
_("Cannot determine balloon device path"));
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");
You need to use virReportError here, so the caller sees an error
messages.
+ 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;
And virReportERror here too
+ 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)
Should that be vm->def->memballoon &&
vm->def->memballoon->period.
eg we don't want to call this if no balloon stats period was set
in the XML.
+ 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) {
Same here.
+ 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)
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 :|