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

Re: [libvirt] [PATCHv2 1/2] Add a new flag VIR_DOMAIN_CPU_STATS_VCPU to virDomainGetCPUStats



On 04/24/2012 03:20 AM, Hu Tao wrote:
> Currently virDomainGetCPUStats gets total cpu usage, which consists
> of:
> 
>   1. vcpu usage: the physical cpu time consumed by virtual cpu(s) of
>      domain
>   2. hypervisor: `total cpu usage' - `vcpu usage'
> 
> The flag VIR_DOMAIN_CPU_STATS_VCPU is for getting vcpu usages.
> ---
>  include/libvirt/libvirt.h.in |    8 ++++++++
>  src/libvirt.c                |    9 +++++++--
>  tools/virsh.c                |   16 +++++++++++-----
>  3 files changed, 26 insertions(+), 7 deletions(-)

Question: is it better to make the user call virDomainGetCPUStats twice
(once for total with flags==0, once for vcpu with flags=VCPU), where
there is a race between the two calls?  Or should we instead return both
counts in the same call, by having two separate virTypedParameter stat
names and no flag?

In other words, I'm thinking it might be better to keep "cpu_time"
(VIR_DOMAIN_CPU_STATS_CPUTIME) as the first stat, and add a second stat
"vcpu_time" (VIR_DOMAIN_CPU_STATS_VCPUTIME) as a second stat, all within
the same call, with no need to add a flag.

> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index ac5df95..2ce8876 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1355,6 +1355,14 @@ int                     virDomainGetState       (virDomainPtr domain,
>   */
>  #define VIR_DOMAIN_CPU_STATS_SYSTEMTIME "system_time"

So that would be a new #define here...

>  
> +typedef enum {
> +    /* virTypedParameterFlags goes here.  */
> +
> +    /* Additionally, these flags may be bitwise-OR'd in. These
> +       flags should not override those of virTypedParameterFlags */
> +    VIR_DOMAIN_CPU_STATS_VCPU = 1 << 3, /* get vcpu stats */
> +} virDomainGetCPUStatsFlags;

drop this enum...

> @@ -18704,7 +18704,12 @@ error:
>   * host perspective, this would typically match the cpus member
>   * of virNodeGetInfo(), but might be less due to host cpu hotplug.
>   *
> - * For now, @flags is unused, and the statistics all relate to the
> + * If no flags specified, then statistics of total physical cpu time
> + * consumed by a domain are returned. If flag VIR_DOMAIN_CPU_STATS_VCPU
> + * is set, then statistics of physical cpu time consumed by virtual cpus
> + * of domain are returned. The difference is:
> + *   total cpu usage = vcpu usage + hypervisor usage
> + * For now, the statistics all relate to the

and tweak this documentation.

> +++ b/tools/virsh.c
> @@ -5562,6 +5562,7 @@ static const vshCmdOptDef opts_cpu_stats[] = {
>      {"total", VSH_OT_BOOL, 0, N_("Show total statistics only")},
>      {"start", VSH_OT_INT, 0, N_("Show statistics from this CPU")},
>      {"count", VSH_OT_INT, 0, N_("Number of shown CPUs at most")},
> +    {"vcpu", VSH_OT_BOOL, 0, N_("Show vcpu statistics")},
>      {NULL, 0, 0, NULL},
>  };

virsh would then automatically output both stats in one pass, and
wouldn't need a --vcpu flag.

The qemu_driver implementation would be mostly the same, but instead of
basing things off a flag, it would always grab both stats (total and
vcpu) at once.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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