On Tue, Jul 02, 2013 at 09:39:23AM -0400, 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_domain.c | 1 + > src/qemu/qemu_domain.h | 2 + > src/qemu/qemu_process.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 165 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 8d79066..5aaf1e1 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -244,6 +244,7 @@ qemuDomainObjPrivateFree(void *data) > VIR_FREE(priv->vcpupids); > VIR_FREE(priv->lockState); > VIR_FREE(priv->origname); > + VIR_FREE(priv->balloonpath); > > virChrdevFree(priv->devs); > > diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h > index 068a4c3..005fd0f 100644 > --- a/src/qemu/qemu_domain.h > +++ b/src/qemu/qemu_domain.h > @@ -161,6 +161,8 @@ struct _qemuDomainObjPrivate { > char *origname; > int nbdPort; /* Port used for migration with NBD */ > > + char *balloonpath; > + > virChrdevsPtr devs; I think I'd prefer this to be kept as a private property in the struct qemuMonitor. > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index ac5ffcf..9a2add1 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -1564,6 +1564,150 @@ qemuProcessLookupPTYs(virDomainChrDefPtr *devices, > return 0; > } > > +/* 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 > +qemuProcessFindBalloonObjectPath(qemuMonitorPtr mon, > + virDomainObjPtr vm, > + const char *curpath, > + char **balloonpath) > +{ > + int i,j; > + int npaths = 0; > + int nprops = 0; > + int ret = 0; > + char *nextpath = NULL; > + qemuMonitorListPathPtr *paths = NULL; > + qemuMonitorListPathPtr *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 (*balloonpath) > + return 1; > + > + VIR_DEBUG("Searching for Balloon Object Path starting at %s", curpath); > + > + npaths = qemuMonitorGetObjectListPaths(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 = qemuMonitorGetObjectListPaths(mon, nextpath, &bprops); > + for (j = 0; j < nprops; j++) { > + if (STREQ(bprops[j]->name, "guest-stats-polling-interval")) { > + *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 = qemuProcessFindBalloonObjectPath(mon, vm, nextpath, > + balloonpath); > + } > + } > + > +cleanup: > + for (i = 0; i < npaths; i++) > + qemuMonitorListPathFree(paths[i]); > + VIR_FREE(paths); > + for (j = 0; j < nprops; j++) > + qemuMonitorListPathFree(bprops[j]); > + VIR_FREE(bprops); > + VIR_FREE(nextpath); > + return ret; > +} I think this method should be kept in qemu_monitor.c and made static to that file We can then populate it on first use. > +/* > + * Using the provided balloonpath, determine if we need to set the > + * collection interval property to enable statistics gathering. > + * > + * NOTE: This assumes we have already called qemuDomainObjEnterMonitor() > + */ > +static void > +qemuProcessUpdateBalloonStatsPeriod(qemuMonitorPtr mon, > + char *balloonpath, > + virDomainObjPtr vm) > +{ > + qemuMonitorObjectProperty prop; > + > + /* Get the current value of the stats polling interval */ > + memset(&prop, 0, sizeof(qemuMonitorObjectProperty)); > + prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT; > + if (qemuMonitorGetObjectProperty(mon, balloonpath, > + "guest-stats-polling-interval", > + &prop) < 0) { > + VIR_DEBUG("Failed to get polling interval for balloon driver"); > + return; > + } > + > + VIR_DEBUG("memballoon period=%d 'guest-stats-polling-interval'=%d", > + vm->def->memballoon->period, prop.val.i); > + > + /* Same value - no need to set */ > + if (vm->def->memballoon->period == prop.val.i) > + return; > + > + /* Set to the value in memballoon (could enable or disable) */ > + memset(&prop, 0, sizeof(qemuMonitorObjectProperty)); > + prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT; > + prop.val.i = vm->def->memballoon->period; > + if (qemuMonitorSetObjectProperty(mon, balloonpath, > + "guest-stats-polling-interval", > + &prop) < 0) > + VIR_DEBUG("Failed to set polling interval for balloon driver"); > + > +} Since I recommended that we don't expose qemuMonitorGetObjectProperty or qemuMonitorSetObjectProperty in qemu_monitor.h, what you'll need todo here is move this code into qemu_monitor.c defining a method something like int qemuMonitorSetBalloonStatsPeriod(qemuMonitorPtr mon, int period); This method can populate the balloonpath field in qemuMOnitorPtr struct on first use. 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list