[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]