[libvirt] [PATCHv2 0/6] Add virNodeGetCpuTime() API
Hi, This is v2 of virNodeGetCpuTime() API. It returns cpu utilization or cumulative cpu time of the node from /proc/stat since node boots up. This patch only supports linux host. Changes v1->v2 - Change user I/F like virDomainGetMemoryStats() - It can return either cpu utilization or cumulative cpu time of the node depends on each driver. Minoru Usui (6): [v2 1/6] virNodeGetCPUTime: Expose new API [v2 2/6] virNodeGetCPUTime: Define internal driver API [v2 3/6] virNodeGetCPUTime: Implement public API [v2 4/6] virNodeGetCPUTime: Implement remote protocol [v2 5/6] virNodeGetCPUTime: Implement virsh support [v2 6/6] virNodeGetCPUTime: Implement linux support daemon/remote.c | 48 +++++++++++++++++ daemon/remote_dispatch_args.h | 1 + daemon/remote_dispatch_prototypes.h | 8 +++ daemon/remote_dispatch_ret.h | 1 + daemon/remote_dispatch_table.h | 5 ++ include/libvirt/libvirt.h.in | 64 +++++++++++++++++++++++ src/driver.h | 8 +++ src/esx/esx_driver.c | 1 + src/libvirt.c | 70 +++++++++++++++++++++++++ src/libvirt_private.syms | 1 + src/libvirt_public.syms | 5 ++ src/libxl/libxl_driver.c | 1 + src/lxc/lxc_driver.c | 1 + src/nodeinfo.c | 78 ++++++++++++++++++++++++++++ src/nodeinfo.h | 5 ++- src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_driver.c | 1 + src/remote/remote_driver.c | 38 ++++++++++++++ src/remote/remote_protocol.c | 33 ++++++++++++ src/remote/remote_protocol.h | 28 ++++++++++ src/remote/remote_protocol.x | 20 +++++++- src/remote_protocol-structs | 14 +++++ src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_tmpl.c | 1 + src/vmware/vmware_driver.c | 1 + src/xen/xen_driver.c | 1 + src/xenapi/xenapi_driver.c | 1 + tools/virsh.c | 96 +++++++++++++++++++++++++++++++++++ tools/virsh.pod | 4 ++ 31 files changed, 537 insertions(+), 2 deletions(-) -- Minoru Usui <usui@mxm.nes.nec.co.jp>
virNodeGetCPUTime: Expose new API Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp> --- include/libvirt/libvirt.h.in | 64 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ 2 files changed, 69 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index bd36015..154c138 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -228,6 +228,57 @@ struct _virNodeInfo { unsigned int threads;/* number of threads per core */ }; +/** + * virNodeCpuTime: + * + * a virNodeCpuTime is a structure filled by virNodeGetCpuTime() and providing + * the information for the cpu time of the node. + */ + +/** + * Cpu Time Statistics Tags: + */ +typedef enum { + /* + * The cumulative CPU time which spends by kernel, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_KERNEL = 0, + /* + * The cumulative CPU time which spends by user processes, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_USER = 1, + /* + * The cumulative idle CPU time, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_IDLE = 2, + /* + * The cumulative I/O wait CPU time, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_IOWAIT = 3, + /* + * The CPU utilization. + * The usage value is in percent and 100% represents all CPUs on + * the server. + */ + VIR_NODE_CPU_TIME_UTILIZATION = 4, + + /* + * The number of statistics supported by this version of the interface. + * To add new statistics, add them to the enum and increase this value. + */ + VIR_NODE_CPU_TIME_NR = 5, +} virNodeCpuTimeTags; + +typedef struct _virNodeCpuTime virNodeCpuTime; + +struct _virNodeCpuTime { + virNodeCpuTimeTags tag; + unsigned long long val; +}; /** * virDomainSchedParameterType: @@ -460,6 +511,14 @@ int virDomainMigrateSetMaxSpeed(virDomainPtr domain, typedef virNodeInfo *virNodeInfoPtr; /** + * virNodeCpuTimePtr: + * + * a virNodeCpuTimePtr is a pointer to a virNodeCpuTime structure. + */ + +typedef virNodeCpuTime *virNodeCpuTimePtr; + +/** * virConnectFlags * * Flags when opening a connection to a hypervisor @@ -593,6 +652,11 @@ int virNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info); char * virConnectGetCapabilities (virConnectPtr conn); +int virNodeGetCpuTime (virConnectPtr conn, + virNodeCpuTimePtr stats, + unsigned int nr_stats, + unsigned int flags); + unsigned long long virNodeGetFreeMemory (virConnectPtr conn); int virNodeGetSecurityModel (virConnectPtr conn, diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index b4aed41..d292bdd 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -436,4 +436,9 @@ LIBVIRT_0.9.0 { virStorageVolUpload; } LIBVIRT_0.8.8; +LIBVIRT_0.9.1 { + global: + virNodeGetCpuTime; +} LIBVIRT_0.9.0; + # .... define new API here using predicted next version number .... -- 1.7.1 -- Minoru Usui <usui@mxm.nes.nec.co.jp>
On Fri, Apr 08, 2011 at 08:33:12PM +0900, Minoru Usui wrote:
virNodeGetCPUTime: Expose new API
Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp> --- include/libvirt/libvirt.h.in | 64 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ 2 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index bd36015..154c138 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -228,6 +228,57 @@ struct _virNodeInfo { unsigned int threads;/* number of threads per core */ };
+/** + * virNodeCpuTime: + * + * a virNodeCpuTime is a structure filled by virNodeGetCpuTime() and providing + * the information for the cpu time of the node. + */ + +/** + * Cpu Time Statistics Tags: + */ +typedef enum { + /* + * The cumulative CPU time which spends by kernel, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_KERNEL = 0, + /* + * The cumulative CPU time which spends by user processes, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_USER = 1, + /* + * The cumulative idle CPU time, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_IDLE = 2, + /* + * The cumulative I/O wait CPU time, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_IOWAIT = 3, + /* + * The CPU utilization. + * The usage value is in percent and 100% represents all CPUs on + * the server. + */ + VIR_NODE_CPU_TIME_UTILIZATION = 4, + + /* + * The number of statistics supported by this version of the interface. + * To add new statistics, add them to the enum and increase this value. + */ + VIR_NODE_CPU_TIME_NR = 5, +} virNodeCpuTimeTags; + +typedef struct _virNodeCpuTime virNodeCpuTime; + +struct _virNodeCpuTime { + virNodeCpuTimeTags tag; + unsigned long long val; +};
NACK, the size of an enum is not defined by the C language and hence is compiler dependant. We cannot use it as a component of a structure in the API.
/** * virDomainSchedParameterType: @@ -460,6 +511,14 @@ int virDomainMigrateSetMaxSpeed(virDomainPtr domain, typedef virNodeInfo *virNodeInfoPtr;
/** + * virNodeCpuTimePtr: + * + * a virNodeCpuTimePtr is a pointer to a virNodeCpuTime structure. + */ + +typedef virNodeCpuTime *virNodeCpuTimePtr; + +/** * virConnectFlags * * Flags when opening a connection to a hypervisor @@ -593,6 +652,11 @@ int virNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info); char * virConnectGetCapabilities (virConnectPtr conn);
+int virNodeGetCpuTime (virConnectPtr conn, + virNodeCpuTimePtr stats, + unsigned int nr_stats, + unsigned int flags); +
I don't understand how the API is suppoed to work. Suppose you want the cumulative CPU time, how do you ask for it ? Seems you can't ! You just ask for a big stucture hoping that on return you may find it within the results. That looks broken to me. Either you make a simple API giving back an unsigned long long and you use the virNodeCpuTimeTags as the flag value int virNodeGetCpuTime(virConnectPtr conn, unsigned long long *time, unsigned int flags); and the flags indicate the value you're interested into, but in that case you have to ask multiple times. Or you make the VIR_NODE_CPU_TIME_* values a bit field, and you pass an array int virNodeGetCpuTime(virConnectPtr conn, unsigned long long *stats, unsigned int nr_stats, unsigned int flags); where flags is the logical or of the values you are interested into, and the implementation put them in order based on the VIR_NODE_CPU_TIME_* values. But in that case you must fail if one of the statistics is not available. But an API where you "go fishing" and you don't know upfront on a successful return if you got the data you're interested into sounds broken to me Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Hi, Daniel. Thank you for your review. On Sun, 10 Apr 2011 12:58:56 +0800 Daniel Veillard <veillard@redhat.com> wrote:
On Fri, Apr 08, 2011 at 08:33:12PM +0900, Minoru Usui wrote:
virNodeGetCPUTime: Expose new API
Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp> --- include/libvirt/libvirt.h.in | 64 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ 2 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index bd36015..154c138 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -228,6 +228,57 @@ struct _virNodeInfo { unsigned int threads;/* number of threads per core */ };
+/** + * virNodeCpuTime: + * + * a virNodeCpuTime is a structure filled by virNodeGetCpuTime() and providing + * the information for the cpu time of the node. + */ + +/** + * Cpu Time Statistics Tags: + */ +typedef enum { + /* + * The cumulative CPU time which spends by kernel, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_KERNEL = 0, + /* + * The cumulative CPU time which spends by user processes, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_USER = 1, + /* + * The cumulative idle CPU time, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_IDLE = 2, + /* + * The cumulative I/O wait CPU time, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_IOWAIT = 3, + /* + * The CPU utilization. + * The usage value is in percent and 100% represents all CPUs on + * the server. + */ + VIR_NODE_CPU_TIME_UTILIZATION = 4, + + /* + * The number of statistics supported by this version of the interface. + * To add new statistics, add them to the enum and increase this value. + */ + VIR_NODE_CPU_TIME_NR = 5, +} virNodeCpuTimeTags; + +typedef struct _virNodeCpuTime virNodeCpuTime; + +struct _virNodeCpuTime { + virNodeCpuTimeTags tag; + unsigned long long val; +};
NACK, the size of an enum is not defined by the C language and hence is compiler dependant. We cannot use it as a component of a structure in the API.
OK. I'll change the enum to int.
/** * virDomainSchedParameterType: @@ -460,6 +511,14 @@ int virDomainMigrateSetMaxSpeed(virDomainPtr domain, typedef virNodeInfo *virNodeInfoPtr;
/** + * virNodeCpuTimePtr: + * + * a virNodeCpuTimePtr is a pointer to a virNodeCpuTime structure. + */ + +typedef virNodeCpuTime *virNodeCpuTimePtr; + +/** * virConnectFlags * * Flags when opening a connection to a hypervisor @@ -593,6 +652,11 @@ int virNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info); char * virConnectGetCapabilities (virConnectPtr conn);
+int virNodeGetCpuTime (virConnectPtr conn, + virNodeCpuTimePtr stats, + unsigned int nr_stats, + unsigned int flags); +
I don't understand how the API is suppoed to work. Suppose you want the cumulative CPU time, how do you ask for it ? Seems you can't ! You just ask for a big stucture hoping that on return you may find it within the results. That looks broken to me.
Either you make a simple API giving back an unsigned long long and you use the virNodeCpuTimeTags as the flag value
int virNodeGetCpuTime(virConnectPtr conn, unsigned long long *time, unsigned int flags);
and the flags indicate the value you're interested into, but in that case you have to ask multiple times. Or you make the VIR_NODE_CPU_TIME_* values a bit field, and you pass an array
int virNodeGetCpuTime(virConnectPtr conn, unsigned long long *stats, unsigned int nr_stats, unsigned int flags);
where flags is the logical or of the values you are interested into, and the implementation put them in order based on the VIR_NODE_CPU_TIME_* values. But in that case you must fail if one of the statistics is not available.
But an API where you "go fishing" and you don't know upfront on a successful return if you got the data you're interested into sounds broken to me
Thank you for your useful advice. I think the API user would be better to request which values return, too. I'll change the I/F to above 2nd style.
Daniel
-- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
-- Minoru Usui <usui@mxm.nes.nec.co.jp>
On Sun, Apr 10, 2011 at 12:58:56PM +0800, Daniel Veillard wrote:
On Fri, Apr 08, 2011 at 08:33:12PM +0900, Minoru Usui wrote:
virNodeGetCPUTime: Expose new API
Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp> --- include/libvirt/libvirt.h.in | 64 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ 2 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index bd36015..154c138 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -228,6 +228,57 @@ struct _virNodeInfo { unsigned int threads;/* number of threads per core */ };
+/** + * virNodeCpuTime: + * + * a virNodeCpuTime is a structure filled by virNodeGetCpuTime() and providing + * the information for the cpu time of the node. + */ + +/** + * Cpu Time Statistics Tags: + */ +typedef enum { + /* + * The cumulative CPU time which spends by kernel, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_KERNEL = 0, + /* + * The cumulative CPU time which spends by user processes, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_USER = 1, + /* + * The cumulative idle CPU time, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_IDLE = 2, + /* + * The cumulative I/O wait CPU time, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_IOWAIT = 3, + /* + * The CPU utilization. + * The usage value is in percent and 100% represents all CPUs on + * the server. + */ + VIR_NODE_CPU_TIME_UTILIZATION = 4, + + /* + * The number of statistics supported by this version of the interface. + * To add new statistics, add them to the enum and increase this value. + */ + VIR_NODE_CPU_TIME_NR = 5, +} virNodeCpuTimeTags; + +typedef struct _virNodeCpuTime virNodeCpuTime; + +struct _virNodeCpuTime { + virNodeCpuTimeTags tag; + unsigned long long val; +};
NACK, the size of an enum is not defined by the C language and hence is compiler dependant. We cannot use it as a component of a structure in the API.
/** * virDomainSchedParameterType: @@ -460,6 +511,14 @@ int virDomainMigrateSetMaxSpeed(virDomainPtr domain, typedef virNodeInfo *virNodeInfoPtr;
/** + * virNodeCpuTimePtr: + * + * a virNodeCpuTimePtr is a pointer to a virNodeCpuTime structure. + */ + +typedef virNodeCpuTime *virNodeCpuTimePtr; + +/** * virConnectFlags * * Flags when opening a connection to a hypervisor @@ -593,6 +652,11 @@ int virNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info); char * virConnectGetCapabilities (virConnectPtr conn);
+int virNodeGetCpuTime (virConnectPtr conn, + virNodeCpuTimePtr stats, + unsigned int nr_stats, + unsigned int flags); +
I don't understand how the API is suppoed to work. Suppose you want the cumulative CPU time, how do you ask for it ? Seems you can't ! You just ask for a big stucture hoping that on return you may find it within the results. That looks broken to me.
This is the same design we've used in the virDomainGetMemoryStats, virDomainGetBlkioParameters, virDomainGetSchedularParamters without any undue trouble.
Either you make a simple API giving back an unsigned long long and you use the virNodeCpuTimeTags as the flag value
int virNodeGetCpuTime(virConnectPtr conn, unsigned long long *time, unsigned int flags);
and the flags indicate the value you're interested into, but in that case you have to ask multiple times. Or you make the VIR_NODE_CPU_TIME_* values a bit field, and you pass an array
int virNodeGetCpuTime(virConnectPtr conn, unsigned long long *stats, unsigned int nr_stats, unsigned int flags);
where flags is the logical or of the values you are interested into, and the implementation put them in order based on the VIR_NODE_CPU_TIME_* values. But in that case you must fail if one of the statistics is not available.
I think these are worse because you're relying on ordering of values, and not all hypervisors will be able to supply all values. In addition it makes this stats API completely different to the other stats APIs we have. 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 :|
On Mon, 18 Apr 2011 12:30:45 +0100 "Daniel P. Berrange" <berrange@redhat.com> wrote:
On Sun, Apr 10, 2011 at 12:58:56PM +0800, Daniel Veillard wrote:
On Fri, Apr 08, 2011 at 08:33:12PM +0900, Minoru Usui wrote:
virNodeGetCPUTime: Expose new API
Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp> --- include/libvirt/libvirt.h.in | 64 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ 2 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index bd36015..154c138 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -228,6 +228,57 @@ struct _virNodeInfo { unsigned int threads;/* number of threads per core */ };
+/** + * virNodeCpuTime: + * + * a virNodeCpuTime is a structure filled by virNodeGetCpuTime() and providing + * the information for the cpu time of the node. + */ + +/** + * Cpu Time Statistics Tags: + */ +typedef enum { + /* + * The cumulative CPU time which spends by kernel, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_KERNEL = 0, + /* + * The cumulative CPU time which spends by user processes, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_USER = 1, + /* + * The cumulative idle CPU time, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_IDLE = 2, + /* + * The cumulative I/O wait CPU time, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_IOWAIT = 3, + /* + * The CPU utilization. + * The usage value is in percent and 100% represents all CPUs on + * the server. + */ + VIR_NODE_CPU_TIME_UTILIZATION = 4, + + /* + * The number of statistics supported by this version of the interface. + * To add new statistics, add them to the enum and increase this value. + */ + VIR_NODE_CPU_TIME_NR = 5, +} virNodeCpuTimeTags; + +typedef struct _virNodeCpuTime virNodeCpuTime; + +struct _virNodeCpuTime { + virNodeCpuTimeTags tag; + unsigned long long val; +};
NACK, the size of an enum is not defined by the C language and hence is compiler dependant. We cannot use it as a component of a structure in the API.
/** * virDomainSchedParameterType: @@ -460,6 +511,14 @@ int virDomainMigrateSetMaxSpeed(virDomainPtr domain, typedef virNodeInfo *virNodeInfoPtr;
/** + * virNodeCpuTimePtr: + * + * a virNodeCpuTimePtr is a pointer to a virNodeCpuTime structure. + */ + +typedef virNodeCpuTime *virNodeCpuTimePtr; + +/** * virConnectFlags * * Flags when opening a connection to a hypervisor @@ -593,6 +652,11 @@ int virNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info); char * virConnectGetCapabilities (virConnectPtr conn);
+int virNodeGetCpuTime (virConnectPtr conn, + virNodeCpuTimePtr stats, + unsigned int nr_stats, + unsigned int flags); +
I don't understand how the API is suppoed to work. Suppose you want the cumulative CPU time, how do you ask for it ? Seems you can't ! You just ask for a big stucture hoping that on return you may find it within the results. That looks broken to me.
This is the same design we've used in the virDomainGetMemoryStats, virDomainGetBlkioParameters, virDomainGetSchedularParamters without any undue trouble.
Yes.
Either you make a simple API giving back an unsigned long long and you use the virNodeCpuTimeTags as the flag value
int virNodeGetCpuTime(virConnectPtr conn, unsigned long long *time, unsigned int flags);
and the flags indicate the value you're interested into, but in that case you have to ask multiple times. Or you make the VIR_NODE_CPU_TIME_* values a bit field, and you pass an array
int virNodeGetCpuTime(virConnectPtr conn, unsigned long long *stats, unsigned int nr_stats, unsigned int flags);
where flags is the logical or of the values you are interested into, and the implementation put them in order based on the VIR_NODE_CPU_TIME_* values. But in that case you must fail if one of the statistics is not available.
I think these are worse because you're relying on ordering of values, and not all hypervisors will be able to supply all values. In addition it makes this stats API completely different to the other stats APIs we have.
OK. I drop v3 design. Everyone, how about this v2 patch series? Any other opinions?
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 :|
-- Minoru Usui <usui@mxm.nes.nec.co.jp>
On Fri, Apr 08, 2011 at 08:33:12PM +0900, Minoru Usui wrote:
virNodeGetCPUTime: Expose new API
Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp> --- include/libvirt/libvirt.h.in | 64 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ 2 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index bd36015..154c138 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -228,6 +228,57 @@ struct _virNodeInfo { unsigned int threads;/* number of threads per core */ };
+/** + * virNodeCpuTime: + * + * a virNodeCpuTime is a structure filled by virNodeGetCpuTime() and providing + * the information for the cpu time of the node. + */ + +/** + * Cpu Time Statistics Tags: + */ +typedef enum { + /* + * The cumulative CPU time which spends by kernel, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_KERNEL = 0, + /* + * The cumulative CPU time which spends by user processes, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_USER = 1, + /* + * The cumulative idle CPU time, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_IDLE = 2, + /* + * The cumulative I/O wait CPU time, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_IOWAIT = 3, + /* + * The CPU utilization. + * The usage value is in percent and 100% represents all CPUs on + * the server. + */ + VIR_NODE_CPU_TIME_UTILIZATION = 4, + + /* + * The number of statistics supported by this version of the interface. + * To add new statistics, add them to the enum and increase this value. + */ + VIR_NODE_CPU_TIME_NR = 5, +} virNodeCpuTimeTags; + +typedef struct _virNodeCpuTime virNodeCpuTime; + +struct _virNodeCpuTime { + virNodeCpuTimeTags tag; + unsigned long long val; +};
I've just remembered that the virSchedParameter, virMemoryParameter and virBlkioParameter structs all use a string to represent the data value, rather than an enum. I wonder if we ought todo the same here. eg, something like #define VIR_NODE_CPUE_FIELD_LENGTH 80 struct _virNodeCpuParameter { char field[VIR_NODE_CPU_FIELD_LENGTH] unsigned long long value; }; They also have a union for returning different data types, beyond just 'unsigned long long' but I think that might be overkill here. 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 :|
Hi, I'm sorry for late reply. On Tue, 19 Apr 2011 11:48:55 +0100 "Daniel P. Berrange" <berrange@redhat.com> wrote:
On Fri, Apr 08, 2011 at 08:33:12PM +0900, Minoru Usui wrote:
virNodeGetCPUTime: Expose new API
Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp> --- include/libvirt/libvirt.h.in | 64 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ 2 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index bd36015..154c138 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -228,6 +228,57 @@ struct _virNodeInfo { unsigned int threads;/* number of threads per core */ };
+/** + * virNodeCpuTime: + * + * a virNodeCpuTime is a structure filled by virNodeGetCpuTime() and providing + * the information for the cpu time of the node. + */ + +/** + * Cpu Time Statistics Tags: + */ +typedef enum { + /* + * The cumulative CPU time which spends by kernel, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_KERNEL = 0, + /* + * The cumulative CPU time which spends by user processes, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_USER = 1, + /* + * The cumulative idle CPU time, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_IDLE = 2, + /* + * The cumulative I/O wait CPU time, + * when the node booting up.(in nanoseconds). + */ + VIR_NODE_CPU_TIME_IOWAIT = 3, + /* + * The CPU utilization. + * The usage value is in percent and 100% represents all CPUs on + * the server. + */ + VIR_NODE_CPU_TIME_UTILIZATION = 4, + + /* + * The number of statistics supported by this version of the interface. + * To add new statistics, add them to the enum and increase this value. + */ + VIR_NODE_CPU_TIME_NR = 5, +} virNodeCpuTimeTags; + +typedef struct _virNodeCpuTime virNodeCpuTime; + +struct _virNodeCpuTime { + virNodeCpuTimeTags tag; + unsigned long long val; +};
I've just remembered that the virSchedParameter, virMemoryParameter and virBlkioParameter structs all use a string to represent the data value, rather than an enum. I wonder if we ought todo the same here.
eg, something like
#define VIR_NODE_CPUE_FIELD_LENGTH 80
struct _virNodeCpuParameter { char field[VIR_NODE_CPU_FIELD_LENGTH] unsigned long long value; };
They also have a union for returning different data types, beyond just 'unsigned long long' but I think that might be overkill here.
I implemented this based on virDomainMemoryStats(). Is it better that the patches is based on virDomainGetMemoryParameters()/BlkioParameters()? If so, I'll rebase these patch series. -- Minoru Usui <usui@mxm.nes.nec.co.jp>
virNodeGetCPUTime: Define internal driver API Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp> --- src/driver.h | 8 ++++++++ src/esx/esx_driver.c | 1 + src/libxl/libxl_driver.c | 1 + src/lxc/lxc_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_driver.c | 1 + src/remote/remote_driver.c | 1 + src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_tmpl.c | 1 + src/vmware/vmware_driver.c | 1 + src/xen/xen_driver.c | 1 + src/xenapi/xenapi_driver.c | 1 + 14 files changed, 21 insertions(+), 0 deletions(-) diff --git a/src/driver.h b/src/driver.h index e5f91ca..e03bcd4 100644 --- a/src/driver.h +++ b/src/driver.h @@ -347,6 +347,13 @@ typedef struct _virDriver virDriver; typedef virDriver *virDriverPtr; typedef int + (*virDrvNodeGetCpuTime) + (virConnectPtr conn, + struct _virNodeCpuTime *stats, + unsigned int nr_stats, + unsigned int flags); + +typedef int (*virDrvNodeGetCellsFreeMemory) (virConnectPtr conn, unsigned long long *freeMems, @@ -602,6 +609,7 @@ struct _virDriver { virDrvDomainBlockPeek domainBlockPeek; virDrvDomainMemoryPeek domainMemoryPeek; virDrvDomainGetBlockInfo domainGetBlockInfo; + virDrvNodeGetCpuTime nodeGetCpuTime; virDrvNodeGetCellsFreeMemory nodeGetCellsFreeMemory; virDrvNodeGetFreeMemory getFreeMemory; virDrvDomainEventRegister domainEventRegister; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index deda372..6e55949 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4638,6 +4638,7 @@ static virDriver esxDriver = { NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* domainGetBlockInfo */ + NULL, /* nodeGetCpuTime */ NULL, /* nodeGetCellsFreeMemory */ esxNodeGetFreeMemory, /* nodeGetFreeMemory */ NULL, /* domainEventRegister */ diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 75f99c1..a9907a1 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2443,6 +2443,7 @@ static virDriver libxlDriver = { NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* domainGetBlockInfo */ + NULL, /* nodeGetCpuTime */ NULL, /* nodeGetCellsFreeMemory */ libxlNodeGetFreeMemory, /* getFreeMemory */ libxlDomainEventRegister, /* domainEventRegister */ diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index e905302..0e0c325 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2869,6 +2869,7 @@ static virDriver lxcDriver = { NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* domainGetBlockInfo */ + nodeGetCpuTime, /* nodeGetCpuTime */ nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ nodeGetFreeMemory, /* getFreeMemory */ lxcDomainEventRegister, /* domainEventRegister */ diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index fb30c37..7335c8d 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1617,6 +1617,7 @@ static virDriver openvzDriver = { NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* domainGetBlockInfo */ + NULL, /* nodeGetCpuTime */ NULL, /* nodeGetCellsFreeMemory */ NULL, /* getFreeMemory */ NULL, /* domainEventRegister */ diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index b17d90b..e274cef 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -4026,6 +4026,7 @@ static virDriver phypDriver = { NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* domainGetBlockInfo */ + NULL, /* nodeGetCpuTime */ NULL, /* nodeGetCellsFreeMemory */ NULL, /* getFreeMemory */ NULL, /* domainEventRegister */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 48fe266..dca8f5c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6889,6 +6889,7 @@ static virDriver qemuDriver = { qemudDomainBlockPeek, /* domainBlockPeek */ qemudDomainMemoryPeek, /* domainMemoryPeek */ qemuDomainGetBlockInfo, /* domainGetBlockInfo */ + nodeGetCpuTime, /* nodeGetCpuTime */ nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ nodeGetFreeMemory, /* getFreeMemory */ qemuDomainEventRegister, /* domainEventRegister */ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 166968a..8bac0cc 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -11261,6 +11261,7 @@ static virDriver remote_driver = { remoteDomainBlockPeek, /* domainBlockPeek */ remoteDomainMemoryPeek, /* domainMemoryPeek */ remoteDomainGetBlockInfo, /* domainGetBlockInfo */ + remoteNodeGetCpuTime, /* nodeGetCpuTime */ remoteNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ remoteNodeGetFreeMemory, /* getFreeMemory */ remoteDomainEventRegister, /* domainEventRegister */ diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 17f5ad9..f122935 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5410,6 +5410,7 @@ static virDriver testDriver = { NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* domainGetBlockInfo */ + NULL, /* nodeGetCpuTime */ testNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ NULL, /* getFreeMemory */ testDomainEventRegister, /* domainEventRegister */ diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index e2bd5f2..93fb59e 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2212,6 +2212,7 @@ static virDriver umlDriver = { umlDomainBlockPeek, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* domainGetBlockInfo */ + nodeGetCpuTime, /* nodeGetCpuTime */ nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ nodeGetFreeMemory, /* getFreeMemory */ NULL, /* domainEventRegister */ diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 0fbfba5..d500ab2 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -8600,6 +8600,7 @@ virDriver NAME(Driver) = { NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* domainGetBlockInfo */ + NULL, /* nodeGetCpuTime */ nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ nodeGetFreeMemory, /* getFreeMemory */ #if VBOX_API_VERSION == 2002 || VBOX_API_VERSION == 4000 diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index b5e416b..635aa59 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -970,6 +970,7 @@ static virDriver vmwareDriver = { NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* domainGetBlockInfo */ + NULL, /* nodeGetCpuTime */ NULL, /* nodeGetCellsFreeMemory */ NULL, /* getFreeMemory */ NULL, /* domainEventRegister */ diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 9f47722..af18baa 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -2104,6 +2104,7 @@ static virDriver xenUnifiedDriver = { xenUnifiedDomainBlockPeek, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* domainGetBlockInfo */ + NULL, /* nodeGetCpuTime */ xenUnifiedNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ xenUnifiedNodeGetFreeMemory, /* getFreeMemory */ xenUnifiedDomainEventRegister, /* domainEventRegister */ diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 60b23c7..6371d03 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1852,6 +1852,7 @@ static virDriver xenapiDriver = { NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* domainGetBlockInfo */ + NULL, /* nodeGetCpuTime */ xenapiNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ xenapiNodeGetFreeMemory, /* getFreeMemory */ NULL, /* domainEventRegister */ -- 1.7.1 -- Minoru Usui <usui@mxm.nes.nec.co.jp>
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; + + 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; + } + + if (!stats || nr_stats == 0) + return 0; + + if (nr_stats > VIR_NODE_CPU_TIME_NR) + nr_stats = VIR_NODE_CPU_TIME_NR; + + 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; +} + +/** * virNodeGetFreeMemory: * @conn: pointer to the hypervisor connection * -- 1.7.1 -- Minoru Usui <usui@mxm.nes.nec.co.jp>
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 :|
On Tue, 19 Apr 2011 11:53:31 +0100 "Daniel P. Berrange" <berrange@redhat.com> wrote:
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.
Ouch. I'll change it.
+ + 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);
OK. I'll change it.
+ 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; }
I'll rebase these patches, if it's better to be based on virDomainGetMemoryParameters()/BlkioParameters()?
+ + 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 :|
-- Minoru Usui <usui@mxm.nes.nec.co.jp>
virNodeGetCPUTime: Implement remote protocol Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp> --- daemon/remote.c | 48 +++++++++++++++++++++++++++++++++++ daemon/remote_dispatch_args.h | 1 + daemon/remote_dispatch_prototypes.h | 8 ++++++ daemon/remote_dispatch_ret.h | 1 + daemon/remote_dispatch_table.h | 5 +++ src/remote/remote_driver.c | 37 +++++++++++++++++++++++++++ src/remote/remote_protocol.c | 33 ++++++++++++++++++++++++ src/remote/remote_protocol.h | 28 ++++++++++++++++++++ src/remote/remote_protocol.x | 20 ++++++++++++++- src/remote_protocol-structs | 14 ++++++++++ 10 files changed, 194 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 1700c2d..a6c21eb 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -679,6 +679,54 @@ remoteDispatchGetCapabilities (struct qemud_server *server ATTRIBUTE_UNUSED, } static int +remoteDispatchNodeGetCpuTime (struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error *rerr, + remote_node_get_cpu_time_args *args, + remote_node_get_cpu_time_ret *ret) +{ + struct _virNodeCpuTime *stats; + unsigned int nr_stats, i; + + if (args->maxStats > REMOTE_NODE_CPU_TIME_MAX) { + remoteDispatchFormatError (rerr, "%s", + _("maxStats > REMOTE_NODE_CPU_TIME_MAX")); + return -1; + } + + /* Allocate stats array for making dispatch call */ + if (VIR_ALLOC_N(stats, args->maxStats) < 0) { + remoteDispatchOOMError(rerr); + return -1; + } + + nr_stats = virNodeGetCpuTime (conn, stats, args->maxStats, 0); + if (nr_stats == -1) { + VIR_FREE(stats); + remoteDispatchConnError(rerr, conn); + return -1; + } + + /* Allocate return buffer */ + if (VIR_ALLOC_N(ret->stats.stats_val, args->maxStats) < 0) { + VIR_FREE(stats); + remoteDispatchOOMError(rerr); + return -1; + } + + /* Copy the stats into the xdr return structure */ + for (i = 0; i < nr_stats; i++) { + ret->stats.stats_val[i].tag = stats[i].tag; + ret->stats.stats_val[i].val = stats[i].val; + } + ret->stats.stats_len = nr_stats; + VIR_FREE(stats); + return 0; +} + +static int remoteDispatchNodeGetCellsFreeMemory (struct qemud_server *server ATTRIBUTE_UNUSED, struct qemud_client *client ATTRIBUTE_UNUSED, virConnectPtr conn, diff --git a/daemon/remote_dispatch_args.h b/daemon/remote_dispatch_args.h index f9537d7..a757899 100644 --- a/daemon/remote_dispatch_args.h +++ b/daemon/remote_dispatch_args.h @@ -178,3 +178,4 @@ remote_domain_migrate_set_max_speed_args val_remote_domain_migrate_set_max_speed_args; remote_storage_vol_upload_args val_remote_storage_vol_upload_args; remote_storage_vol_download_args val_remote_storage_vol_download_args; + remote_node_get_cpu_time_args val_remote_node_get_cpu_time_args; diff --git a/daemon/remote_dispatch_prototypes.h b/daemon/remote_dispatch_prototypes.h index 18bf41d..936ba23 100644 --- a/daemon/remote_dispatch_prototypes.h +++ b/daemon/remote_dispatch_prototypes.h @@ -1138,6 +1138,14 @@ static int remoteDispatchNodeGetCellsFreeMemory( remote_error *err, remote_node_get_cells_free_memory_args *args, remote_node_get_cells_free_memory_ret *ret); +static int remoteDispatchNodeGetCpuTime( + struct qemud_server *server, + struct qemud_client *client, + virConnectPtr conn, + remote_message_header *hdr, + remote_error *err, + remote_node_get_cpu_time_args *args, + remote_node_get_cpu_time_ret *ret); static int remoteDispatchNodeGetFreeMemory( struct qemud_server *server, struct qemud_client *client, diff --git a/daemon/remote_dispatch_ret.h b/daemon/remote_dispatch_ret.h index 114e832..dfed98b 100644 --- a/daemon/remote_dispatch_ret.h +++ b/daemon/remote_dispatch_ret.h @@ -140,3 +140,4 @@ remote_domain_is_updated_ret val_remote_domain_is_updated_ret; remote_get_sysinfo_ret val_remote_get_sysinfo_ret; remote_domain_get_blkio_parameters_ret val_remote_domain_get_blkio_parameters_ret; + remote_node_get_cpu_time_ret val_remote_node_get_cpu_time_ret; diff --git a/daemon/remote_dispatch_table.h b/daemon/remote_dispatch_table.h index b39f7c2..5922d56 100644 --- a/daemon/remote_dispatch_table.h +++ b/daemon/remote_dispatch_table.h @@ -1052,3 +1052,8 @@ .args_filter = (xdrproc_t) xdr_remote_storage_vol_download_args, .ret_filter = (xdrproc_t) xdr_void, }, +{ /* NodeGetCpuTime => 210 */ + .fn = (dispatch_fn) remoteDispatchNodeGetCpuTime, + .args_filter = (xdrproc_t) xdr_remote_node_get_cpu_time_args, + .ret_filter = (xdrproc_t) xdr_remote_node_get_cpu_time_ret, +}, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 8bac0cc..20c6e75 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1936,6 +1936,43 @@ done: } static int +remoteNodeGetCpuTime (virConnectPtr conn, + virNodeCpuTimePtr stats, + unsigned int nr_stats, + unsigned int flags) +{ + int rv = -1; + remote_node_get_cpu_time_args args; + remote_node_get_cpu_time_ret ret; + struct private_data *priv = conn->privateData; + unsigned int i; + + remoteDriverLock(priv); + + args.maxStats = nr_stats; + args.flags = flags; + memset (&ret, 0, sizeof ret); + + if (call (conn, priv, 0, REMOTE_PROC_NODE_GET_CPU_TIME, + (xdrproc_t) xdr_remote_node_get_cpu_time_args, + (char *) &args, + (xdrproc_t) xdr_remote_node_get_cpu_time_ret, + (char *) &ret) == -1) + goto done; + + for (i = 0; i < ret.stats.stats_len; i++) { + stats[i].tag = ret.stats.stats_val[i].tag; + stats[i].val = ret.stats.stats_val[i].val; + } + rv = ret.stats.stats_len; + xdr_free((xdrproc_t) xdr_remote_node_get_cpu_time_ret, (char *) &ret); + +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *freeMems, int startCell, diff --git a/src/remote/remote_protocol.c b/src/remote/remote_protocol.c index 5604371..e7bb93d 100644 --- a/src/remote/remote_protocol.c +++ b/src/remote/remote_protocol.c @@ -607,6 +607,39 @@ xdr_remote_get_capabilities_ret (XDR *xdrs, remote_get_capabilities_ret *objp) } bool_t +xdr_remote_node_get_cpu_time_args (XDR *xdrs, remote_node_get_cpu_time_args *objp) +{ + + if (!xdr_u_int (xdrs, &objp->maxStats)) + return FALSE; + if (!xdr_u_int (xdrs, &objp->flags)) + return FALSE; + return TRUE; +} + +bool_t +xdr_remote_node_cpu_time (XDR *xdrs, remote_node_cpu_time *objp) +{ + + if (!xdr_int (xdrs, &objp->tag)) + return FALSE; + if (!xdr_uint64_t (xdrs, &objp->val)) + return FALSE; + return TRUE; +} + +bool_t +xdr_remote_node_get_cpu_time_ret (XDR *xdrs, remote_node_get_cpu_time_ret *objp) +{ + char **objp_cpp0 = (char **) (void *) &objp->stats.stats_val; + + if (!xdr_array (xdrs, objp_cpp0, (u_int *) &objp->stats.stats_len, REMOTE_NODE_CPU_TIME_MAX, + sizeof (remote_node_cpu_time), (xdrproc_t) xdr_remote_node_cpu_time)) + return FALSE; + return TRUE; +} + +bool_t xdr_remote_node_get_cells_free_memory_args (XDR *xdrs, remote_node_get_cells_free_memory_args *objp) { diff --git a/src/remote/remote_protocol.h b/src/remote/remote_protocol.h index d9bf151..42ad76b 100644 --- a/src/remote/remote_protocol.h +++ b/src/remote/remote_protocol.h @@ -55,6 +55,7 @@ typedef remote_nonnull_string *remote_string; #define REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX 16 #define REMOTE_DOMAIN_BLKIO_PARAMETERS_MAX 16 #define REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX 16 +#define REMOTE_NODE_CPU_TIME_MAX 8 #define REMOTE_NODE_MAX_CELLS 1024 #define REMOTE_AUTH_SASL_DATA_MAX 65536 #define REMOTE_AUTH_TYPE_LIST_MAX 20 @@ -304,6 +305,26 @@ struct remote_get_capabilities_ret { }; typedef struct remote_get_capabilities_ret remote_get_capabilities_ret; +struct remote_node_get_cpu_time_args { + u_int maxStats; + u_int flags; +}; +typedef struct remote_node_get_cpu_time_args remote_node_get_cpu_time_args; + +struct remote_node_cpu_time { + int tag; + uint64_t val; +}; +typedef struct remote_node_cpu_time remote_node_cpu_time; + +struct remote_node_get_cpu_time_ret { + struct { + u_int stats_len; + remote_node_cpu_time *stats_val; + } stats; +}; +typedef struct remote_node_get_cpu_time_ret remote_node_get_cpu_time_ret; + struct remote_node_get_cells_free_memory_args { int startCell; int maxCells; @@ -2413,6 +2434,7 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_MIGRATE_SET_MAX_SPEED = 207, REMOTE_PROC_STORAGE_VOL_UPLOAD = 208, REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209, + REMOTE_PROC_NODE_GET_CPU_TIME = 210, }; typedef enum remote_procedure remote_procedure; @@ -2486,6 +2508,9 @@ extern bool_t xdr_remote_get_max_vcpus_args (XDR *, remote_get_max_vcpus_args*) extern bool_t xdr_remote_get_max_vcpus_ret (XDR *, remote_get_max_vcpus_ret*); extern bool_t xdr_remote_node_get_info_ret (XDR *, remote_node_get_info_ret*); extern bool_t xdr_remote_get_capabilities_ret (XDR *, remote_get_capabilities_ret*); +extern bool_t xdr_remote_node_get_cpu_time_args (XDR *, remote_node_get_cpu_time_args*); +extern bool_t xdr_remote_node_cpu_time (XDR *, remote_node_cpu_time*); +extern bool_t xdr_remote_node_get_cpu_time_ret (XDR *, remote_node_get_cpu_time_ret*); extern bool_t xdr_remote_node_get_cells_free_memory_args (XDR *, remote_node_get_cells_free_memory_args*); extern bool_t xdr_remote_node_get_cells_free_memory_ret (XDR *, remote_node_get_cells_free_memory_ret*); extern bool_t xdr_remote_node_get_free_memory_ret (XDR *, remote_node_get_free_memory_ret*); @@ -2843,6 +2868,9 @@ extern bool_t xdr_remote_get_max_vcpus_args (); extern bool_t xdr_remote_get_max_vcpus_ret (); extern bool_t xdr_remote_node_get_info_ret (); extern bool_t xdr_remote_get_capabilities_ret (); +extern bool_t xdr_remote_node_get_cpu_time_args (); +extern bool_t xdr_remote_node_cpu_time (); +extern bool_t xdr_remote_node_get_cpu_time_ret (); extern bool_t xdr_remote_node_get_cells_free_memory_args (); extern bool_t xdr_remote_node_get_cells_free_memory_ret (); extern bool_t xdr_remote_node_get_free_memory_ret (); diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 675eccd..46e789c 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -134,6 +134,9 @@ const REMOTE_DOMAIN_BLKIO_PARAMETERS_MAX = 16; /* Upper limit on list of memory parameters. */ const REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX = 16; +/* Upper limit on list of cpu time stats. */ +const REMOTE_NODE_CPU_TIME_MAX = 8; + /* Upper limit on number of NUMA cells */ const REMOTE_NODE_MAX_CELLS = 1024; @@ -440,6 +443,20 @@ struct remote_get_capabilities_ret { remote_nonnull_string capabilities; }; +struct remote_node_get_cpu_time_args { + u_int maxStats; + u_int flags; +}; + +struct remote_node_cpu_time { + int tag; + unsigned hyper val; +}; + +struct remote_node_get_cpu_time_ret { + remote_node_cpu_time stats<REMOTE_NODE_CPU_TIME_MAX>; +}; + struct remote_node_get_cells_free_memory_args { int startCell; int maxCells; @@ -2176,7 +2193,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_BLKIO_PARAMETERS = 206, REMOTE_PROC_DOMAIN_MIGRATE_SET_MAX_SPEED = 207, REMOTE_PROC_STORAGE_VOL_UPLOAD = 208, - REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209 + REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209, + REMOTE_PROC_NODE_GET_CPU_TIME = 210 /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 944553c..adf1fb5 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -151,6 +151,20 @@ struct remote_node_get_info_ret { struct remote_get_capabilities_ret { remote_nonnull_string capabilities; }; +struct remote_node_get_cpu_time_args { + u_int maxStats; + u_int flags; +}; +struct remote_node_cpu_time { + int tag; + uint64_t val; +}; +struct remote_node_get_cpu_time_ret { + struct { + u_int stats_len; + remote_node_cpu_time *stats_val; + } stats; +}; struct remote_node_get_cells_free_memory_args { int startCell; int maxCells; -- 1.7.1 -- Minoru Usui <usui@mxm.nes.nec.co.jp>
On Fri, Apr 08, 2011 at 08:36:02PM +0900, Minoru Usui wrote:
virNodeGetCPUTime: Implement remote protocol
Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp> --- daemon/remote.c | 48 +++++++++++++++++++++++++++++++++++ daemon/remote_dispatch_args.h | 1 + daemon/remote_dispatch_prototypes.h | 8 ++++++ daemon/remote_dispatch_ret.h | 1 + daemon/remote_dispatch_table.h | 5 +++ src/remote/remote_driver.c | 37 +++++++++++++++++++++++++++ src/remote/remote_protocol.c | 33 ++++++++++++++++++++++++ src/remote/remote_protocol.h | 28 ++++++++++++++++++++ src/remote/remote_protocol.x | 20 ++++++++++++++- src/remote_protocol-structs | 14 ++++++++++ 10 files changed, 194 insertions(+), 1 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 1700c2d..a6c21eb 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -679,6 +679,54 @@ remoteDispatchGetCapabilities (struct qemud_server *server ATTRIBUTE_UNUSED, }
static int +remoteDispatchNodeGetCpuTime (struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error *rerr, + remote_node_get_cpu_time_args *args, + remote_node_get_cpu_time_ret *ret) +{ + struct _virNodeCpuTime *stats; + unsigned int nr_stats, i; + + if (args->maxStats > REMOTE_NODE_CPU_TIME_MAX) { + remoteDispatchFormatError (rerr, "%s", + _("maxStats > REMOTE_NODE_CPU_TIME_MAX")); + return -1; + } + + /* Allocate stats array for making dispatch call */ + if (VIR_ALLOC_N(stats, args->maxStats) < 0) { + remoteDispatchOOMError(rerr); + return -1; + } + + nr_stats = virNodeGetCpuTime (conn, stats, args->maxStats, 0); + if (nr_stats == -1) { + VIR_FREE(stats); + remoteDispatchConnError(rerr, conn); + return -1; + } + + /* Allocate return buffer */ + if (VIR_ALLOC_N(ret->stats.stats_val, args->maxStats) < 0) { + VIR_FREE(stats); + remoteDispatchOOMError(rerr); + return -1; + } + + /* Copy the stats into the xdr return structure */ + for (i = 0; i < nr_stats; i++) { + ret->stats.stats_val[i].tag = stats[i].tag; + ret->stats.stats_val[i].val = stats[i].val; + } + ret->stats.stats_len = nr_stats; + VIR_FREE(stats); + return 0; +}
I recently re-structured all the APIs in this file, to follow a new coding style. This method would need to be updated to match. { virNodeCpuTime *stats = NULL; int nr_stats, i; int rv = -1; if (args->maxStats > REMOTE_NODE_CPU_TIME_MAX) { virNetError (VIR_ERR_INTERNAL_ERROR, "%s", _("maxStats > REMOTE_NODE_CPU_TIME_MAX")); goto cleanup; } /* Allocate stats array for making dispatch call */ if (VIR_ALLOC_N(stats, args->maxStats) < 0) { virReportOOMError(); goto cleanup; } if ((nr_stats = virNodeGetCpuTime (conn, stats, args->maxStats, 0)) < 0) goto cleanup; /* Allocate return buffer */ if (VIR_ALLOC_N(ret->stats.stats_val, args->maxStats) < 0) { virReportOOMError(); goto cleanup; } /* Copy the stats into the xdr return structure */ for (i = 0; i < nr_stats; i++) { ret->stats.stats_val[i].tag = stats[i].tag; ret->stats.stats_val[i].val = stats[i].val; } ret->stats.stats_len = nr_stats; rv = 0; cleanup: if (rv < 0) remoteDispatchError(rerr); VIR_FREE(stats); return rv; } Of course, if we change the public API as I suggest, it'll need a couple more changes too. 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 :|
On Tue, 19 Apr 2011 11:58:00 +0100 "Daniel P. Berrange" <berrange@redhat.com> wrote:
On Fri, Apr 08, 2011 at 08:36:02PM +0900, Minoru Usui wrote:
virNodeGetCPUTime: Implement remote protocol
Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp> --- daemon/remote.c | 48 +++++++++++++++++++++++++++++++++++ daemon/remote_dispatch_args.h | 1 + daemon/remote_dispatch_prototypes.h | 8 ++++++ daemon/remote_dispatch_ret.h | 1 + daemon/remote_dispatch_table.h | 5 +++ src/remote/remote_driver.c | 37 +++++++++++++++++++++++++++ src/remote/remote_protocol.c | 33 ++++++++++++++++++++++++ src/remote/remote_protocol.h | 28 ++++++++++++++++++++ src/remote/remote_protocol.x | 20 ++++++++++++++- src/remote_protocol-structs | 14 ++++++++++ 10 files changed, 194 insertions(+), 1 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 1700c2d..a6c21eb 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -679,6 +679,54 @@ remoteDispatchGetCapabilities (struct qemud_server *server ATTRIBUTE_UNUSED, }
static int +remoteDispatchNodeGetCpuTime (struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error *rerr, + remote_node_get_cpu_time_args *args, + remote_node_get_cpu_time_ret *ret) +{ + struct _virNodeCpuTime *stats; + unsigned int nr_stats, i; + + if (args->maxStats > REMOTE_NODE_CPU_TIME_MAX) { + remoteDispatchFormatError (rerr, "%s", + _("maxStats > REMOTE_NODE_CPU_TIME_MAX")); + return -1; + } + + /* Allocate stats array for making dispatch call */ + if (VIR_ALLOC_N(stats, args->maxStats) < 0) { + remoteDispatchOOMError(rerr); + return -1; + } + + nr_stats = virNodeGetCpuTime (conn, stats, args->maxStats, 0); + if (nr_stats == -1) { + VIR_FREE(stats); + remoteDispatchConnError(rerr, conn); + return -1; + } + + /* Allocate return buffer */ + if (VIR_ALLOC_N(ret->stats.stats_val, args->maxStats) < 0) { + VIR_FREE(stats); + remoteDispatchOOMError(rerr); + return -1; + } + + /* Copy the stats into the xdr return structure */ + for (i = 0; i < nr_stats; i++) { + ret->stats.stats_val[i].tag = stats[i].tag; + ret->stats.stats_val[i].val = stats[i].val; + } + ret->stats.stats_len = nr_stats; + VIR_FREE(stats); + return 0; +}
I recently re-structured all the APIs in this file, to follow a new coding style. This method would need to be updated to match.
OK. I'll change this to following style.
{ virNodeCpuTime *stats = NULL; int nr_stats, i; int rv = -1;
if (args->maxStats > REMOTE_NODE_CPU_TIME_MAX) { virNetError (VIR_ERR_INTERNAL_ERROR, "%s", _("maxStats > REMOTE_NODE_CPU_TIME_MAX")); goto cleanup; }
/* Allocate stats array for making dispatch call */ if (VIR_ALLOC_N(stats, args->maxStats) < 0) { virReportOOMError(); goto cleanup; }
if ((nr_stats = virNodeGetCpuTime (conn, stats, args->maxStats, 0)) < 0) goto cleanup;
/* Allocate return buffer */ if (VIR_ALLOC_N(ret->stats.stats_val, args->maxStats) < 0) { virReportOOMError(); goto cleanup; }
/* Copy the stats into the xdr return structure */ for (i = 0; i < nr_stats; i++) { ret->stats.stats_val[i].tag = stats[i].tag; ret->stats.stats_val[i].val = stats[i].val; } ret->stats.stats_len = nr_stats; rv = 0;
cleanup: if (rv < 0) remoteDispatchError(rerr); VIR_FREE(stats); return rv; }
Of course, if we change the public API as I suggest, it'll need a couple more changes too.
Yes. I'll change this to suite public API, of cource.
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 :|
-- Minoru Usui <usui@mxm.nes.nec.co.jp>
virNodeGetCPUTime: Implement virsh support Add nodecputime subcommand to virsh. This subcommand prints below output. [Linux] # build/tools/virsh nodecputime usage: 2.8% user : 0.8% system: 1.9% idle : 97.2% iowait: 0.0% [can get cpu utilization directly(ESX?)] # build/tools/virsh nodecputime usage: 2.8% Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp> --- tools/virsh.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 4 ++ 2 files changed, 100 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 19e3449..93288ba 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3388,6 +3388,101 @@ cmdNodeinfo(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } /* + * "nodecputime" command + */ +static const vshCmdInfo info_nodecputime[] = { + {"help", N_("Prints cpu utilizatoin of the node.")}, + {"desc", N_("Returns cpu utilizatoin of the node.(%)")}, + {NULL, NULL} +}; + +static int +cmdNodeCpuTime(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + int i, j; + int flag_utilization = FALSE; + virNodeCpuTime stats[VIR_NODE_CPU_TIME_NR]; + int nr_stats; + struct cpu_time { + unsigned long long user; + unsigned long long sys; + unsigned long long idle; + unsigned long long iowait; + unsigned long long util; + } cpu_time[2]; + double user_time, sys_time, idle_time, iowait_time, total_time; + double usage; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return FALSE; + + memset(cpu_time, 0, sizeof(struct cpu_time) * 2); + + for (i = 0; i < 2; i++) { + memset(stats, 0, sizeof(virNodeCpuTime) * VIR_NODE_CPU_TIME_NR); + nr_stats = virNodeGetCpuTime(ctl->conn, &stats[0], + VIR_NODE_CPU_TIME_NR, 0); + if (nr_stats < 0) { + vshError(ctl, "%s", _("failed to get cpu time of the node.")); + return FALSE; + } + + for (j = 0; j < nr_stats; j++) { + switch (stats[j].tag) { + case VIR_NODE_CPU_TIME_KERNEL: + cpu_time[i].sys = stats[j].val; + break; + case VIR_NODE_CPU_TIME_USER: + cpu_time[i].user = stats[j].val; + break; + case VIR_NODE_CPU_TIME_IDLE: + cpu_time[i].idle = stats[j].val; + break; + case VIR_NODE_CPU_TIME_IOWAIT: + cpu_time[i].iowait = stats[j].val; + break; + case VIR_NODE_CPU_TIME_UTILIZATION: + flag_utilization = TRUE; + cpu_time[i].util = stats[j].val; + break; + case VIR_NODE_CPU_TIME_NR: + default: + break; + } + } + + sleep(1); + } + + if (flag_utilization == TRUE) { + usage = cpu_time[0].util; + } else { + user_time = cpu_time[1].user - cpu_time[0].user; + sys_time = cpu_time[1].sys - cpu_time[0].sys; + idle_time = cpu_time[1].idle - cpu_time[0].idle; + iowait_time = cpu_time[1].iowait - cpu_time[0].iowait; + total_time = user_time + sys_time + idle_time + iowait_time; + + usage = (user_time + sys_time) / total_time * 100; + } + + vshPrint(ctl, "%-15s %5.1lf%%\n", _("usage:"), usage); + + if (flag_utilization != TRUE) { + vshPrint(ctl, "%-15s %5.1lf%%\n", + _(" user :"), user_time / total_time * 100); + vshPrint(ctl, "%-15s %5.1lf%%\n", + _(" system:"), sys_time / total_time * 100); + vshPrint(ctl, "%-15s %5.1lf%%\n", + _(" idle :"), idle_time / total_time * 100); + vshPrint(ctl, "%-15s %5.1lf%%\n", + _(" iowait:"), iowait_time / total_time * 100); + } + + return TRUE; +} + +/* * "capabilities" command */ static const vshCmdInfo info_capabilities[] = { @@ -10852,6 +10947,7 @@ static const vshCmdDef hostAndHypervisorCmds[] = { {"freecell", cmdFreecell, opts_freecell, info_freecell}, {"hostname", cmdHostname, NULL, info_hostname}, {"nodeinfo", cmdNodeinfo, NULL, info_nodeinfo}, + {"nodecputime", cmdNodeCpuTime, NULL, info_nodecputime}, {"qemu-monitor-command", cmdQemuMonitorCommand, opts_qemu_monitor_command, info_qemu_monitor_command}, {"sysinfo", cmdSysinfo, NULL, info_sysinfo}, {"uri", cmdURI, NULL, info_uri}, diff --git a/tools/virsh.pod b/tools/virsh.pod index f4bd294..d3e7eab 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -237,6 +237,10 @@ Print the XML representation of the hypervisor sysinfo, if available. Returns basic information about the node, like number and type of CPU, and size of the physical memory. +=item B<nodecputime> + +Returns cpu utilization of the node. + =item B<capabilities> Print an XML document describing the capabilities of the hypervisor -- 1.7.1 -- Minoru Usui <usui@mxm.nes.nec.co.jp>
On Fri, Apr 08, 2011 at 08:36:52PM +0900, Minoru Usui wrote:
virNodeGetCPUTime: Implement virsh support
Add nodecputime subcommand to virsh. This subcommand prints below output.
[Linux] # build/tools/virsh nodecputime usage: 2.8% user : 0.8% system: 1.9% idle : 97.2% iowait: 0.0%
[can get cpu utilization directly(ESX?)] # build/tools/virsh nodecputime usage: 2.8%
Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp> --- tools/virsh.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 4 ++ 2 files changed, 100 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 19e3449..93288ba 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3388,6 +3388,101 @@ cmdNodeinfo(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) }
/* + * "nodecputime" command + */ +static const vshCmdInfo info_nodecputime[] = { + {"help", N_("Prints cpu utilizatoin of the node.")}, + {"desc", N_("Returns cpu utilizatoin of the node.(%)")}, + {NULL, NULL} +}; + +static int +cmdNodeCpuTime(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + int i, j; + int flag_utilization = FALSE; + virNodeCpuTime stats[VIR_NODE_CPU_TIME_NR]; + int nr_stats; + struct cpu_time { + unsigned long long user; + unsigned long long sys; + unsigned long long idle; + unsigned long long iowait; + unsigned long long util; + } cpu_time[2]; + double user_time, sys_time, idle_time, iowait_time, total_time; + double usage; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return FALSE; + + memset(cpu_time, 0, sizeof(struct cpu_time) * 2); + + for (i = 0; i < 2; i++) { + memset(stats, 0, sizeof(virNodeCpuTime) * VIR_NODE_CPU_TIME_NR); + nr_stats = virNodeGetCpuTime(ctl->conn, &stats[0], + VIR_NODE_CPU_TIME_NR, 0); + if (nr_stats < 0) { + vshError(ctl, "%s", _("failed to get cpu time of the node.")); + return FALSE; + } + + for (j = 0; j < nr_stats; j++) { + switch (stats[j].tag) { + case VIR_NODE_CPU_TIME_KERNEL: + cpu_time[i].sys = stats[j].val; + break; + case VIR_NODE_CPU_TIME_USER: + cpu_time[i].user = stats[j].val; + break; + case VIR_NODE_CPU_TIME_IDLE: + cpu_time[i].idle = stats[j].val; + break; + case VIR_NODE_CPU_TIME_IOWAIT: + cpu_time[i].iowait = stats[j].val; + break; + case VIR_NODE_CPU_TIME_UTILIZATION: + flag_utilization = TRUE; + cpu_time[i].util = stats[j].val; + break; + case VIR_NODE_CPU_TIME_NR: + default: + break; + } + } + + sleep(1); + }
I'm not sure about this bit. In other places in virsh, we just report the absolute CPU time value, and don't try to calculate the deltas. There is a virt-top program that might like to use this data to provide a continuous display of utilization. Alternatively, we could add an arg for this to request that it calculate delta over 'n' seconds. 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 :|
On Tue, 19 Apr 2011 12:01:34 +0100 "Daniel P. Berrange" <berrange@redhat.com> wrote:
On Fri, Apr 08, 2011 at 08:36:52PM +0900, Minoru Usui wrote:
virNodeGetCPUTime: Implement virsh support
Add nodecputime subcommand to virsh. This subcommand prints below output.
[Linux] # build/tools/virsh nodecputime usage: 2.8% user : 0.8% system: 1.9% idle : 97.2% iowait: 0.0%
[can get cpu utilization directly(ESX?)] # build/tools/virsh nodecputime usage: 2.8%
Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp> --- tools/virsh.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 4 ++ 2 files changed, 100 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 19e3449..93288ba 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3388,6 +3388,101 @@ cmdNodeinfo(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) }
/* + * "nodecputime" command + */ +static const vshCmdInfo info_nodecputime[] = { + {"help", N_("Prints cpu utilizatoin of the node.")}, + {"desc", N_("Returns cpu utilizatoin of the node.(%)")}, + {NULL, NULL} +}; + +static int +cmdNodeCpuTime(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + int i, j; + int flag_utilization = FALSE; + virNodeCpuTime stats[VIR_NODE_CPU_TIME_NR]; + int nr_stats; + struct cpu_time { + unsigned long long user; + unsigned long long sys; + unsigned long long idle; + unsigned long long iowait; + unsigned long long util; + } cpu_time[2]; + double user_time, sys_time, idle_time, iowait_time, total_time; + double usage; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return FALSE; + + memset(cpu_time, 0, sizeof(struct cpu_time) * 2); + + for (i = 0; i < 2; i++) { + memset(stats, 0, sizeof(virNodeCpuTime) * VIR_NODE_CPU_TIME_NR); + nr_stats = virNodeGetCpuTime(ctl->conn, &stats[0], + VIR_NODE_CPU_TIME_NR, 0); + if (nr_stats < 0) { + vshError(ctl, "%s", _("failed to get cpu time of the node.")); + return FALSE; + } + + for (j = 0; j < nr_stats; j++) { + switch (stats[j].tag) { + case VIR_NODE_CPU_TIME_KERNEL: + cpu_time[i].sys = stats[j].val; + break; + case VIR_NODE_CPU_TIME_USER: + cpu_time[i].user = stats[j].val; + break; + case VIR_NODE_CPU_TIME_IDLE: + cpu_time[i].idle = stats[j].val; + break; + case VIR_NODE_CPU_TIME_IOWAIT: + cpu_time[i].iowait = stats[j].val; + break; + case VIR_NODE_CPU_TIME_UTILIZATION: + flag_utilization = TRUE; + cpu_time[i].util = stats[j].val; + break; + case VIR_NODE_CPU_TIME_NR: + default: + break; + } + } + + sleep(1); + }
I'm not sure about this bit. In other places in virsh, we just report the absolute CPU time value, and don't try to calculate the deltas. There is a virt-top program that might like to use this data to provide a continuous display of utilization.
Matthias said, ESX cannot get absolute CPU time. It just get CPU percentage. http://www.mail-archive.com/libvir-list@redhat.com/msg35769.html And I think CPU percentage is very useful for user. The user don't care about absolute CPU time value. They want to know about CPU load of each host. So I wrote this specification.
Alternatively, we could add an arg for this to request that it calculate delta over 'n' seconds.
I agree. It's very userful. But ESX cannot change interval, perhaps.
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 :|
-- Minoru Usui <usui@mxm.nes.nec.co.jp>
virNodeGetCPUTime: Implement linux support Signed-off-by: Minoru Usui <usui@mxm.nes.nec.co.jp> --- src/libvirt_private.syms | 1 + src/nodeinfo.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++ src/nodeinfo.h | 5 ++- 3 files changed, 83 insertions(+), 1 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 65a86d3..b879454 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -651,6 +651,7 @@ virNodeDeviceObjUnlock; # nodeinfo.h nodeCapsInitNUMA; +nodeGetCpuTime; nodeGetCellsFreeMemory; nodeGetFreeMemory; nodeGetInfo; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 5d40aca..4dd1d00 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -57,12 +57,17 @@ #ifdef __linux__ # define CPUINFO_PATH "/proc/cpuinfo" # define CPU_SYS_PATH "/sys/devices/system/cpu" +# define PROCSTAT_PATH "/proc/stat" /* NB, this is not static as we need to call it from the testsuite */ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, virNodeInfoPtr nodeinfo, bool need_hyperthreads); +int linuxNodeCpuTime(FILE *procstat, + virNodeCpuTimePtr cpu_time, + unsigned int nr_stats); + /* Return the positive decimal contents of the given * CPU_SYS_PATH/cpu%u/FILE, or -1 on error. If MISSING_OK and the * file could not be found, return 1 instead of an error; this is @@ -322,6 +327,52 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, return 0; } +#define TICK_TO_NSEC (1000ull * 1000ull * 1000ull / sysconf(_SC_CLK_TCK)) + +int linuxNodeCpuTime(FILE *procstat, virNodeCpuTimePtr stats, + unsigned int nr_stats) +{ + char line[1024]; + unsigned long long usr, ni, sys, idle, iowait; + unsigned long long irq, softirq, steal, guest, guest_nice; + + if (nr_stats < 4) { + nodeReportError(VIR_ERR_INVALID_ARG, "%s", + _("Invalid parameter count")); + return -1; + } + + while (fgets(line, sizeof(line), procstat) != NULL) { + char *buf = line; + + if (STRPREFIX(buf, "cpu ")) { /* aka total logical CPU time */ + if (sscanf(buf, + "%*s %llu %llu %llu %llu %llu" // user ~ iowait + "%llu %llu %llu %llu %llu", // irq ~ guest_nice + &usr, &ni, &sys, &idle, &iowait, + &irq, &softirq, &steal, &guest, &guest_nice) < 4) { + continue; + } + + stats[0].tag = VIR_NODE_CPU_TIME_KERNEL; + stats[0].val = (sys + irq + softirq) * TICK_TO_NSEC; + + stats[1].tag = VIR_NODE_CPU_TIME_USER; + stats[1].val = (usr + ni) * TICK_TO_NSEC; + + stats[2].tag = VIR_NODE_CPU_TIME_IDLE; + stats[2].val = idle * TICK_TO_NSEC; + + stats[3].tag = VIR_NODE_CPU_TIME_IOWAIT; + stats[3].val = iowait * TICK_TO_NSEC; + + return 4; + } + } + + nodeReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no \'cpu \' line found")); + return -1; +} #endif int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { @@ -360,6 +411,33 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { #endif } +int nodeGetCpuTime(virConnectPtr conn ATTRIBUTE_UNUSED, + virNodeCpuTimePtr stats, + unsigned int nr_stats, + unsigned int flags ATTRIBUTE_UNUSED) +{ + +#ifdef __linux__ + { + int ret; + FILE *procstat = fopen(PROCSTAT_PATH, "r"); + if (!procstat) { + virReportSystemError(errno, + _("cannot open %s"), PROCSTAT_PATH); + return -1; + } + ret = linuxNodeCpuTime(procstat, stats, nr_stats); + VIR_FORCE_FCLOSE(procstat); + + return ret; + } +#else + nodeReportError(VIR_ERR_NO_SUPPORT, "%s", + _("node CPU time not implemented on this platform")); + return -1; +#endif +} + #if HAVE_NUMACTL # if LIBNUMA_API_VERSION <= 1 # define NUMA_MAX_N_CPUS 4096 diff --git a/src/nodeinfo.h b/src/nodeinfo.h index 88bac6c..b024758 100644 --- a/src/nodeinfo.h +++ b/src/nodeinfo.h @@ -30,7 +30,10 @@ int nodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo); int nodeCapsInitNUMA(virCapsPtr caps); - +int nodeGetCpuTime(virConnectPtr conn ATTRIBUTE_UNUSED, + virNodeCpuTimePtr stats, + unsigned int nr_stats, + unsigned int flags ATTRIBUTE_UNUSED); int nodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *freeMems, int startCell, -- 1.7.1 -- Minoru Usui <usui@mxm.nes.nec.co.jp>
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Minoru Usui