[libvirt] [PATCH v3 05/10] Determine whether to start balloon memory stats gathering.
Daniel P. Berrange
berrange at redhat.com
Fri Jul 12 12:39:34 UTC 2013
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 :|
More information about the libvir-list
mailing list