[libvirt] [PATCH v2 05/10] Determine whether to start balloon memory stats gathering.
Michal Privoznik
mprivozn at redhat.com
Thu Jul 11 14:05:48 UTC 2013
On 08.07.2013 21:20, John Ferlan wrote:
> At vm startup, reconnect, and attach - check for the presence of the balloon
> driver and save the path in the private area of the driver. This path will
> remain constant throughout the life of the domain and can then be used rather
> than attempting to find the path each time balloon driver statistics are
> fetched or the collection period changes. The qom object model model requires
> setting object properties after device startup. That is, it's not possible
> to pass the period along via the startup code as it won't be recognized.
> If a balloon driver path is found a check of the existing collection period
> will be made against the saved domain value in order to determine if an
> adjustment needs to be made to the period to start or stop collecting stats
> ---
> src/qemu/qemu_monitor.c | 130 +++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor.h | 2 +
> src/qemu/qemu_monitor_json.c | 42 ++++++++++++++
> src/qemu/qemu_monitor_json.h | 3 +
> src/qemu/qemu_process.c | 21 ++++++-
> 5 files changed, 196 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index fe5ab0a..038c9e8 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -83,6 +83,9 @@ struct _qemuMonitor {
>
> /* cache of query-command-line-options results */
> virJSONValuePtr options;
> +
> + /* If found, path to the virtio memballoon driver */
> + char *balloonpath;
> };
>
> static virClassPtr qemuMonitorClass;
> @@ -248,6 +251,7 @@ static void qemuMonitorDispose(void *obj)
> virCondDestroy(&mon->notify);
> VIR_FREE(mon->buffer);
> virJSONValueFree(mon->options);
> + VIR_FREE(mon->balloonpath);
> }
>
>
> @@ -929,6 +933,107 @@ qemuMonitorSetOptions(qemuMonitorPtr mon, virJSONValuePtr options)
> mon->options = options;
> }
>
> +/* Search the qom objects for the balloon driver object by it's known name
> + * of "virtio-balloon-pci". The entry for the driver will be found in the
> + * returned 'type' field using the syntax "child<virtio-balloon-pci>".
> + *
> + * Once found, check the entry to ensure it has the correct property listed.
> + * If it does not, then obtaining statistics from qemu will not be possible.
> + * This feature was added to qemu 1.5.
> + *
> + * This procedure will be call recursively until found or the qom-list is
> + * exhausted.
> + *
> + * Returns:
> + *
> + * 1 - Found
> + * 0 - Not found still looking
> + * -1 - Error bail out
> + *
> + * NOTE: This assumes we have already called qemuDomainObjEnterMonitor()
> + */
> +static int
> +qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
> + virDomainObjPtr vm,
> + const char *curpath)
> +{
> + int i,j;
size_t i,j, npaths = 0, nprops = 0;
> + int npaths = 0;
> + int nprops = 0;
> + int ret = 0;
> + char *nextpath = NULL;
> + qemuMonitorJSONListPathPtr *paths = NULL;
> + qemuMonitorJSONListPathPtr *bprops = NULL;
> +
> + /* Not supported */
> + if (vm->def->memballoon &&
> + vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
> + VIR_DEBUG("Model must be virtio to get memballoon path");
> + return -1;
> + }
> +
> + /* Already set and won't change */
> + if (mon->balloonpath)
> + return 1;
> +
> + VIR_DEBUG("Searching for Balloon Object Path starting at %s", curpath);
> +
> + npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, &paths);
> +
> + for (i = 0; i < npaths && ret == 0; i++) {
> +
> + if (STREQ_NULLABLE(paths[i]->type, "link<virtio-balloon-pci>")) {
> + VIR_DEBUG("Path to <virtio-balloon-pci> is '%s/%s'",
> + curpath, paths[i]->name);
> + if (virAsprintf(&nextpath, "%s/%s", curpath, paths[i]->name) < 0) {
> + ret = -1;
> + goto cleanup;
> + }
> +
> + /* Now look at the each of the property entries to determine
> + * whether "guest-stats-polling-interval" exists. If not,
> + * then this version of qemu/kvm does not support the feature.
> + */
> + nprops = qemuMonitorJSONGetObjectListPaths(mon, nextpath, &bprops);
> + for (j = 0; j < nprops; j++) {
> + if (STREQ(bprops[j]->name, "guest-stats-polling-interval")) {
> + mon->balloonpath = nextpath;
> + nextpath = NULL;
> + ret = 1;
> + goto cleanup;
Maybe I'd add VIR_DEBUG() here to log the path.
> + }
> + }
> +
> + /* If we get here, we found the path, but not the property */
> + VIR_DEBUG("Property 'guest-stats-polling-interval' not found");
> + ret = -1;
> + goto cleanup;
> + }
> +
> + /* Type entries that begin with "child<" are a branch that can be
> + * traversed looking for more entries
> + */
> + if (paths[i]->type && STRPREFIX(paths[i]->type, "child<")) {
> + if (virAsprintf(&nextpath, "%s/%s", curpath, paths[i]->name) < 0) {
> + virReportOOMError();
> + ret = -1;
> + goto cleanup;
> + }
> + ret = qemuMonitorFindBalloonObjectPath(mon, vm, nextpath);
> + }
> + }
> +
> +cleanup:
> + for (i = 0; i < npaths; i++)
> + qemuMonitorJSONListPathFree(paths[i]);
> + VIR_FREE(paths);
> + for (j = 0; j < nprops; j++)
> + qemuMonitorJSONListPathFree(bprops[j]);
> + VIR_FREE(bprops);
> + VIR_FREE(nextpath);
> + return ret;
> +}
> +
> int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon,
> const char *cmd,
> int scm_fd,
> @@ -1390,6 +1495,31 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
> return ret;
> }
>
> +int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
> + int period)
> +{
> + int ret = -1;
> + VIR_DEBUG("mon=%p period=%d", mon, period);
> +
> + if (!mon) {
> + virReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("monitor must not be NULL"));
> + return -1;
> + }
> +
> + if (!mon->json) {
> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> + _("JSON monitor is required"));
> + return -1;
> + }
> +
> + if (qemuMonitorFindBalloonObjectPath(mon, mon->vm, "/") == 1) {
> + ret = qemuMonitorJSONSetMemoryStatsPeriod(mon, mon->balloonpath,
> + period);
> + }
> + return ret;
> +}
> +
> int
> qemuMonitorBlockIOStatusToError(const char *status)
> {
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 78011ee..12b730a 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -265,6 +265,8 @@ int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon,
> int qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
> virDomainMemoryStatPtr stats,
> unsigned int nr_stats);
> +int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
> + int period);
>
> int qemuMonitorBlockIOStatusToError(const char *status);
> virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon);
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 9487955..2d7f9b6 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -1495,6 +1495,48 @@ cleanup:
> }
>
>
> +/*
> + * Using the provided balloonpath, determine if we need to set the
s/balloonpath/balloon path/
> + * collection interval property to enable statistics gathering.
> + */
> +int
> +qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon,
> + char *balloonpath,
> + int period)
> +{
> + qemuMonitorJSONObjectProperty prop;
> +
> + /* Get the current value of the stats polling interval */
> + memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
> + prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
> + if (qemuMonitorJSONGetObjectProperty(mon, balloonpath,
> + "guest-stats-polling-interval",
> + &prop) < 0) {
> + VIR_DEBUG("Failed to get polling interval for balloon driver");
> + return 0;
Shouldn't we return -1 here? I mean, if caller requires us to set an
interval, we should error out if we fail. Why are we querying for the
interval prior setting it anyway?
> + }
> +
> + VIR_DEBUG("memballoon period=%d 'guest-stats-polling-interval'=%d",
> + period, prop.val.i);
> +
> + /* Same value - no need to set */
> + if (period == prop.val.i)
> + return 0;
> +
> + /* Set to the value in memballoon (could enable or disable) */
> + memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
> + prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
> + prop.val.i = period;
> + if (qemuMonitorJSONSetObjectProperty(mon, balloonpath,
> + "guest-stats-polling-interval",
> + &prop) < 0) {
> + VIR_DEBUG("Failed to set polling interval for balloon driver");
This is unnecessary as long as qemuMonitorJSONSetObjectProperty reports
error on retval < 0, which it does.
> + return -1;
> + }
> + return 0;
> +}
> +
> +
> int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
> virHashTablePtr table)
> {
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index fd0dedd..b0068ff 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -61,6 +61,9 @@ int qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon,
> int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
> virDomainMemoryStatPtr stats,
> unsigned int nr_stats);
> +int qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon,
> + char *balloonpath,
> + int period);
> int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
> virHashTablePtr table);
> int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index f8c652f..fd3b7a8 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3091,6 +3091,13 @@ qemuProcessReconnect(void *opaque)
> if (qemuProcessRecoverJob(driver, obj, conn, &oldjob) < 0)
> goto error;
>
> + if (obj->def->memballoon) {
> + qemuDomainObjEnterMonitor(driver, obj);
> + qemuMonitorSetMemoryStatsPeriod(priv->mon,
> + obj->def->memballoon->period);
> + qemuDomainObjExitMonitor(driver, obj);
> + }
> +
Why do we want to enable this on ProcessReconnect?
> /* update domain state XML with possibly updated state in virDomainObj */
> if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, obj) < 0)
> goto error;
> @@ -3910,6 +3917,9 @@ int qemuProcessStart(virConnectPtr conn,
> goto cleanup;
> }
> qemuDomainObjEnterMonitor(driver, vm);
> + if (vm->def->memballoon)
> + qemuMonitorSetMemoryStatsPeriod(priv->mon,
> + vm->def->memballoon->period);
It makes sense here.
> if (qemuMonitorSetBalloon(priv->mon, cur_balloon) < 0) {
> qemuDomainObjExitMonitor(driver, vm);
> goto cleanup;
> @@ -4439,11 +4449,18 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
> if (!virDomainObjIsActive(vm))
> goto cleanup;
>
> - if (running)
> + if (running) {
> virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
> VIR_DOMAIN_RUNNING_UNPAUSED);
> - else
> + if (vm->def->memballoon) {
> + qemuDomainObjEnterMonitor(driver, vm);
> + qemuMonitorSetMemoryStatsPeriod(priv->mon,
> + vm->def->memballoon->period);
> + qemuDomainObjExitMonitor(driver, vm);
> + }
> + } else {
> virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, reason);
> + }
>
> VIR_DEBUG("Writing domain status to disk");
> if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
>
Michal
More information about the libvir-list
mailing list