9:05 a.m.
Resending the otherwise unmodified V2 series, as the cover letter didn't
show up on the mailing list (at least not in the archive).
Sorry for the extra traffic, but I have no clue what went wrong.
V2 Changes:
Added python binding for virNodeGetCPUMapFlags.
Removed RFC stanza.
Rationale:
In order to use the APIs listed below it is necessary for a client
to know the maximum number of node CPUs which is passed via the
maplen argument.
virDomainGetEmulatorPinInfo
virDomainGetVcpuPinInfo
virDomainGetVcpus
virDomainPinEmulator
virDomainPinVcpu
virDomainPinVcpuFlags
The current approach uses virNodeGetInfo to determine the maximum CPU number.
This can lead to incorrect results if not all node CPUs are online.
The maximum CPU number should always be the number of CPUs present on the
host, regardless of their online/offline state.
The following example illustrates the issue:
Host has 3 logical CPUs, 1 socket, 3 cores, 1 thread.
Guest has 1 virtual CPU and is started while all 3 host CPUs are
online.
$ virsh vcpuinfo guest
VCPU: 0
CPU: 0
State: running
CPU time: 35.4s
CPU Affinity: yyy
$ echo 0 > /sys/devices/system/cpu/cpu1/online
$ virsh vcpuinfo guest
VCPU: 0
CPU: 0
State: running
CPU time: 35.5s
CPU Affinity: y-
The correct display for CPU affinity would have been y-y, as the guest
continues to use CPUs 0 and 2.
This is not a display problem only, because it is also not possible to
explicitly pin the virtual CPU to host CPUs 0 and 2, due to the
truncated CPU mask.
PROPOSAL:
To help solve the issue above I suggest a new public API:
int virNodeGetCPUMapFlags(virConnectPtr conn,
unsigned char **cpumap,
unsigned int *online,
unsigned int flags);
The function will return the number of CPUs present on the host
or -1 on failure;
If cpumap is non-NULL virNodeGetCPUMapFlags will allocate an array
containing a bit map representation of the online CPUs. It's
the callers responsibility to deallocate cpumap using free().
If online is non-NULL, the variable pointed to will contain
the number of online host node CPUs.
The variable flags has been added to support future extensions
and must be set to 0.
Clients can use virNodeGetCPUMapFlags to properly determine the maximum
number of node CPUs and their online/offline state.
Remarks:
This series implements the QEMU and test drivers.
This series doesn't introduce changes to the existing vcpu pinning,
which I would submit as a succeeding patch set.
Viktor Mihajlovski (9):
virNodeGetCPUMapFlags: Define public API.
virNodeGetCPUMapFlags: Define driver API.
virNodeGetCPUMapFlags: Implement public API.
virNodeGetCPUMapFlags: Implement wire protocol.
libvirt.h.in: Add new cpumap macro VIR_CPU_USED
virNodeGetCPUMapFlags: Implement virsh support.
virNodeGetCPUMapFlags: Implement support function in nodeinfo
virNodeGetCPUMapFlags: Implement driver support
virNodeGetCPUMapFlags: Add python binding
daemon/remote.c | 44 ++++++++++++++++++++++++++++++
include/libvirt/libvirt.h.in | 28 ++++++++++++++++---
python/generator.py | 1 +
python/libvirt-override-api.xml | 6 ++++
python/libvirt-override.c | 56 +++++++++++++++++++++++++++++++++++++++
src/driver.h | 7 +++++
src/libvirt.c | 56 +++++++++++++++++++++++++++++++++++++++
src/libvirt_private.syms | 1 +
src/libvirt_public.syms | 5 +++
src/nodeinfo.c | 49 ++++++++++++++++++++++++++++++++++
src/nodeinfo.h | 6 ++++
src/qemu/qemu_driver.c | 1 +
src/remote/remote_driver.c | 49 ++++++++++++++++++++++++++++++++++
src/remote/remote_protocol.x | 13 ++++++++-
src/remote_protocol-structs | 12 ++++++++
src/test/test_driver.c | 30 +++++++++++++++++++++
tools/virsh-host.c | 41 ++++++++++++++++++++++++++++
tools/virsh.pod | 5 +++
18 files changed, 405 insertions(+), 5 deletions(-)
9:05 a.m.
New subject: [libvirt] [PATCHv2 1/9] virNodeGetCPUMapFlags: Define public API.
Adding a new API to obtain information about the
host node's present, online and offline CPUs.
int virNodeGetCPUMapFlags(virConnectPtr conn,
unsigned char **cpumap,
unsigned int *online,
unsigned int flags);
The function will return the number of CPUs present on the host
or -1 on failure;
If cpumap is non-NULL virNodeGetCPUMapFlags will allocate an array
containing a bit map representation of the online CPUs. It's
the callers responsibility to deallocate cpumap using free().
If online is non-NULL, the variable pointed to will contain
the number of online host node CPUs.
The variable flags has been added to support future extensions
and must be set to 0.
Note: the name virNodeGetCPUMapFlags has been chosen to avoid
confusion with the nodeinfo function nodeGetCPUmap.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
include/libvirt/libvirt.h.in | 8 ++++++++
python/generator.py | 1 +
src/libvirt_public.syms | 5 +++++
3 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index a4e8ca9..6b72159 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4500,6 +4500,14 @@ int virNodeSetMemoryParameters(virConnectPtr conn,
int nparams,
unsigned int flags);
+/*
+ * node CPU map
+ */
+int virNodeGetCPUMapFlags(virConnectPtr conn,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags);
+
#ifdef __cplusplus
}
#endif
diff --git a/python/generator.py b/python/generator.py
index ced7e41..8591b8d 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -429,6 +429,7 @@ skip_impl = (
'virConnectRegisterCloseCallback',
'virNodeGetMemoryParameters',
'virNodeSetMemoryParameters',
+ 'virNodeGetCPUMapFlags',
)
qemu_skip_impl = (
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 2c924d5..0c8f4ff 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -569,4 +569,9 @@ LIBVIRT_0.10.2 {
virStoragePoolListAllVolumes;
} LIBVIRT_0.10.0;
+LIBVIRT_1.0.0 {
+ global:
+ virNodeGetCPUMapFlags;
+} LIBVIRT_0.10.2;
+
# .... define new API here using predicted next version number ....
--
1.7.0.4
1:06 p.m.
New subject: [libvirt] [PATCHv2 1/9] virNodeGetCPUMapFlags: Define public API.
On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
Adding a new API to obtain information about the
host node's present, online and offline CPUs.
int virNodeGetCPUMapFlags(virConnectPtr conn,
unsigned char **cpumap,
unsigned int *online,
TAB damage.
unsigned int flags);
No need for the Flags suffix in the API name. The suffix is only needed
when we are adding a new API to fix shortcomings in an older one that
forgot a flags argument.
The function will return the number of CPUs present on the host
or -1 on failure;
If cpumap is non-NULL virNodeGetCPUMapFlags will allocate an array
containing a bit map representation of the online CPUs. It's
the callers responsibility to deallocate cpumap using free().
If online is non-NULL, the variable pointed to will contain
the number of online host node CPUs.
The variable flags has been added to support future extensions
and must be set to 0.
Note: the name virNodeGetCPUMapFlags has been chosen to avoid
confusion with the nodeinfo function nodeGetCPUmap.
Not necessary - nodeGetCPUmap is an internal function, which we can
rename at will. Public API naming should not be held hostage to
internal names.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
include/libvirt/libvirt.h.in | 8 ++++++++
python/generator.py | 1 +
src/libvirt_public.syms | 5 +++++
3 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index a4e8ca9..6b72159 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4500,6 +4500,14 @@ int virNodeSetMemoryParameters(virConnectPtr conn,
int nparams,
unsigned int flags);
+/*
+ * node CPU map
+ */
+int virNodeGetCPUMapFlags(virConnectPtr conn,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags);
+
#ifdef __cplusplus
}
#endif
Too low in the file - you ended up declaring it in the section for
deprecated names. Not entirely your fault; virNodeSetMemoryParameters
was also declared in the wrong place, so I prepared a separate patch to
hoist that up first:
https://www.redhat.com/archives/libvir-list/2012-October/msg01360.html
diff --git a/python/generator.py b/python/generator.py
index ced7e41..8591b8d 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -429,6 +429,7 @@ skip_impl = (
'virConnectRegisterCloseCallback',
'virNodeGetMemoryParameters',
'virNodeSetMemoryParameters',
+ 'virNodeGetCPUMapFlags',
)
qemu_skip_impl = (
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 2c924d5..0c8f4ff 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -569,4 +569,9 @@ LIBVIRT_0.10.2 {
virStoragePoolListAllVolumes;
} LIBVIRT_0.10.0;
+LIBVIRT_1.0.0 {
+ global:
+ virNodeGetCPUMapFlags;
+} LIBVIRT_0.10.2;
+
# .... define new API here using predicted next version number ....
ACK to the idea, but the documentation should be present in
src/libvirt.c at the time the function is introduced. In addition to
touching up the commit message, I will probably squash in your later
patch to libvirt.c, as well as these changes (on top of my patch
mentioned above for reordering the header file):
diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in
index 2b33ba8..368e014 100644
--- i/include/libvirt/libvirt.h.in
+++ w/include/libvirt/libvirt.h.in
@@ -750,6 +750,14 @@ int virNodeSetMemoryParameters(virConnectPtr conn,
int nparams,
unsigned int flags);
+/*
+ * node CPU map
+ */
+int virNodeGetCPUMap(virConnectPtr conn,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags);
+
/* Management of scheduler parameters */
/**
@@ -4546,14 +4554,6 @@ typedef virMemoryParameter *virMemoryParameterPtr;
/* Add new interfaces to the appropriate sections earlier in this
* file; the end of the file is reserved for deprecated names. */
-/*
- * node CPU map
- */
-int virNodeGetCPUMapFlags(virConnectPtr conn,
- unsigned char **cpumap,
- unsigned int *online,
- unsigned int flags);
-
#ifdef __cplusplus
}
#endif
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:05 a.m.
New subject: [libvirt] [PATCHv2 2/9] virNodeGetCPUMapFlags: Define driver API.
Extend the driver structure by nodeGetCPUMapFlags entry in support of the
new API virNodeGetCPUMapFlags.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
src/driver.h | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/src/driver.h b/src/driver.h
index bdcaa01..1af9568 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -898,6 +898,12 @@ typedef int
int nparams,
unsigned int flags);
+typedef int
+ (*virDrvNodeGetCPUMapFlags)(virConnectPtr conn,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags);
+
/**
* _virDriver:
*
@@ -1087,6 +1093,7 @@ struct _virDriver {
virDrvDomainGetMetadata domainGetMetadata;
virDrvNodeGetMemoryParameters nodeGetMemoryParameters;
virDrvNodeSetMemoryParameters nodeSetMemoryParameters;
+ virDrvNodeGetCPUMapFlags nodeGetCPUMapFlags;
};
typedef int
--
1.7.0.4
1:12 p.m.
New subject: [libvirt] [PATCHv2 2/9] virNodeGetCPUMapFlags: Define driver API.
On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
Extend the driver structure by nodeGetCPUMapFlags entry in support of
the
new API virNodeGetCPUMapFlags.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
src/driver.h | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
This patch belongs squashed with 1/9.
diff --git a/src/driver.h b/src/driver.h
index bdcaa01..1af9568 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -898,6 +898,12 @@ typedef int
int nparams,
unsigned int flags);
+typedef int
+ (*virDrvNodeGetCPUMapFlags)(virConnectPtr conn,
Once again, I'm dropping 'Flags'. ACK with this squashed in:
diff --git i/src/driver.h w/src/driver.h
index 1af9568..7ba66ad 100644
--- i/src/driver.h
+++ w/src/driver.h
@@ -899,10 +899,10 @@ typedef int
unsigned int flags);
typedef int
- (*virDrvNodeGetCPUMapFlags)(virConnectPtr conn,
- unsigned char **cpumap,
- unsigned int *online,
- unsigned int flags);
+ (*virDrvNodeGetCPUMap)(virConnectPtr conn,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags);
/**
* _virDriver:
@@ -1093,7 +1093,7 @@ struct _virDriver {
virDrvDomainGetMetadata domainGetMetadata;
virDrvNodeGetMemoryParameters nodeGetMemoryParameters;
virDrvNodeSetMemoryParameters nodeSetMemoryParameters;
- virDrvNodeGetCPUMapFlags nodeGetCPUMapFlags;
+ virDrvNodeGetCPUMap nodeGetCPUMap;
};
typedef int
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:17 p.m.
New subject: [libvirt] [PATCHv2 2/9] virNodeGetCPUMapFlags: Define driver API.
On 10/23/2012 12:12 PM, Eric Blake wrote:
On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
> Extend the driver structure by nodeGetCPUMapFlags entry in support of the
> new API virNodeGetCPUMapFlags.
>
> Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
> ---
> src/driver.h | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
This patch belongs squashed with 1/9.
Also, this patch in isolation fails 'make check':
make[2]: Entering directory `/home/remote/eblake/libvirt/docs'
GEN hvsupport.html.in
driver NodeGetCPUMap does not have a public API at ./hvsupport.pl line
165, <FILE> line 1096.
All the more reason to squash things together.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:41 p.m.
New subject: [libvirt] [PATCHv2 2/9] virNodeGetCPUMapFlags: Define driver API.
On 10/23/2012 01:17 PM, Eric Blake wrote:
On 10/23/2012 12:12 PM, Eric Blake wrote:
> On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
>> Extend the driver structure by nodeGetCPUMapFlags entry in support of the
>> new API virNodeGetCPUMapFlags.
>>
>> Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
>> ---
>> src/driver.h | 7 +++++++
>> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> This patch belongs squashed with 1/9.
Also, this patch in isolation fails 'make check':
make[2]: Entering directory `/home/remote/eblake/libvirt/docs'
GEN hvsupport.html.in
driver NodeGetCPUMap does not have a public API at ./hvsupport.pl line
165, <FILE> line 1096.
All the more reason to squash things together.
Never mind; this one was a case of me failing to remove enough 'Flags'
in patch 1:
diff --git i/python/generator.py w/python/generator.py
index 8591b8d..c76ff2a 100755
--- i/python/generator.py
+++ w/python/generator.py
@@ -429,7 +429,7 @@ skip_impl = (
'virConnectRegisterCloseCallback',
'virNodeGetMemoryParameters',
'virNodeSetMemoryParameters',
- 'virNodeGetCPUMapFlags',
+ 'virNodeGetCPUMap',
)
qemu_skip_impl = (
diff --git i/src/libvirt_public.syms w/src/libvirt_public.syms
index 0c8f4ff..f494821 100644
--- i/src/libvirt_public.syms
+++ w/src/libvirt_public.syms
@@ -571,7 +571,7 @@ LIBVIRT_0.10.2 {
LIBVIRT_1.0.0 {
global:
- virNodeGetCPUMapFlags;
+ virNodeGetCPUMap;
} LIBVIRT_0.10.2;
# .... define new API here using predicted next version number ....
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:05 a.m.
New subject: [libvirt] [PATCHv2 3/9] virNodeGetCPUMapFlags: Implement public API.
Added implementation of virNodeGetCPUMapFlags to libvirt.c
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
src/libvirt.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 3c6d67d..25c37d3 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -20098,3 +20098,59 @@ error:
virDispatchError(domain->conn);
return NULL;
}
+
+/**
+ * virNodeGetCPUMapFlags:
+ * @conn: pointer to the hypervisor connection
+ * @cpumap: optional pointer to a bit map of real CPUs on the host node
+ * (in 8-bit bytes) (OUT)
+ * In case of success each bit set to 1 means that corresponding
+ * CPU is online.
+ * Bytes are stored in little-endian order: CPU0-7, 8-15...
+ * In each byte, lowest CPU number is least significant bit.
+ * The bit map is allocated by virNodeGetCPUMapFlags and needs
+ * to be released using free() by the caller.
+ * @online: optional number of online CPUs in cpumap (OUT)
+ * Contains the number of online CPUs if the call was successful.
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Get CPU map of host node CPUs.
+ *
+ * Returns number of CPUs present on the host node,
+ * or -1 if there was an error.
+ */
+int
+virNodeGetCPUMapFlags (virConnectPtr conn,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags)
+{
+ VIR_DEBUG("conn=%p, cpumap=%p, online=%p, flags=%x",
+ conn, cpumap, online, flags);
+
+ virResetLastError();
+
+ if (!VIR_IS_CONNECT (conn)) {
+ virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
+ virDispatchError(NULL);
+ return -1;
+ }
+
+ if (conn->driver->nodeGetCPUMapFlags) {
+ int ret = conn->driver->nodeGetCPUMapFlags (conn,
+ cpumap,
+ online,
+ flags);
+ if (ret < 0)
+ goto error;
+ VIR_DEBUG("conn=%p, cpumap=%p, online=%u, flags=%x, ret=%d",
+ conn, cpumap, online ? *online : 0 , flags, ret);
+ return ret;
+ }
+
+ virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+ virDispatchError(conn);
+ return -1;
+}
--
1.7.0.4
2:41 p.m.
New subject: [libvirt] [PATCHv2 3/9] virNodeGetCPUMapFlags: Implement public API.
On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
Added implementation of virNodeGetCPUMapFlags to libvirt.c
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
src/libvirt.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 56 insertions(+), 0 deletions(-)
Ah, and here we finally get the documentation for the new API; hence,
this should also be squashed in with 1/9.
diff --git a/src/libvirt.c b/src/libvirt.c
index 3c6d67d..25c37d3 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -20098,3 +20098,59 @@ error:
virDispatchError(domain->conn);
return NULL;
}
+
+/**
+ * virNodeGetCPUMapFlags:
Again, I'm dropping Flags.
+int
+virNodeGetCPUMapFlags (virConnectPtr conn,
Although it isn't (yet) enforced, our style uses no space before ( in
function definitions. Dan has a patch in the wings which would flag this.
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags)
+{
+ VIR_DEBUG("conn=%p, cpumap=%p, online=%p, flags=%x",
+ conn, cpumap, online, flags);
+
+ virResetLastError();
+
+ if (!VIR_IS_CONNECT (conn)) {
Again, no space before ( in macro calls.
+ virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
+ virDispatchError(NULL);
+ return -1;
+ }
+
+ if (conn->driver->nodeGetCPUMapFlags) {
+ int ret = conn->driver->nodeGetCPUMapFlags (conn,
And again.
+ cpumap,
+ online,
+ flags);
+ if (ret < 0)
+ goto error;
+ VIR_DEBUG("conn=%p, cpumap=%p, online=%u, flags=%x, ret=%d",
+ conn, cpumap, online ? *online : 0 , flags, ret);
None of the other functions have a debug point here.
+ return ret;
+ }
+
+ virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+ virDispatchError(conn);
+ return -1;
+}
ACK with this squashed in:
diff --git i/src/libvirt.c w/src/libvirt.c
index 2415be3..dfab6ae 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -20108,7 +20108,7 @@ error:
}
/**
- * virNodeGetCPUMapFlags:
+ * virNodeGetCPUMap:
* @conn: pointer to the hypervisor connection
* @cpumap: optional pointer to a bit map of real CPUs on the host node
* (in 8-bit bytes) (OUT)
@@ -20128,31 +20128,26 @@ error:
* or -1 if there was an error.
*/
int
-virNodeGetCPUMapFlags (virConnectPtr conn,
- unsigned char **cpumap,
- unsigned int *online,
- unsigned int flags)
+virNodeGetCPUMap(virConnectPtr conn,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags)
{
VIR_DEBUG("conn=%p, cpumap=%p, online=%p, flags=%x",
conn, cpumap, online, flags);
virResetLastError();
- if (!VIR_IS_CONNECT (conn)) {
+ if (!VIR_IS_CONNECT(conn)) {
virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
virDispatchError(NULL);
return -1;
}
- if (conn->driver->nodeGetCPUMapFlags) {
- int ret = conn->driver->nodeGetCPUMapFlags (conn,
- cpumap,
- online,
- flags);
+ if (conn->driver->nodeGetCPUMap) {
+ int ret = conn->driver->nodeGetCPUMap(conn, cpumap, online, flags);
if (ret < 0)
goto error;
- VIR_DEBUG("conn=%p, cpumap=%p, online=%u, flags=%x, ret=%d",
- conn, cpumap, online ? *online : 0 , flags, ret);
return ret;
}
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:05 a.m.
New subject: [libvirt] [PATCHv2 4/9] virNodeGetCPUMapFlags: Implement wire protocol.
- Defined the wire protocol format for virNodeGetCPUMapFlags and its
arguments
- Implemented remote method invocation (remoteNodeGetCPUMapFlags)
- Implemented method dispacher (remoteDispatchNodeGetCPUMapFlags)
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
daemon/remote.c | 44 +++++++++++++++++++++++++++++++++++++
src/remote/remote_driver.c | 49 ++++++++++++++++++++++++++++++++++++++++++
src/remote/remote_protocol.x | 13 ++++++++++-
src/remote_protocol-structs | 12 ++++++++++
4 files changed, 117 insertions(+), 1 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index e7fe128..36291d1 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -4544,6 +4544,50 @@ cleanup:
return rv;
}
+static int
+remoteDispatchNodeGetCPUMapFlags(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client ATTRIBUTE_UNUSED,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_node_get_cpu_map_flags_args *args,
+ remote_node_get_cpu_map_flags_ret *ret)
+{
+ unsigned char *cpumap;
+ unsigned int online;
+ unsigned int flags;
+ int cpunum;
+ int rv = -1;
+ struct daemonClientPrivate *priv =
+ virNetServerClientGetPrivateData(client);
+
+ if (!priv->conn) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
+ goto cleanup;
+ }
+
+ flags = args->flags;
+
+ cpunum = virNodeGetCPUMapFlags(priv->conn, &cpumap, &online, flags);
+ if (cpunum < 0)
+ goto cleanup;
+
+ /* 'serialize' return cpumap */
+ ret->cpumap.cpumap_len = VIR_CPU_MAPLEN(cpunum);
+ ret->cpumap.cpumap_val = (char *) cpumap;
+ cpumap = NULL;
+
+ ret->online = online;
+ ret->ret = cpunum;
+
+ rv = 0;
+
+cleanup:
+ if (rv < 0)
+ virNetMessageSaveError(rerr);
+ VIR_FREE(cpumap);
+ return rv;
+}
+
/*----- Helpers. -----*/
/* get_nonnull_domain and get_nonnull_network turn an on-wire
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index fc4c696..7c89477 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -5749,6 +5749,54 @@ done:
return rv;
}
+int
+remoteNodeGetCPUMapFlags(virConnectPtr conn,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags)
+{
+ int rv = -1;
+ remote_node_get_cpu_map_flags_args args;
+ remote_node_get_cpu_map_flags_ret ret;
+ struct private_data *priv = conn->privateData;
+
+ remoteDriverLock(priv);
+
+ /* IMPROVEME: send info about optional args */
+ args.flags = flags;
+
+ memset (&ret, 0, sizeof(ret));
+ if (call (conn, priv, 0, REMOTE_PROC_NODE_GET_CPU_MAP_FLAGS,
+ (xdrproc_t) xdr_remote_node_get_cpu_map_flags_args,
+ (char *) &args,
+ (xdrproc_t) xdr_remote_node_get_cpu_map_flags_ret,
+ (char *) &ret) == -1)
+ goto done;
+
+ if (ret.ret < 0)
+ goto cleanup;
+
+ if (cpumap) {
+ if (VIR_ALLOC_N(*cpumap, ret.cpumap.cpumap_len) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ memcpy(*cpumap, ret.cpumap.cpumap_val, ret.cpumap.cpumap_len);
+ }
+
+ if (online)
+ *online = ret.online;
+
+ rv = ret.ret;
+
+cleanup:
+ xdr_free ((xdrproc_t) xdr_remote_node_get_cpu_map_flags_ret,
+ (char *) &ret);
+done:
+ remoteDriverUnlock(priv);
+ return rv;
+}
+
static void
remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event)
{
@@ -6063,6 +6111,7 @@ static virDriver remote_driver = {
.domainGetHostname = remoteDomainGetHostname, /* 0.10.0 */
.nodeSetMemoryParameters = remoteNodeSetMemoryParameters, /* 0.10.2 */
.nodeGetMemoryParameters = remoteNodeGetMemoryParameters, /* 0.10.2 */
+ .nodeGetCPUMapFlags = remoteNodeGetCPUMapFlags, /* 1.0.0 */
};
static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index b0b530c..bc5e70f 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2666,6 +2666,16 @@ struct remote_node_get_memory_parameters_ret {
int nparams;
};
+struct remote_node_get_cpu_map_flags_args {
+ unsigned int flags;
+};
+
+struct remote_node_get_cpu_map_flags_ret {
+ opaque cpumap<REMOTE_CPUMAP_MAX>;
+ unsigned int online;
+ int ret;
+};
+
/*----- Protocol. -----*/
/* Define the program number, protocol version and procedure numbers here. */
@@ -3008,7 +3018,8 @@ enum remote_procedure {
REMOTE_PROC_NODE_GET_MEMORY_PARAMETERS = 289, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_BLOCK_COMMIT = 290, /* autogen autogen */
- REMOTE_PROC_NETWORK_UPDATE = 291 /* autogen autogen priority:high */
+ REMOTE_PROC_NETWORK_UPDATE = 291, /* autogen autogen priority:high */
+ REMOTE_PROC_NODE_GET_CPU_MAP_FLAGS = 292 /* skipgen skipgen */
/*
* Notice how the entries are grouped in sets of 10 ?
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 4d2627a..8f1e678 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -2123,6 +2123,17 @@ struct remote_node_get_memory_parameters_ret {
} params;
int nparams;
};
+struct remote_node_get_cpu_map_flags_args {
+ u_int flags;
+};
+struct remote_node_get_cpu_map_flags_ret {
+ struct {
+ u_int cpumap_len;
+ char * cpumap_val;
+ } cpumap;
+ u_int online;
+ int ret;
+};
enum remote_procedure {
REMOTE_PROC_OPEN = 1,
REMOTE_PROC_CLOSE = 2,
@@ -2415,4 +2426,5 @@ enum remote_procedure {
REMOTE_PROC_NODE_GET_MEMORY_PARAMETERS = 289,
REMOTE_PROC_DOMAIN_BLOCK_COMMIT = 290,
REMOTE_PROC_NETWORK_UPDATE = 291,
+ REMOTE_PROC_NODE_GET_CPU_MAP_FLAGS = 292,
};
--
1.7.0.4
4:48 p.m.
New subject: [libvirt] [PATCHv2 4/9] virNodeGetCPUMapFlags: Implement wire protocol.
On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
- Defined the wire protocol format for virNodeGetCPUMapFlags and its
arguments
- Implemented remote method invocation (remoteNodeGetCPUMapFlags)
- Implemented method dispacher (remoteDispatchNodeGetCPUMapFlags)
s/dispacher/dispatcher/
and of course, s/Flags//g :)
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
daemon/remote.c | 44 +++++++++++++++++++++++++++++++++++++
src/remote/remote_driver.c | 49 ++++++++++++++++++++++++++++++++++++++++++
src/remote/remote_protocol.x | 13 ++++++++++-
src/remote_protocol-structs | 12 ++++++++++
4 files changed, 117 insertions(+), 1 deletions(-)
I'm reviewing a bit out-of-order here:
diff --git a/src/remote/remote_protocol.x
b/src/remote/remote_protocol.x
index b0b530c..bc5e70f 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2666,6 +2666,16 @@ struct remote_node_get_memory_parameters_ret {
int nparams;
};
+struct remote_node_get_cpu_map_flags_args {
+ unsigned int flags;
+};
This is insufficient for the caller to know if the map and/or online
parameters were non-NULL. No need to make the remote side allocate
something in reply if the local side doesn't want them. 'online' is
cheap enough to always provide, but skipping out on 'cpumap' will make
the return more efficient. So we need an additional member in this
struct, stating whether cpumap started life NULL.
+
+struct remote_node_get_cpu_map_flags_ret {
+ opaque cpumap<REMOTE_CPUMAP_MAX>;
+ unsigned int online;
+ int ret;
+};
+
This looks reasonable, modulo s/_flags//.
/*----- Protocol. -----*/
/* Define the program number, protocol version and procedure numbers
here. */
@@ -3008,7 +3018,8 @@ enum remote_procedure {
REMOTE_PROC_NODE_GET_MEMORY_PARAMETERS = 289, /* skipgen skipgen */
Hmm - pre-existing, but I think node memory parameters should be
priority:high, since obtaining that information doesn't block. I'll
think about that a bit more, and maybe submit a patch.
REMOTE_PROC_DOMAIN_BLOCK_COMMIT = 290, /* autogen autogen */
- REMOTE_PROC_NETWORK_UPDATE = 291 /* autogen autogen priority:high */
+ REMOTE_PROC_NETWORK_UPDATE = 291, /* autogen autogen priority:high */
+ REMOTE_PROC_NODE_GET_CPU_MAP_FLAGS = 292 /* skipgen skipgen */
292 was already claimed. But I think you are right that we can't use
skipgen, due to special-case handling of cpumap allocation.
Also, this can be priority:high, since there's nothing we do in this API
that blocks.
diff --git a/daemon/remote.c b/daemon/remote.c
index e7fe128..36291d1 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -4544,6 +4544,50 @@ cleanup:
return rv;
}
+static int
+remoteDispatchNodeGetCPUMapFlags(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client ATTRIBUTE_UNUSED,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_node_get_cpu_map_flags_args *args,
+ remote_node_get_cpu_map_flags_ret *ret)
+{
+ unsigned char *cpumap;
Uninitialized...
+ unsigned int online;
+ unsigned int flags;
+ int cpunum;
+ int rv = -1;
+ struct daemonClientPrivate *priv =
+ virNetServerClientGetPrivateData(client);
+
+ if (!priv->conn) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
+ goto cleanup;
...and goto cleanup...
+ }
+
+ flags = args->flags;
+
+ cpunum = virNodeGetCPUMapFlags(priv->conn, &cpumap, &online, flags);
Here, we should pass NULL instead of &cpumap if the user on the client
side passed NULL.
+ if (cpunum < 0)
+ goto cleanup;
+
+ /* 'serialize' return cpumap */
+ ret->cpumap.cpumap_len = VIR_CPU_MAPLEN(cpunum);
+ ret->cpumap.cpumap_val = (char *) cpumap;
+ cpumap = NULL;
Again, if the user didn't ask for a map, then we have nothing to
transfer here.
+
+ ret->online = online;
+ ret->ret = cpunum;
+
+ rv = 0;
+
+cleanup:
+ if (rv < 0)
+ virNetMessageSaveError(rerr);
+ VIR_FREE(cpumap);
...equals boom.
+ return rv;
+}
+
/*----- Helpers. -----*/
/* get_nonnull_domain and get_nonnull_network turn an on-wire
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index fc4c696..7c89477 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -5749,6 +5749,54 @@ done:
return rv;
}
+int
Needs to be static.
+remoteNodeGetCPUMapFlags(virConnectPtr conn,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags)
+{
+ int rv = -1;
+ remote_node_get_cpu_map_flags_args args;
+ remote_node_get_cpu_map_flags_ret ret;
+ struct private_data *priv = conn->privateData;
+
+ remoteDriverLock(priv);
+
+ /* IMPROVEME: send info about optional args */
Aha - you were even thinking about sending information about optional
arguments!
+ args.flags = flags;
+
+ memset (&ret, 0, sizeof(ret));
+ if (call (conn, priv, 0, REMOTE_PROC_NODE_GET_CPU_MAP_FLAGS,
We aren't consistent about space after 'call' here, so not your fault on
this one.
+++ b/src/remote_protocol-structs
@@ -2123,6 +2123,17 @@ struct remote_node_get_memory_parameters_ret {
} params;
int nparams;
};
+struct remote_node_get_cpu_map_flags_args {
+ u_int flags;
+};
And this needs to match whatever I changed in the .x.
Here's what I'm squashing in:
diff --git i/daemon/remote.c w/daemon/remote.c
index 9482b86..7a9df60 100644
--- i/daemon/remote.c
+++ w/daemon/remote.c
@@ -4570,14 +4570,14 @@ cleanup:
}
static int
-remoteDispatchNodeGetCPUMapFlags(virNetServerPtr server ATTRIBUTE_UNUSED,
- virNetServerClientPtr client
ATTRIBUTE_UNUSED,
- virNetMessagePtr msg ATTRIBUTE_UNUSED,
- virNetMessageErrorPtr rerr,
- remote_node_get_cpu_map_flags_args *args,
- remote_node_get_cpu_map_flags_ret *ret)
+remoteDispatchNodeGetCPUMap(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client ATTRIBUTE_UNUSED,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_node_get_cpu_map_args *args,
+ remote_node_get_cpu_map_ret *ret)
{
- unsigned char *cpumap;
+ unsigned char *cpumap = NULL;
unsigned int online;
unsigned int flags;
int cpunum;
@@ -4592,14 +4592,17 @@ remoteDispatchNodeGetCPUMapFlags(virNetServerPtr
server ATTRIBUTE_UNUSED,
flags = args->flags;
- cpunum = virNodeGetCPUMapFlags(priv->conn, &cpumap, &online, flags);
+ cpunum = virNodeGetCPUMap(priv->conn, args->need_results ? &cpumap
: NULL,
+ &online, flags);
if (cpunum < 0)
goto cleanup;
/* 'serialize' return cpumap */
- ret->cpumap.cpumap_len = VIR_CPU_MAPLEN(cpunum);
- ret->cpumap.cpumap_val = (char *) cpumap;
- cpumap = NULL;
+ if (args->need_results) {
+ ret->cpumap.cpumap_len = VIR_CPU_MAPLEN(cpunum);
+ ret->cpumap.cpumap_val = (char *) cpumap;
+ cpumap = NULL;
+ }
ret->online = online;
ret->ret = cpunum;
diff --git i/src/remote/remote_driver.c w/src/remote/remote_driver.c
index 48affac..71218f0 100644
--- i/src/remote/remote_driver.c
+++ w/src/remote/remote_driver.c
@@ -5780,28 +5780,28 @@ done:
return rv;
}
-int
-remoteNodeGetCPUMapFlags(virConnectPtr conn,
- unsigned char **cpumap,
- unsigned int *online,
- unsigned int flags)
+static int
+remoteNodeGetCPUMap(virConnectPtr conn,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags)
{
int rv = -1;
- remote_node_get_cpu_map_flags_args args;
- remote_node_get_cpu_map_flags_ret ret;
+ remote_node_get_cpu_map_args args;
+ remote_node_get_cpu_map_ret ret;
struct private_data *priv = conn->privateData;
remoteDriverLock(priv);
- /* IMPROVEME: send info about optional args */
+ args.need_results = !!cpumap;
args.flags = flags;
memset (&ret, 0, sizeof(ret));
- if (call (conn, priv, 0, REMOTE_PROC_NODE_GET_CPU_MAP_FLAGS,
- (xdrproc_t) xdr_remote_node_get_cpu_map_flags_args,
- (char *) &args,
- (xdrproc_t) xdr_remote_node_get_cpu_map_flags_ret,
- (char *) &ret) == -1)
+ if (call(conn, priv, 0, REMOTE_PROC_NODE_GET_CPU_MAP,
+ (xdrproc_t) xdr_remote_node_get_cpu_map_args,
+ (char *) &args,
+ (xdrproc_t) xdr_remote_node_get_cpu_map_ret,
+ (char *) &ret) == -1)
goto done;
if (ret.ret < 0)
@@ -5821,8 +5821,7 @@ remoteNodeGetCPUMapFlags(virConnectPtr conn,
rv = ret.ret;
cleanup:
- xdr_free ((xdrproc_t) xdr_remote_node_get_cpu_map_flags_ret,
- (char *) &ret);
+ xdr_free((xdrproc_t) xdr_remote_node_get_cpu_map_ret, (char *) &ret);
done:
remoteDriverUnlock(priv);
return rv;
@@ -6142,7 +6141,7 @@ static virDriver remote_driver = {
.domainGetHostname = remoteDomainGetHostname, /* 0.10.0 */
.nodeSetMemoryParameters = remoteNodeSetMemoryParameters, /* 0.10.2 */
.nodeGetMemoryParameters = remoteNodeGetMemoryParameters, /* 0.10.2 */
- .nodeGetCPUMapFlags = remoteNodeGetCPUMapFlags, /* 1.0.0 */
+ .nodeGetCPUMap = remoteNodeGetCPUMap, /* 1.0.0 */
};
static virNetworkDriver network_driver = {
diff --git i/src/remote/remote_protocol.x w/src/remote/remote_protocol.x
index ebfaa6a..765ffcd 100644
--- i/src/remote/remote_protocol.x
+++ w/src/remote/remote_protocol.x
@@ -2670,11 +2670,12 @@ struct remote_node_get_memory_parameters_ret {
int nparams;
};
-struct remote_node_get_cpu_map_flags_args {
+struct remote_node_get_cpu_map_args {
+ int need_results;
unsigned int flags;
};
-struct remote_node_get_cpu_map_flags_ret {
+struct remote_node_get_cpu_map_ret {
opaque cpumap<REMOTE_CPUMAP_MAX>;
unsigned int online;
int ret;
@@ -3024,7 +3025,7 @@ enum remote_procedure {
REMOTE_PROC_NETWORK_UPDATE = 291, /* autogen autogen priority:high */
REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND_DISK = 292, /* autogen autogen */
- REMOTE_PROC_NODE_GET_CPU_MAP_FLAGS = 293 /* skipgen skipgen */
+ REMOTE_PROC_NODE_GET_CPU_MAP = 293 /* skipgen skipgen */
/*
* Notice how the entries are grouped in sets of 10 ?
diff --git i/src/remote_protocol-structs w/src/remote_protocol-structs
index 894fd1d..567864a 100644
--- i/src/remote_protocol-structs
+++ w/src/remote_protocol-structs
@@ -2126,10 +2126,11 @@ struct remote_node_get_memory_parameters_ret {
} params;
int nparams;
};
-struct remote_node_get_cpu_map_flags_args {
+struct remote_node_get_cpu_map_args {
+ int need_results;
u_int flags;
};
-struct remote_node_get_cpu_map_flags_ret {
+struct remote_node_get_cpu_map_ret {
struct {
u_int cpumap_len;
char * cpumap_val;
@@ -2430,5 +2431,5 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_BLOCK_COMMIT = 290,
REMOTE_PROC_NETWORK_UPDATE = 291,
REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND_DISK = 292,
- REMOTE_PROC_NODE_GET_CPU_MAP_FLAGS = 293,
+ REMOTE_PROC_NODE_GET_CPU_MAP = 293,
};
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:05 a.m.
New subject: [libvirt] [PATCHv2 5/9] libvirt.h.in: Add new cpumap macro VIR_CPU_USED
New macro VIR_CPU_USED added to facilitate the interpretation of
cpu maps.
Further, hardened the other cpumap macros against invocations
like VIR_CPU_USE(cpumap + 1, cpu)
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
include/libvirt/libvirt.h.in | 20 ++++++++++++++++----
1 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 6b72159..3e8e4f8 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1954,7 +1954,7 @@ int virDomainGetEmulatorPinInfo (virDomainPtr
domain,
* USE_CPU macro set the bit (CPU usable) of the related cpu in cpumap.
*/
-#define VIR_USE_CPU(cpumap,cpu) (cpumap[(cpu)/8] |= (1<<((cpu)%8)))
+#define VIR_USE_CPU(cpumap,cpu) ((cpumap)[(cpu)/8] |= (1<<((cpu)%8)))
/**
* VIR_UNUSE_CPU:
@@ -1965,7 +1965,19 @@ int virDomainGetEmulatorPinInfo (virDomainPtr
domain,
* USE_CPU macro reset the bit (CPU not usable) of the related cpu in cpumap.
*/
-#define VIR_UNUSE_CPU(cpumap,cpu) (cpumap[(cpu)/8] &= ~(1<<((cpu)%8)))
+#define VIR_UNUSE_CPU(cpumap,cpu) ((cpumap)[(cpu)/8] &= ~(1<<((cpu)%8)))
+
+/**
+ * VIR_CPU_USED:
+ * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN)
+ * @cpu: the physical CPU number
+ *
+ * This macro can be used in conjunction with virNodeGetCPUMapFlags() API.
+ * VIR_CPU_USED returns true if the bit of the related CPU is set.
+ *
+ */
+
+#define VIR_CPU_USED(cpumap,cpu) ((cpumap)[(cpu)/8] & (1<<((cpu)%8)))
/**
* VIR_CPU_MAPLEN:
@@ -1976,7 +1988,7 @@ int virDomainGetEmulatorPinInfo (virDomainPtr
domain,
* CPU map between a single virtual & all physical CPUs of a domain.
*/
-#define VIR_CPU_MAPLEN(cpu) (((cpu)+7)/8)
+#define VIR_CPU_MAPLEN(cpu) (((cpu)+7)/8)
int virDomainGetVcpus (virDomainPtr domain,
@@ -1998,7 +2010,7 @@ int virDomainGetVcpus (virDomainPtr
domain,
*/
#define VIR_CPU_USABLE(cpumaps,maplen,vcpu,cpu) \
- (cpumaps[((vcpu)*(maplen))+((cpu)/8)] & (1<<((cpu)%8)))
+ ((cpumaps)[((vcpu)*(maplen))+((cpu)/8)] & (1<<((cpu)%8)))
/**
* VIR_COPY_CPUMAP:
--
1.7.0.4
5:26 p.m.
New subject: [libvirt] [PATCHv2 5/9] libvirt.h.in: Add new cpumap macro VIR_CPU_USED
On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
New macro VIR_CPU_USED added to facilitate the interpretation of
cpu maps.
Further, hardened the other cpumap macros against invocations
like VIR_CPU_USE(cpumap + 1, cpu)
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
include/libvirt/libvirt.h.in | 20 ++++++++++++++++----
1 files changed, 16 insertions(+), 4 deletions(-)
Hmm - should we rewrite VIR_CPU_USABLE in terms of VIR_CPU_USED()?
You also missed hardening VIR_COPY_CPUMAP and VIR_GET_CPUMAP.
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 6b72159..3e8e4f8 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1954,7 +1954,7 @@ int virDomainGetEmulatorPinInfo (virDomainPtr
domain,
* USE_CPU macro set the bit (CPU usable) of the related cpu in cpumap.
While we are touching here, lets fix the poor grammar:
s/USE_CPU macro set/It sets/
*/
-#define VIR_USE_CPU(cpumap,cpu) (cpumap[(cpu)/8] |= (1<<((cpu)%8)))
+#define VIR_USE_CPU(cpumap,cpu) ((cpumap)[(cpu)/8] |= (1<<((cpu)%8)))
As long as we are touching this, we might as well use conventional
spacing (space after comma and around operators).
/**
* VIR_UNUSE_CPU:
@@ -1965,7 +1965,19 @@ int virDomainGetEmulatorPinInfo (virDomainPtr
domain,
* USE_CPU macro reset the bit (CPU not usable) of the related cpu in cpumap.
Again, bad grammar.
*/
-#define VIR_UNUSE_CPU(cpumap,cpu) (cpumap[(cpu)/8] &= ~(1<<((cpu)%8)))
+#define VIR_UNUSE_CPU(cpumap,cpu) ((cpumap)[(cpu)/8] &= ~(1<<((cpu)%8)))
And again, odd spacing.
+
+/**
+ * VIR_CPU_USED:
+ * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN)
+ * @cpu: the physical CPU number
+ *
+ * This macro can be used in conjunction with virNodeGetCPUMapFlags() API.
+ * VIR_CPU_USED returns true if the bit of the related CPU is set.
Must it return true (== 1), or should we document merely that it returns
non-zero...
+ *
+ */
+
+#define VIR_CPU_USED(cpumap,cpu) ((cpumap)[(cpu)/8] & (1<<((cpu)%8)))
...given that you aren't returning a bool here? But VIR_CPU_USABLE
didn't bother converting to bool, so this is just a docs issue.
/**
* VIR_CPU_MAPLEN:
@@ -1976,7 +1988,7 @@ int virDomainGetEmulatorPinInfo (virDomainPtr
domain,
* CPU map between a single virtual & all physical CPUs of a domain.
*/
-#define VIR_CPU_MAPLEN(cpu) (((cpu)+7)/8)
+#define VIR_CPU_MAPLEN(cpu) (((cpu)+7)/8)
Why the whitespace change? But we might as well fix operator spacing.
int virDomainGetVcpus (virDomainPtr domain,
@@ -1998,7 +2010,7 @@ int virDomainGetVcpus (virDomainPtr
domain,
*/
#define VIR_CPU_USABLE(cpumaps,maplen,vcpu,cpu) \
- (cpumaps[((vcpu)*(maplen))+((cpu)/8)] & (1<<((cpu)%8)))
+ ((cpumaps)[((vcpu)*(maplen))+((cpu)/8)] & (1<<((cpu)%8)))
/**
* VIR_COPY_CPUMAP:
Here's what I'm squashing:
diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in
index adf5afb..428db39 100644
--- i/include/libvirt/libvirt.h.in
+++ w/include/libvirt/libvirt.h.in
@@ -2045,10 +2045,10 @@ int
virDomainGetEmulatorPinInfo (virDomainPtr domain,
* @cpu: the physical CPU number
*
* This macro is to be used in conjunction with virDomainPinVcpu() API.
- * USE_CPU macro set the bit (CPU usable) of the related cpu in cpumap.
+ * It sets the bit (CPU usable) of the related cpu in cpumap.
*/
-#define VIR_USE_CPU(cpumap,cpu) ((cpumap)[(cpu)/8] |= (1<<((cpu)%8)))
+#define VIR_USE_CPU(cpumap, cpu) ((cpumap)[(cpu) / 8] |= (1 << ((cpu) %
8)))
/**
* VIR_UNUSE_CPU:
@@ -2056,10 +2056,10 @@ int
virDomainGetEmulatorPinInfo (virDomainPtr domain,
* @cpu: the physical CPU number
*
* This macro is to be used in conjunction with virDomainPinVcpu() API.
- * USE_CPU macro reset the bit (CPU not usable) of the related cpu in
cpumap.
+ * It resets the bit (CPU not usable) of the related cpu in cpumap.
*/
-#define VIR_UNUSE_CPU(cpumap,cpu) ((cpumap)[(cpu)/8] &= ~(1<<((cpu)%8)))
+#define VIR_UNUSE_CPU(cpumap, cpu) ((cpumap)[(cpu) / 8] &= ~(1 <<
((cpu) % 8)))
/**
* VIR_CPU_USED:
@@ -2067,11 +2067,10 @@ int
virDomainGetEmulatorPinInfo (virDomainPtr domain,
* @cpu: the physical CPU number
*
* This macro can be used in conjunction with virNodeGetCPUMapFlags() API.
- * VIR_CPU_USED returns true if the bit of the related CPU is set.
- *
+ * It returns non-zero if the bit of the related CPU is set.
*/
-#define VIR_CPU_USED(cpumap,cpu) ((cpumap)[(cpu)/8] & (1<<((cpu)%8)))
+#define VIR_CPU_USED(cpumap, cpu) ((cpumap)[(cpu) / 8] & (1 << ((cpu) %
8)))
/**
* VIR_CPU_MAPLEN:
@@ -2082,7 +2081,7 @@ int
virDomainGetEmulatorPinInfo (virDomainPtr domain,
* CPU map between a single virtual & all physical CPUs of a domain.
*/
-#define VIR_CPU_MAPLEN(cpu) (((cpu)+7)/8)
+#define VIR_CPU_MAPLEN(cpu) (((cpu) + 7) / 8)
int virDomainGetVcpus (virDomainPtr domain,
@@ -2099,12 +2098,12 @@ int virDomainGetVcpus
(virDomainPtr domain,
* @cpu: the physical CPU number
*
* This macro is to be used in conjunction with virDomainGetVcpus() API.
- * VIR_CPU_USABLE macro returns a non zero value (true) if the cpu
+ * VIR_CPU_USABLE macro returns a non-zero value (true) if the cpu
* is usable by the vcpu, and 0 otherwise.
*/
-#define VIR_CPU_USABLE(cpumaps,maplen,vcpu,cpu) \
- ((cpumaps)[((vcpu)*(maplen))+((cpu)/8)] & (1<<((cpu)%8)))
+#define VIR_CPU_USABLE(cpumaps, maplen, vcpu, cpu) \
+ VIR_CPU_USED(VIR_GET_CPUMAP(cpumaps, maplen, vcpu), cpu)
/**
* VIR_COPY_CPUMAP:
@@ -2116,12 +2115,12 @@ int virDomainGetVcpus
(virDomainPtr domain,
* (ie: malloc(maplen))
*
* This macro is to be used in conjunction with virDomainGetVcpus() and
- * virDomainPinVcpu() APIs. VIR_COPY_CPUMAP macro extract the cpumap of
- * the specified vcpu from cpumaps array and copy it into cpumap to be used
+ * virDomainPinVcpu() APIs. VIR_COPY_CPUMAP macro extracts the cpumap of
+ * the specified vcpu from cpumaps array and copies it into cpumap to
be used
* later by virDomainPinVcpu() API.
*/
-#define VIR_COPY_CPUMAP(cpumaps,maplen,vcpu,cpumap) \
- memcpy(cpumap, &(cpumaps[(vcpu)*(maplen)]), (maplen))
+#define VIR_COPY_CPUMAP(cpumaps, maplen, vcpu, cpumap) \
+ memcpy(cpumap, VIR_GET_CPUMAP(cpumaps, maplen, vcpu), maplen)
/**
@@ -2134,7 +2133,7 @@ int virDomainGetVcpus
(virDomainPtr domain,
* virDomainPinVcpu() APIs. VIR_GET_CPUMAP macro returns a pointer to the
* cpumap of the specified vcpu from cpumaps array.
*/
-#define VIR_GET_CPUMAP(cpumaps,maplen,vcpu) &(cpumaps[(vcpu)*(maplen)])
+#define VIR_GET_CPUMAP(cpumaps, maplen, vcpu) (&((cpumaps)[(vcpu) *
(maplen)]))
typedef enum {
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:05 a.m.
New subject: [libvirt] [PATCHv2 6/9] virNodeGetCPUMapFlags: Implement virsh support.
- Added a new host command nodecpumap
- Added documentation
Example:
$ virsh nodecpumap
CPUs present: 8
CPUs online: 3
CPU map: 10101000
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
tools/virsh-host.c | 41 +++++++++++++++++++++++++++++++++++++++++
tools/virsh.pod | 5 +++++
2 files changed, 46 insertions(+), 0 deletions(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 5cf192d..e4f9327 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -271,6 +271,46 @@ cmdNodeinfo(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
}
/*
+ * "nodecpumap" command
+ */
+static const vshCmdInfo info_node_cpumap[] = {
+ {"help", N_("node cpu map")},
+ {"desc", N_("Displays the node's total number of CPUs, the number
of"
+ " online CPUs and the list of online CPUs.")},
+ {NULL, NULL}
+};
+
+static bool
+cmdNodeCpuMap(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
+{
+ int cpu, cpunum;
+ unsigned char *cpumap = NULL;
+ unsigned int online;
+ bool ret = false;
+
+ cpunum = virNodeGetCPUMapFlags(ctl->conn, &cpumap, &online, 0);
+ if (cpunum < 0) {
+ vshError(ctl, "%s", _("Unable to get cpu map"));
+ goto cleanup;
+ }
+
+ vshPrint(ctl, "%-15s %d\n", _("CPUs present:"), cpunum);
+ vshPrint(ctl, "%-15s %d\n", _("CPUs online:"), online);
+
+ vshPrint(ctl, "%-15s ", _("CPU map:"));
+ for (cpu = 0; cpu < cpunum; cpu++) {
+ vshPrint(ctl, "%d", VIR_CPU_USED(cpumap, cpu) ? 1 : 0);
+ }
+ vshPrint(ctl, "\n");
+
+ ret = true;
+
+ cleanup:
+ VIR_FREE(cpumap);
+ return ret;
+}
+
+/*
* "nodecpustats" command
*/
static const vshCmdInfo info_nodecpustats[] = {
@@ -1026,6 +1066,7 @@ const vshCmdDef hostAndHypervisorCmds[] = {
{"hostname", cmdHostname, NULL, info_hostname, 0},
{"node-memory-tune", cmdNodeMemoryTune,
opts_node_memory_tune, info_node_memory_tune, 0},
+ {"nodecpumap", cmdNodeCpuMap, NULL, info_node_cpumap, 0},
{"nodecpustats", cmdNodeCpuStats, opts_node_cpustats, info_nodecpustats,
0},
{"nodeinfo", cmdNodeinfo, NULL, info_nodeinfo, 0},
{"nodememstats", cmdNodeMemStats, opts_node_memstats, info_nodememstats,
0},
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 2d90b7b..9a2c0e1 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -277,6 +277,11 @@ and size of the physical memory. The output corresponds to
virNodeInfo
structure. Specifically, the "CPU socket(s)" field means number of CPU
sockets per NUMA cell.
+=item B<nodecpumap>
+
+Displays the node's total number of CPUs, the number of online CPUs
+and the list of online CPUs.
+
=item B<nodecpustats> [I<cpu>] [I<--percent>]
Returns cpu stats of the node.
--
1.7.0.4
5:35 p.m.
New subject: [libvirt] [PATCHv2 6/9] virNodeGetCPUMapFlags: Implement virsh support.
On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
s/Flags// - but by now we know the drill :)
- Added a new host command nodecpumap
- Added documentation
Example:
$ virsh nodecpumap
CPUs present: 8
CPUs online: 3
CPU map: 10101000
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
tools/virsh-host.c | 41 +++++++++++++++++++++++++++++++++++++++++
tools/virsh.pod | 5 +++++
2 files changed, 46 insertions(+), 0 deletions(-)
+ vshPrint(ctl, "%-15s ", _("CPU map:"));
+ for (cpu = 0; cpu < cpunum; cpu++) {
+ vshPrint(ctl, "%d", VIR_CPU_USED(cpumap, cpu) ? 1 : 0);
Perhaps more efficient as %c, and matching the vcpuinfo command's output
('y' and '-' instead of 1/0). Might also be nice to put in some spacing
for legibility, but we didn't do that for vcpuinfo.
Thanks for remembering the docs!
Here's what I'm squashing in, plus the obvious update to the commit message:
diff --git i/tools/virsh-host.c w/tools/virsh-host.c
index 4ef4a57..2d814cd 100644
--- i/tools/virsh-host.c
+++ w/tools/virsh-host.c
@@ -288,7 +288,7 @@ cmdNodeCpuMap(vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
unsigned int online;
bool ret = false;
- cpunum = virNodeGetCPUMapFlags(ctl->conn, &cpumap, &online, 0);
+ cpunum = virNodeGetCPUMap(ctl->conn, &cpumap, &online, 0);
if (cpunum < 0) {
vshError(ctl, "%s", _("Unable to get cpu map"));
goto cleanup;
@@ -298,9 +298,8 @@ cmdNodeCpuMap(vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
vshPrint(ctl, "%-15s %d\n", _("CPUs online:"), online);
vshPrint(ctl, "%-15s ", _("CPU map:"));
- for (cpu = 0; cpu < cpunum; cpu++) {
- vshPrint(ctl, "%d", VIR_CPU_USED(cpumap, cpu) ? 1 : 0);
- }
+ for (cpu = 0; cpu < cpunum; cpu++)
+ vshPrint(ctl, "%c", VIR_CPU_USED(cpumap, cpu) ? 'y' :
'-');
vshPrint(ctl, "\n");
ret = true;
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:05 a.m.
New subject: [libvirt] [PATCHv2 7/9] virNodeGetCPUMapFlags: Implement support function in nodeinfo
Added an implemention of virNodeGetCPUMapFlags to nodeinfo.c,
(nodeGetCPUMapFlags) which can be used by all drivers for a Linux
hypervisor host.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
src/libvirt_private.syms | 1 +
src/nodeinfo.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
src/nodeinfo.h | 6 +++++
3 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fe31bbe..44d5927 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -900,6 +900,7 @@ virNodeDeviceObjUnlock;
# nodeinfo.h
nodeCapsInitNUMA;
nodeGetCPUmap;
+nodeGetCPUMapFlags;
nodeGetCPUStats;
nodeGetCellsFreeMemory;
nodeGetFreeMemory;
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index c0e60d8..84cf796 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -1203,6 +1203,55 @@ nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
#endif
}
+int nodeGetCPUMapFlags(virConnectPtr conn,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags)
+{
+ virBitmapPtr cpusPresent = NULL;
+ virBitmapPtr cpusOnline = NULL;
+ int maxpresent, maxonline, i;
+ int ret = -1;
+
+ virCheckFlags(0, 1);
+
+ if (!(cpusPresent = nodeGetCPUmap(conn, &maxpresent, "present"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to retrieve 'present' CPU map"));
+ goto cleanup;
+ }
+
+ if (!(cpusOnline = nodeGetCPUmap(conn, &maxonline, "online"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to retrieve 'online' CPU map"));
+ goto cleanup;
+ }
+
+ if (cpumap && VIR_ALLOC_N(*cpumap, VIR_CPU_MAPLEN(maxpresent)) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (online)
+ *online = 0;
+
+ i = -1;
+ while ((i=virBitmapNextSetBit(cpusOnline, i)) >= 0) {
+ if (online)
+ (*online)++;
+
+ if (cpumap)
+ VIR_USE_CPU(*cpumap,i);
+ }
+
+ ret = maxpresent + 1;
+
+cleanup:
+ virBitmapFree(cpusPresent);
+ virBitmapFree(cpusOnline);
+ return ret;
+}
+
#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 2eda846..e210e6b 100644
--- a/src/nodeinfo.h
+++ b/src/nodeinfo.h
@@ -59,4 +59,10 @@ int nodeSetMemoryParameters(virConnectPtr conn,
virTypedParameterPtr params,
int nparams,
unsigned int flags);
+
+int nodeGetCPUMapFlags(virConnectPtr conn,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags);
+
#endif /* __VIR_NODEINFO_H__*/
--
1.7.0.4
6:21 p.m.
New subject: [libvirt] [PATCHv2 7/9] virNodeGetCPUMapFlags: Implement support function in nodeinfo
On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
Added an implemention of virNodeGetCPUMapFlags to nodeinfo.c,
s/implemention/implementation/
s/Flags//g
(nodeGetCPUMapFlags) which can be used by all drivers for a Linux
hypervisor host.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
src/libvirt_private.syms | 1 +
src/nodeinfo.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
src/nodeinfo.h | 6 +++++
3 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fe31bbe..44d5927 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -900,6 +900,7 @@ virNodeDeviceObjUnlock;
# nodeinfo.h
nodeCapsInitNUMA;
nodeGetCPUmap;
+nodeGetCPUMapFlags;
Ah, I see what you are up against. Well, like I said in 1/9, we should
not hold the public API hostage to internal naming, so I'd prefer to
s/nodeGetCPUmap/nodeGetCPUBitmap for the internal function.
nodeGetCPUStats;
nodeGetCellsFreeMemory;
nodeGetFreeMemory;
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index c0e60d8..84cf796 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -1203,6 +1203,55 @@ nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
#endif
}
+int nodeGetCPUMapFlags(virConnectPtr conn,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags)
+{
+ virBitmapPtr cpusPresent = NULL;
+ virBitmapPtr cpusOnline = NULL;
+ int maxpresent, maxonline, i;
+ int ret = -1;
+
+ virCheckFlags(0, 1);
s/1/-1/ - you must fail for unknown flags, rather than treating it like
returning a single online CPU.
+
+ if (!(cpusPresent = nodeGetCPUmap(conn, &maxpresent, "present"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to retrieve 'present' CPU map"));
+ goto cleanup;
+ }
+
+ if (!(cpusOnline = nodeGetCPUmap(conn, &maxonline, "online"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to retrieve 'online' CPU map"));
+ goto cleanup;
+ }
You are throwing away the inner error message, which may have been more
informative (such as OOM or missing platform support).
Eww. Why should we have to collect two bitmaps? nodeGetCPUmap (which
I'm renaming to nodeGetCPUBitmap) is returning non-useful information -
the "present" map will always be contiguous, and the only interesting
data is in "online"; meanwhile, the maximum online cpu for "online"
is
not what we care about, so much as the maximum present.
For that matter, qemu_driver.c is using nodeGetCPUmap incorrectly - it
is trying to collect the list of online CPUs (which is variable), but
passes "present" instead of "online" (the list of present CPUs is not
variable, at least not on bare metal; I suppose it is variable in a
guest once vcpu hotplug works, but that's a completely different level
of complication). I argue that nodeGetCPUmap shouldn't need a 'name'
argument, but should just magically give us the max present and the
online bitmap every time.
I'm going to do some major refactoring of this area of code as a
preliminary, and then we can rebase this patch on top of my improvements.
+
+ if (cpumap && VIR_ALLOC_N(*cpumap, VIR_CPU_MAPLEN(maxpresent)) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (online)
+ *online = 0;
+
+ i = -1;
+ while ((i=virBitmapNextSetBit(cpusOnline, i)) >= 0) {
Space around operators. Besides, isn't virBitmapToData better than
rolling this loop by hand?
+ if (online)
+ (*online)++;
This argues that util/bitmap.h should have a function for returning
bitmap population.
+
+ if (cpumap)
+ VIR_USE_CPU(*cpumap,i);
Space after comma.
I'm going to commit the earlier patches in this series today (to hit the
freeze deadline, so that we guarantee that we are committed to the API
for 1.0.0), while leaving this patch and later for further work for
after the freeze when I finish my improvements.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:46 a.m.
New subject: [libvirt] [PATCHv2 7/9] virNodeGetCPUMapFlags: Implement support function in nodeinfo
On 10/24/2012 01:21 AM, Eric Blake wrote:
> +
> + if (!(cpusPresent = nodeGetCPUmap(conn, &maxpresent, "present")))
{
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Unable to retrieve 'present' CPU
map"));
> + goto cleanup;
> + }
> +
> + if (!(cpusOnline = nodeGetCPUmap(conn, &maxonline, "online"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Unable to retrieve 'online' CPU map"));
> + goto cleanup;
> + }
You are throwing away the inner error message, which may have been more
informative (such as OOM or missing platform support).
Good point, the messages are
not really useful.
Eww. Why should we have to collect two bitmaps? nodeGetCPUmap (which
I'm renaming to nodeGetCPUBitmap) is returning non-useful information -
the "present" map will always be contiguous, and the only interesting
data is in "online"; meanwhile, the maximum online cpu for "online"
is
not what we care about, so much as the maximum present.
For that matter, qemu_driver.c is using nodeGetCPUmap incorrectly - it
is trying to collect the list of online CPUs (which is variable), but
passes "present" instead of "online" (the list of present CPUs is
not
variable, at least not on bare metal; I suppose it is variable in a
guest once vcpu hotplug works, but that's a completely different level
of complication). I argue that nodeGetCPUmap shouldn't need a 'name'
argument, but should just magically give us the max present and the
online bitmap every time.
Right. I was just (ab)using what was currently there. Not
sure whether
someone would be interested in an offline bitmap though. So maybe one
would want to have both flavors, online and offline and use a boolean
or enum to select the bitmap type.
I'm going to do some major refactoring of this area of code as a
preliminary, and then we can rebase this patch on top of my improvements.
Great, thanks!
> +
> + if (cpumap && VIR_ALLOC_N(*cpumap, VIR_CPU_MAPLEN(maxpresent)) < 0)
{
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + if (online)
> + *online = 0;
> +
> + i = -1;
> + while ((i=virBitmapNextSetBit(cpusOnline, i)) >= 0) {
Space around operators. Besides, isn't virBitmapToData better than
rolling this loop by hand?
Right, but either virBitmapToData should return the number of bits set or
a function as you suggest below is needed.
> + if (online)
> + (*online)++;
This argues that util/bitmap.h should have a function for returning
bitmap population.
I'm going to commit the earlier patches in this series today (to hit the
freeze deadline, so that we guarantee that we are committed to the API
for 1.0.0), while leaving this patch and later for further work for
after the freeze when I finish my improvements.
Many thanks for reworking the preceding patches!
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
9:05 a.m.
New subject: [libvirt] [PATCHv2 8/9] virNodeGetCPUMapFlags: Implement driver support
Driver support added for:
- test, pretending 8 host CPUS, 3 being online
- qemu, using nodeGetCPUMapFlags
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
src/qemu/qemu_driver.c | 1 +
src/test/test_driver.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0787039..2c6364b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13928,6 +13928,7 @@ static virDriver qemuDriver = {
.domainGetCPUStats = qemuDomainGetCPUStats, /* 0.9.11 */
.nodeGetMemoryParameters = nodeGetMemoryParameters, /* 0.10.2 */
.nodeSetMemoryParameters = nodeSetMemoryParameters, /* 0.10.2 */
+ .nodeGetCPUMapFlags = nodeGetCPUMapFlags, /* 1.0.0 */
};
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index c9f9115..24f3b11 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5687,6 +5687,35 @@ static int testListAllDomains(virConnectPtr conn,
return ret;
}
+static int testNodeGetCPUMapFlags(virConnectPtr conn,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags)
+{
+ testConnPtr privconn = conn->privateData;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ testDriverLock(privconn);
+ if (cpumap) {
+ if (VIR_ALLOC_N(*cpumap, 1) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ *cpumap[0] = 0x15;
+ }
+
+ if (online)
+ *online = 3;
+
+ ret = 8;
+
+cleanup:
+ testDriverUnlock(privconn);
+ return ret;
+}
+
static virDriver testDriver = {
.no = VIR_DRV_TEST,
@@ -5756,6 +5785,7 @@ static virDriver testDriver = {
.domainEventRegisterAny = testDomainEventRegisterAny, /* 0.8.0 */
.domainEventDeregisterAny = testDomainEventDeregisterAny, /* 0.8.0 */
.isAlive = testIsAlive, /* 0.9.8 */
+ .nodeGetCPUMapFlags = testNodeGetCPUMapFlags, /* 1.0.0 */
};
static virNetworkDriver testNetworkDriver = {
--
1.7.0.4
6:31 p.m.
New subject: [libvirt] [PATCHv2 8/9] virNodeGetCPUMapFlags: Implement driver support
On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
Driver support added for:
- test, pretending 8 host CPUS, 3 being online
- qemu, using nodeGetCPUMapFlags
We should implement this for as many drivers as possible; at least LXC
also comes to mind as being able to support this (I'm not quite as sure
about Xen).
Obviously, we can't push this until 7/9 is sorted out; but this is what
I'm thinking of squashing.
diff --git i/src/lxc/lxc_driver.c w/src/lxc/lxc_driver.c
index 87305db..2072f00 100644
--- i/src/lxc/lxc_driver.c
+++ w/src/lxc/lxc_driver.c
@@ -2736,6 +2736,7 @@ static virDriver lxcDriver = {
.nodeGetMemoryStats = nodeGetMemoryStats, /* 0.9.3 */
.nodeGetCellsFreeMemory = nodeGetCellsFreeMemory, /* 0.6.5 */
.nodeGetFreeMemory = nodeGetFreeMemory, /* 0.6.5 */
+ .nodeGetCPUMap = nodeGetCPUMap, /* 1.0.0 */
.domainEventRegister = lxcDomainEventRegister, /* 0.7.0 */
.domainEventDeregister = lxcDomainEventDeregister, /* 0.7.0 */
.isEncrypted = lxcIsEncrypted, /* 0.7.3 */
diff --git i/src/openvz/openvz_driver.c w/src/openvz/openvz_driver.c
index 2f51c1c..fb3c552 100644
--- i/src/openvz/openvz_driver.c
+++ w/src/openvz/openvz_driver.c
@@ -2127,6 +2127,7 @@ static virDriver openvzDriver = {
.nodeGetMemoryStats = nodeGetMemoryStats, /* 0.9.12 */
.nodeGetCellsFreeMemory = nodeGetCellsFreeMemory, /* 0.9.12 */
.nodeGetFreeMemory = nodeGetFreeMemory, /* 0.9.12 */
+ .nodeGetCPUMap = nodeGetCPUMap, /* 1.0.0 */
.getCapabilities = openvzGetCapabilities, /* 0.4.6 */
.listDomains = openvzListDomains, /* 0.3.1 */
.numOfDomains = openvzNumDomains, /* 0.3.1 */
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 37e334c..5a1c185 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -14196,7 +14196,7 @@ static virDriver qemuDriver = {
.domainGetCPUStats = qemuDomainGetCPUStats, /* 0.9.11 */
.nodeGetMemoryParameters = nodeGetMemoryParameters, /* 0.10.2 */
.nodeSetMemoryParameters = nodeSetMemoryParameters, /* 0.10.2 */
- .nodeGetCPUMapFlags = nodeGetCPUMapFlags, /* 1.0.0 */
+ .nodeGetCPUMap = nodeGetCPUMap, /* 1.0.0 */
};
diff --git i/src/test/test_driver.c w/src/test/test_driver.c
index 24f3b11..58c4e67 100644
--- i/src/test/test_driver.c
+++ w/src/test/test_driver.c
@@ -5687,10 +5687,11 @@ static int testListAllDomains(virConnectPtr conn,
return ret;
}
-static int testNodeGetCPUMapFlags(virConnectPtr conn,
- unsigned char **cpumap,
- unsigned int *online,
- unsigned int flags)
+static int
+testNodeGetCPUMap(virConnectPtr conn,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags)
{
testConnPtr privconn = conn->privateData;
int ret = -1;
@@ -5785,7 +5786,7 @@ static virDriver testDriver = {
.domainEventRegisterAny = testDomainEventRegisterAny, /* 0.8.0 */
.domainEventDeregisterAny = testDomainEventDeregisterAny, /* 0.8.0 */
.isAlive = testIsAlive, /* 0.9.8 */
- .nodeGetCPUMapFlags = testNodeGetCPUMapFlags, /* 1.0.0 */
+ .nodeGetCPUMap = testNodeGetCPUMap, /* 1.0.0 */
};
static virNetworkDriver testNetworkDriver = {
diff --git i/src/uml/uml_driver.c w/src/uml/uml_driver.c
index ba37eb7..2c6e820 100644
--- i/src/uml/uml_driver.c
+++ w/src/uml/uml_driver.c
@@ -2612,6 +2612,7 @@ static virDriver umlDriver = {
.nodeGetMemoryStats = nodeGetMemoryStats, /* 0.9.3 */
.nodeGetCellsFreeMemory = nodeGetCellsFreeMemory, /* 0.5.0 */
.nodeGetFreeMemory = nodeGetFreeMemory, /* 0.5.0 */
+ .nodeGetCPUMap = nodeGetCPUMap, /* 1.0.0 */
.domainEventRegister = umlDomainEventRegister, /* 0.9.4 */
.domainEventDeregister = umlDomainEventDeregister, /* 0.9.4 */
.isEncrypted = umlIsEncrypted, /* 0.7.3 */
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:58 a.m.
New subject: [libvirt] [PATCHv2 8/9] virNodeGetCPUMapFlags: Implement driver support
On 10/24/2012 01:31 AM, Eric Blake wrote:
On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
> Driver support added for:
> - test, pretending 8 host CPUS, 3 being online
> - qemu, using nodeGetCPUMapFlags
We should implement this for as many drivers as possible; at least LXC
also comes to mind as being able to support this (I'm not quite as sure
about Xen).
Right, in principle every Linux host (except for Xen) can support that.
BTW: I have another patch series prepared to solve the initial issue
(incorrect display of vcpu pinning) but will hold off until the
rework of 7/9 is done.
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
9:05 a.m.
New subject: [libvirt] [PATCHv2 9/9] virNodeGetCPUMapFlags: Add python binding
Added a method getCPUMapFlags to virConnect.
It can be used as follows:
import libvirt
import sys
import os
conn = libvirt.openReadOnly(None)
if conn == None:
print 'Failed to open connection to the hypervisor'
sys.exit(1)
try:
(cpus, cpumap, online) = conn.getCPUMapFlags(0)
except:
print 'Failed to extract the node cpu map information'
sys.exit(1)
print 'CPUs total %d, online %d' % (cpus, online)
print 'CPU map %s' % str(cpumap)
del conn
print "OK"
sys.exit(0)
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
python/libvirt-override-api.xml | 6 ++++
python/libvirt-override.c | 56 +++++++++++++++++++++++++++++++++++++++
2 files changed, 62 insertions(+), 0 deletions(-)
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
index b76fb4e..59ac190 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@ -542,5 +542,11 @@
<arg name='conn' type='virConnectPtr' info='pointer to the
hypervisor connection'/>
<arg name='flags' type='int' info='unused, always pass
0'/>
</function>
+ <function name='virNodeGetCPUMapFlags' file='python'>
+ <info>Get node CPU information</info>
+ <return type='str *' info='(cpunum, online, cpumap) on success, None
on error'/>
+ <arg name='conn' type='virConnectPtr' info='pointer to the
hypervisor connection'/>
+ <arg name='flags' type='int' info='unused, pass 0'/>
+ </function>
</symbols>
</api>
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index 81099b1..9d86964 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -6347,6 +6347,61 @@ cleanup:
return ret;
}
+static PyObject *
+libvirt_virNodeGetCPUMapFlags(PyObject *self ATTRIBUTE_UNUSED,
+ PyObject *args)
+{
+ virConnectPtr conn;
+ PyObject *pyobj_conn;
+ PyObject *ret = NULL;
+ PyObject *pycpumap = NULL;
+ PyObject *pyused = NULL;
+ int i_retval;
+ unsigned char *cpumap = NULL;
+ unsigned int online = 0;
+ unsigned int flags;
+ int i;
+
+ if (!PyArg_ParseTuple(args, (char *)"Oi:virNodeGetCPUMapFlags",
+ &pyobj_conn, &flags))
+ return NULL;
+ conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
+
+ LIBVIRT_BEGIN_ALLOW_THREADS;
+ i_retval = virNodeGetCPUMapFlags(conn, &cpumap, &online, flags);
+ LIBVIRT_END_ALLOW_THREADS;
+
+ if (i_retval < 0)
+ goto error;
+
+ if ((ret = PyTuple_New(3)) == NULL)
+ goto error;
+
+ if ((pycpumap = PyList_New(i_retval)) == NULL)
+ goto error;
+
+ for (i = 0; i < i_retval; i++) {
+ if ((pyused = PyBool_FromLong(VIR_CPU_USED(cpumap, i))) == NULL)
+ goto error;
+ if (PyList_SetItem(pycpumap, i, pyused) < 0)
+ goto error;
+ }
+
+ PyTuple_SetItem(ret, 0, PyLong_FromLong(i_retval));
+ PyTuple_SetItem(ret, 1, pycpumap);
+ PyTuple_SetItem(ret, 2, PyLong_FromLong(online));
+
+cleanup:
+ VIR_FREE(cpumap);
+ return ret;
+error:
+ Py_XDECREF(ret);
+ Py_XDECREF(pycpumap);
+ Py_XDECREF(pyused);
+ ret = VIR_PY_NONE;
+ goto cleanup;
+}
+
/************************************************************************
* *
@@ -6464,6 +6519,7 @@ static PyMethodDef libvirtMethods[] = {
{(char *) "virDomainGetDiskErrors", libvirt_virDomainGetDiskErrors,
METH_VARARGS, NULL},
{(char *) "virNodeGetMemoryParameters", libvirt_virNodeGetMemoryParameters,
METH_VARARGS, NULL},
{(char *) "virNodeSetMemoryParameters", libvirt_virNodeSetMemoryParameters,
METH_VARARGS, NULL},
+ {(char *) "virNodeGetCPUMapFlags", libvirt_virNodeGetCPUMapFlags,
METH_VARARGS, NULL},
{NULL, NULL, 0, NULL}
};
--
1.7.0.4
6:50 p.m.
New subject: [libvirt] [PATCHv2 9/9] virNodeGetCPUMapFlags: Add python binding
On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
Added a method getCPUMapFlags to virConnect.
s/Flags//g
It can be used as follows:
import libvirt
import sys
import os
conn = libvirt.openReadOnly(None)
if conn == None:
print 'Failed to open connection to the hypervisor'
sys.exit(1)
try:
(cpus, cpumap, online) = conn.getCPUMapFlags(0)
except:
print 'Failed to extract the node cpu map information'
sys.exit(1)
print 'CPUs total %d, online %d' % (cpus, online)
print 'CPU map %s' % str(cpumap)
del conn
print "OK"
sys.exit(0)
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
python/libvirt-override-api.xml | 6 ++++
python/libvirt-override.c | 56 +++++++++++++++++++++++++++++++++++++++
2 files changed, 62 insertions(+), 0 deletions(-)
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
index b76fb4e..59ac190 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@ -542,5 +542,11 @@
<arg name='conn' type='virConnectPtr' info='pointer to the
hypervisor connection'/>
<arg name='flags' type='int' info='unused, always pass
0'/>
</function>
+ <function name='virNodeGetCPUMapFlags' file='python'>
+ <info>Get node CPU information</info>
+ <return type='str *' info='(cpunum, online, cpumap) on success,
None on error'/>
+ <arg name='conn' type='virConnectPtr' info='pointer to the
hypervisor connection'/>
+ <arg name='flags' type='int' info='unused, pass
0'/>
+ </function>
</symbols>
</api>
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index 81099b1..9d86964 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -6347,6 +6347,61 @@ cleanup:
return ret;
}
+static PyObject *
+libvirt_virNodeGetCPUMapFlags(PyObject *self ATTRIBUTE_UNUSED,
+ PyObject *args)
+{
+ virConnectPtr conn;
+ PyObject *pyobj_conn;
+ PyObject *ret = NULL;
+ PyObject *pycpumap = NULL;
+ PyObject *pyused = NULL;
+ int i_retval;
+ unsigned char *cpumap = NULL;
+ unsigned int online = 0;
+ unsigned int flags;
+ int i;
+
+ if (!PyArg_ParseTuple(args, (char *)"Oi:virNodeGetCPUMapFlags",
+ &pyobj_conn, &flags))
+ return NULL;
+ conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
+
+ LIBVIRT_BEGIN_ALLOW_THREADS;
+ i_retval = virNodeGetCPUMapFlags(conn, &cpumap, &online, flags);
+ LIBVIRT_END_ALLOW_THREADS;
+
+ if (i_retval < 0)
+ goto error;
The error label returns Py_None, which is the clue for the next layer up
to parse the libvirt error - good in this case...
+
+ if ((ret = PyTuple_New(3)) == NULL)
+ goto error;
...but bad here - if PyTuple_New failed, then there is a python
exception, and we should be returning NULL instead of Py_None so that
the exception is properly propagated (and since there is no libvirt
error to parse).
+
+ if ((pycpumap = PyList_New(i_retval)) == NULL)
+ goto error;
+
+ for (i = 0; i < i_retval; i++) {
+ if ((pyused = PyBool_FromLong(VIR_CPU_USED(cpumap, i))) == NULL)
+ goto error;
+ if (PyList_SetItem(pycpumap, i, pyused) < 0)
+ goto error;
+ }
Again, all three of these goto error points should cause the function to
return NULL, not Py_None. Fix that by returning early on libvirt
failure (when there is nothing else to clean up), then setting ret to
NULL in the error label.
+
+ PyTuple_SetItem(ret, 0, PyLong_FromLong(i_retval));
+ PyTuple_SetItem(ret, 1, pycpumap);
+ PyTuple_SetItem(ret, 2, PyLong_FromLong(online));
Missing error checking. PyLong_FromLong() can return NULL on OOM
errors, but PyTuple_SetItem() must not be called with a NULL argument;
furthermore, PyTuple_SetItem() must be checked for failures.
There's still a lot of work that needs to be done here to properly check
for errors, so I'll hand this one back to you to clean up. But squash
this in to start your cleanup:
diff --git i/python/libvirt-override-api.xml
w/python/libvirt-override-api.xml
index 59ac190..e54701c 100644
--- i/python/libvirt-override-api.xml
+++ w/python/libvirt-override-api.xml
@@ -542,7 +542,7 @@
<arg name='conn' type='virConnectPtr' info='pointer to the
hypervisor connection'/>
<arg name='flags' type='int' info='unused, always pass
0'/>
</function>
- <function name='virNodeGetCPUMapFlags' file='python'>
+ <function name='virNodeGetCPUMap' file='python'>
<info>Get node CPU information</info>
<return type='str *' info='(cpunum, online, cpumap) on success,
None on error'/>
<arg name='conn' type='virConnectPtr' info='pointer to the
hypervisor connection'/>
diff --git i/python/libvirt-override.c w/python/libvirt-override.c
index 7f90abf..79afc91 100644
--- i/python/libvirt-override.c
+++ w/python/libvirt-override.c
@@ -6398,8 +6398,8 @@ cleanup:
}
static PyObject *
-libvirt_virNodeGetCPUMapFlags(PyObject *self ATTRIBUTE_UNUSED,
- PyObject *args)
+libvirt_virNodeGetCPUMap(PyObject *self ATTRIBUTE_UNUSED,
+ PyObject *args)
{
virConnectPtr conn;
PyObject *pyobj_conn;
@@ -6412,17 +6412,17 @@ libvirt_virNodeGetCPUMapFlags(PyObject *self
ATTRIBUTE_UNUSED,
unsigned int flags;
int i;
- if (!PyArg_ParseTuple(args, (char *)"Oi:virNodeGetCPUMapFlags",
+ if (!PyArg_ParseTuple(args, (char *)"Oi:virNodeGetCPUMap",
&pyobj_conn, &flags))
return NULL;
conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
LIBVIRT_BEGIN_ALLOW_THREADS;
- i_retval = virNodeGetCPUMapFlags(conn, &cpumap, &online, flags);
+ i_retval = virNodeGetCPUMap(conn, &cpumap, &online, flags);
LIBVIRT_END_ALLOW_THREADS;
if (i_retval < 0)
- goto error;
+ return VIR_PY_NONE;
if ((ret = PyTuple_New(3)) == NULL)
goto error;
@@ -6448,7 +6448,7 @@ error:
Py_XDECREF(ret);
Py_XDECREF(pycpumap);
Py_XDECREF(pyused);
- ret = VIR_PY_NONE;
+ ret = NULL;
goto cleanup;
}
@@ -6569,7 +6569,7 @@ static PyMethodDef libvirtMethods[] = {
{(char *) "virDomainGetDiskErrors", libvirt_virDomainGetDiskErrors,
METH_VARARGS, NULL},
{(char *) "virNodeGetMemoryParameters",
libvirt_virNodeGetMemoryParameters, METH_VARARGS, NULL},
{(char *) "virNodeSetMemoryParameters",
libvirt_virNodeSetMemoryParameters, METH_VARARGS, NULL},
- {(char *) "virNodeGetCPUMapFlags", libvirt_virNodeGetCPUMapFlags,
METH_VARARGS, NULL},
+ {(char *) "virNodeGetCPUMap", libvirt_virNodeGetCPUMap,
METH_VARARGS, NULL},
{NULL, NULL, 0, NULL}
};
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11 a.m.
New subject: [libvirt] [PATCHv2 9/9] virNodeGetCPUMapFlags: Add python binding
On 10/24/2012 01:50 AM, Eric Blake wrote:
There's still a lot of work that needs to be done here to
properly check
for errors, so I'll hand this one back to you to clean up. But squash
this in to start your cleanup:
Will do, thanks for the feedback and corrections.
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
4413
days inactive
4421
days old
23 comments
2 participants
participants (2)
-
Eric Blake -
Viktor Mihajlovski