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

Re: [libvirt] [PATCHv2 3/6] virNodeGetCPUTime: Implement public API



On Fri, Apr 08, 2011 at 08:35:12PM +0900, Minoru Usui wrote:
> virNodeGetCPUTime: Implement public API
> 
> Signed-off-by: Minoru Usui <usui mxm nes nec co jp>
> ---
>  src/libvirt.c |   70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 70 insertions(+), 0 deletions(-)
> 
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 8be18d4..a65bfc6 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -4260,6 +4260,76 @@ error:
>  }
>  
>  /**
> + * virNodeGetCpuTime:
> + * @stats: nr_stats-sized array of stat structures (returned)
> + * @nr_stats: number of cpu time statistics requested of the node.
> + * @flags: unused, always pass 0
> + *
> + * This function provides cpu time statistics of the node.
> + *
> + * Up to 'nr_stats' elements of 'stats' will be populated with cpu time statistics
> + * of the node.  Only statistics supported by the driver, and this version of
> + * libvirt will be returned.
> + *
> + * Cpu time Statistics:
> + *
> + * VIR_NODE_CPU_TIME_KERNEL:
> + *     The cumulative CPU time which spends by kernel,
> + *     when the node booting up.(nanoseconds)
> + * VIR_NODE_CPU_TIME_USER:
> + *     The cumulative CPU time which spends by user processes,
> + *     when the node booting up.(nanoseconds)
> + * VIR_NODE_CPU_TIME_IDLE:
> + *     The cumulative idle CPU time, when the node booting up.(nanoseconds)
> + * VIR_NODE_CPU_TIME_IOWAIT:
> + *     The cumulative I/O wait CPU time, when the node booting up.(nanoseconds)
> + * VIR_NODE_CPU_TIME_UTILIZATION:
> + *     The CPU utilization. The usage value is in percent and 100%
> + *     represents all CPUs on the server.
> + *
> + * Returns: The number of stats provided or -1 in case of failure.
> + */
> +int virNodeGetCpuTime (virConnectPtr conn, virNodeCpuTimePtr stats,
> +                          unsigned int nr_stats, unsigned int flags)
> +{
> +    unsigned long nr_stats_ret = 0;

This ought to have been an 'int' to match the function return
type, and better to declare it later in the block where it was
first used.

> +
> +    VIR_DEBUG("conn=%p, stats=%p, nr_stats=%u", conn, stats, nr_stats);
> +
> +    virResetLastError();
> +
> +    if (!VIR_IS_CONNECT (conn)) {
> +        virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
> +        virDispatchError(NULL);
> +        return 0;
> +    }
> +
> +    if (flags != 0) {
> +        virLibConnError(VIR_ERR_INVALID_ARG, _("flags must be zero"));
> +        goto error;
> +    }

Instead of this, use the standard macro we have:

   virCheckFlags(flags, 0);

> +    if (!stats || nr_stats == 0)
> +        return 0;
> +
> +    if (nr_stats > VIR_NODE_CPU_TIME_NR)
> +        nr_stats = VIR_NODE_CPU_TIME_NR;

If we're following the pattern of virDomainGetMemoryParameters/BlkioParameters
then we should have passed in a pointer to nr_stats. It would be allowed to
be zero, in which case the hypervisor would initialize it with the required
size for the number of parameters.

eg, the signature would be

int
virNodeGetCpuParameters(virConnectPtr conn,
                        virNodeCpuParameterPtr params,
                        int *nparams, unsigned int flags)

and then check

    if ((nparams == NULL) || (*nparams < 0)) {
        virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
        goto error;
    }



> +
> +    if (conn->driver->nodeGetCpuTime) {
> +        nr_stats_ret = conn->driver->nodeGetCpuTime (conn, stats, nr_stats, flags);
> +        if (nr_stats_ret == -1)
> +            goto error;
> +        return nr_stats_ret;
> +    }
> +
> +    virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> +error:
> +    virDispatchError(conn);
> +    return -1;
> +}
> +
> +/**


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]