Re: [libvirt] [PATCHv6 0/6] Add virNodeGetCPUStats() API

["Daniel P. Berrange" <berrange redhat com>]

On Wed, May 25, 2011 at 03:01:29PM +0900, Minoru Usui wrote:
> Hi,
> 
> This is v6 of virNodeGetCPUTimeParameters() API. 
> (This time, I rename it to virNodeGetCPUStats())
> It returns cpu utilization or 
> cumulative cpu time of the node from /proc/stat since node boots up.
> This patch only supports linux host.

The code in this patch series looks fine to me now. I would
ACK the whole series, but I have one design question I should
have asked previously.

This API returns CPU information for the host as a whole.
Historically, when adding an API like this, someone has always
then asked for a per-CPU breakdown of the same data.

So I wonder if we should add a parameter for the CPU num
to the API:

 int                     virNodeGetCPUStats (virConnectPtr conn,
                                             int cpuNum,
                                             virCPUStatsPtr params,
                                             int *nparams,
                                             unsigned int flags);

And have a constant for getting the host data as a whole:

   #define  VIR_NODE_CPU_STATS_ALL_CPUS -1

so if you want to get individual data for each CPU you
can do

   virNodeGetCPUStats(conn, 0, params, nparams, flags);
   virNodeGetCPUStats(conn, 1, params, nparams, flags);
   virNodeGetCPUStats(conn, 2, params, nparams, flags);
   virNodeGetCPUStats(conn, 3, params, nparams, flags);

But if you just want the data for the host as a whole
you can do

   virNodeGetCPUStats(conn, VIR_NODE_CPU_STATS_ALL_CPUS,
                      params, nparams, flags);

The /proc/stat file already has the data we need for both
these styles

$ grep cpu /proc/stat 
cpu  2632143 17400 1341532 32819705 1177560 674 21606 0 11997 0
cpu0 1263069 9280 596278 16274365 620428 631 17303 0 6131 0
cpu1 1369073 8119 745254 16545339 557131 43 4302 0 5865 0


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