Re: [libvirt] [PATCH v2 05/10] 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_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;

+ 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;

+ } + } + + /* 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

+ * 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;

+ } + + 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");

+ 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); + } +

/* 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);

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)