On Sat, 16 Apr 2011 14:52:51 +0100 "Richard W.M. Jones" <rjones@redhat.com> wrote:
On Fri, Apr 15, 2011 at 04:04:52PM +0900, KAMEZAWA Hiroyuki wrote: [...]
The idea and the implementation is sound. However I have a problem with this proposed API:
+typedef struct _virDomainPcpuStat virDomainPcpuStatStruct; + +struct _virDomainPcpuStat { + unsigned long long cpuTime; +}; + +typedef virDomainPcpuStatStruct *virDomainPcpuStatPtr;
[...]
/** + * virDomainPcpuStats: + * @dom: pointer to the domain object + * @stats: nr_stats-sized array of stat structures (returned) + * @nr_stats: number of cpu statistics requested + * @flags: unused, always pass 0 + * + * This function provides per-cpu statistics for the domain. 'cpu' here means + * not vcpu. + * + * Up to 'nr_stats' elements of 'stats' will be populated with cpu statistics + * from the domain. Only statistics supported by the domain, the driver, and + * this version of libvirt will be returned. + * + * Now, only cpuTime per cpu is reported in nanoseconds. + * + * Returns: The number of stats provided or -1 in case of failure. + */ +int virDomainPcpuStats (virDomainPtr dom, virDomainPcpuStatPtr stats, + unsigned int nr_stats, unsigned int flags)
Am I correct that the caller sets nr_stats == number of physical cores, and allocates an array of nr_stats * virDomainPcpuStats structs?
We could never add more elements to the _virDomainPcpuStat structure, because that would break the ABI for existing callers. (Or I guess we could use the flags field).
I think we should forget about the structure, and just use an array of unsigned long long's, but I'd like to hear what others think.
I received the same reply from others and I'd like to go that way.
Is it likely that we would return other per-physical-CPU stats in future?
Not sure. But if some guy adds more statistics to cpuacct cgroup, as # of vmenter/vmexit etc...we can count it. So, I'd like to pass 'What you want' flag as VirDomainPhysicalCpuStat(domain, unsigned long long *starts, int nr_stats, int what_I_want) and then, pass VIT_DOMAIN_PHYSICAL_CPU_STAT_CPUTIME as it argument. what_I_want. Thanks, -Kame