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

Re: [libvirt] [PATCH 09/12] [v2] virNodeGetMemoryStats: Implement public API



On Tue, Jun 07, 2011 at 10:04:54AM +0900, Minoru Usui wrote:
> virNodeGetMemoryStats: Implement public API
> 
> Signed-off-by: Minoru Usui <usui mxm nes nec co jp>
> ---
>  include/libvirt/libvirt.h.in |    2 +-
>  src/libvirt.c                |   87 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 88 insertions(+), 1 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 885db25..b7772ba 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -824,7 +824,7 @@ int                     virNodeGetCPUStats (virConnectPtr conn,
>  
>  int                     virNodeGetMemoryStats (virConnectPtr conn,
>                                                 int cellNum,
> -                                               virCPUStatsPtr params,
> +                                               virMemoryStatsPtr params,
>                                                 int *nparams,
>                                                 unsigned int flags);


Opps, I think this chunk should be in your first patch.

>  
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 45d9be7..976eaa0 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -5429,6 +5429,93 @@ error:
>  }
>  
>  /**
> + * virNodeGetMemoryStats:
> + * @conn: pointer to the hypervisor connection.
> + * @cellNum: number of node cell. (VIR_MEMORY_STATS_ALL_CELLS means total cell
> + *           statistics)
> + * @params: pointer to node memory stats objects
> + * @nparams: number of node memory stats (this value should be same or
> + *          less than the number of stats supported)
> + * @flags: currently unused, for future extension. always pass 0.
> + *
> + * This function provides memory stats of the node.
> + * If you want to get total cpu statistics of the node, you must specify
> + * VIR_MEMORY_STATS_ALL_CELLS to @cellNum.
> + * The @params array will be filled with the values equal to the number of
> + * stats suggested by @nparams
> + *
> + * As the value of @nparams is dynamic, call the API setting @nparams to 0 and
> + * @params as NULL, the API returns the number of parameters supported by the
> + * HV by updating @nparams on SUCCESS. The caller should then allocate @params
> + * array, i.e. (sizeof(@virMemoryStats) * @nparams) bytes and call
> + * the API again.
> + *
> + * Here is the sample code snippet:
> + *
> + * if ((virNodeGetMemoryStats(conn, cellNum, NULL, &nparams, 0) == 0) &&
> + *     (nparams != 0)) {
> + *     params = vshMalloc(ctl, sizeof(virMemoryStats) * nparams);
> + *     memset(params, cellNum, 0, sizeof(virMemoryStats) * nparams);
> + *     if (virNodeGetMemoryStats(conn, params, &nparams, 0)) {
> + *         vshError(ctl, "%s", _("Unable to get node memory stats"));
> + *         goto error;
> + *     }
> + * }


Same comment as the earlier patch - use malloc() and fprintf(stderr)
here.

> + *
> + * This function doesn't requires privileged access to the hypervisor.
> + * This function expects the caller to allocate the @params.
> + *
> + * Memory Stats:
> + *
> + * VIR_MEMORY_STATS_TOTAL:
> + *     The total memory usage.(KB)
> + * VIR_MEMORY_STATS_FREE:
> + *     The free memory usage.(KB)
> + *     On linux, this usage includes buffers and cached.
> + * VIR_MEMORY_STATS_BUFFERS:
> + *     The buffers memory usage.(KB)
> + * VIR_MEMORY_STATS_CACHED:
> + *     The cached memory usage.(KB)
> + *
> + * Returns -1 in case of error, 0 in case of success.
> + */
> +int virNodeGetMemoryStats (virConnectPtr conn,
> +                           int cellNum,
> +                           virMemoryStatsPtr params,
> +                           int *nparams, unsigned int flags)
> +{
> +    VIR_DEBUG("conn=%p, cellNum=%d, params=%p, nparams=%d, flags=%u",
> +              conn, cellNum, params, nparams ? *nparams : -1, flags);
> +
> +    virResetLastError();
> +
> +    if (!VIR_IS_CONNECT(conn)) {
> +        virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
> +        virDispatchError(NULL);
> +        return -1;
> +    }
> +
> +    if ((nparams == NULL) || (*nparams < 0) ||
> +        ((cellNum < 0) && (cellNum != VIR_MEMORY_STATS_ALL_CELLS))) {
> +        virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> +        goto error;
> +    }
> +
> +    if (conn->driver->nodeGetMemoryStats) {
> +        int ret;
> +        ret = conn->driver->nodeGetMemoryStats (conn, cellNum, params, nparams, flags);
> +        if (ret < 0)
> +            goto error;
> +        return ret;
> +    }
> +    virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> +error:
> +    virDispatchError(conn);
> +    return -1;
> +}
> +
> +/**
>   * virNodeGetFreeMemory:
>   * @conn: pointer to the hypervisor connection
>   *

ACK, if those 2 points above are addressed

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]