Re: [libvirt] [PATCH 5/8] Determine whether to start balloon memory stats gathering.

3 Jul 2013

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 :|

    

Re: [libvirt] [PATCH 5/8] Determine whether to start balloon memory stats gathering.

Daniel P. Berrange