Re: [libvirt] [PATCHv2 1/6] virNodeGetCPUTime: Expose new API
Hi,
I'm sorry for late reply.
On Tue, 19 Apr 2011 11:48:55 +0100
Daniel P. Berrange berra...@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 u...@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 u...@mxm.nes.nec.co.jp
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/6] virNodeGetCPUTime: Expose new API
On Fri, Apr 08, 2011 at 08:33:12PM +0900, Minoru Usui wrote:
virNodeGetCPUTime: Expose new API
Signed-off-by: Minoru Usui u...@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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/6] virNodeGetCPUTime: Expose new API
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 u...@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/ :|
|:
Re: [libvirt] [PATCHv2 1/6] virNodeGetCPUTime: Expose new API
On Mon, 18 Apr 2011 12:30:45 +0100
Daniel P. Berrange berra...@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 u...@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
