[libvirt] [PATCH libvirt-python] virNodeInfo.memory is in KiB

[Philipp Hahn]

but the Python library does an extra left shift of 10 bits returning MiB

# cat y.c
#include <stdlib.h>
#include <stdio.h>
#include <libvirt.h>
int main(void) {
virConnectPtr conn = virConnectOpen("qemu:///system");
virNodeInfo info;
int rv = virNodeGetInfo(conn, &info);
printf("%ld\n", info.memory);
return rv;
}
# gcc y.c -I/usr/include/libvirt -lvirt
# ./a.out
4041088
# python -c 'import libvirt;c=libvirt.open("qemu:///system");print(c.getInfo()[1])'
3946

Fixes: 197153c6
Signed-off-by: Philipp Hahn <***@univention.de>
---
libvirt-override.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt-override.c b/libvirt-override.c
index f7b2f6b..616fa1c 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -2740,7 +2740,7 @@ libvirt_virNodeGetInfo(PyObject *self ATTRIBUTE_UNUSED,
VIR_PY_LIST_SET_GOTO(py_retval, 0,
libvirt_constcharPtrWrap(&info.model[0]), error);
VIR_PY_LIST_SET_GOTO(py_retval, 1,
- libvirt_longWrap((long) info.memory >> 10), error);
+ libvirt_longWrap((long) info.memory), error);
VIR_PY_LIST_SET_GOTO(py_retval, 2, libvirt_intWrap((int) info.cpus), error);
VIR_PY_LIST_SET_GOTO(py_retval, 3, libvirt_intWrap((int) info.mhz), error);
VIR_PY_LIST_SET_GOTO(py_retval, 4, libvirt_intWrap((int) info.nodes), error);

[Daniel P. Berrangé]

Post by Philipp Hahn
but the Python library does an extra left shift of 10 bits returning MiB

# cat y.c
#include <stdlib.h>
#include <stdio.h>
#include <libvirt.h>
int main(void) {
virConnectPtr conn = virConnectOpen("qemu:///system");
virNodeInfo info;
int rv = virNodeGetInfo(conn, &info);
printf("%ld\n", info.memory);
return rv;
}
# gcc y.c -I/usr/include/libvirt -lvirt
# ./a.out
4041088
# python -c 'import libvirt;c=libvirt.open("qemu:///system");print(c.getInfo()[1])'
3946

Fixes: 197153c6

Not sure why you're quoting that commit has as it is unrelated.

This use of MB is from pretty much day 1 in 506fb7d8

Post by Philipp Hahn
---
libvirt-override.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libvirt-override.c b/libvirt-override.c
index f7b2f6b..616fa1c 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -2740,7 +2740,7 @@ libvirt_virNodeGetInfo(PyObject *self ATTRIBUTE_UNUSED,
VIR_PY_LIST_SET_GOTO(py_retval, 0,
libvirt_constcharPtrWrap(&info.model[0]), error);
VIR_PY_LIST_SET_GOTO(py_retval, 1,
- libvirt_longWrap((long) info.memory >> 10), error);
+ libvirt_longWrap((long) info.memory), error);

We can't change this as it would break every single existing user of this
API which have been written to expect this to be a MB value.

Regards,
Daniel

[Philipp Hahn]

Hello Daniel,

Post by Daniel P. Berrangé

Post by Philipp Hahn
but the Python library does an extra left shift of 10 bits returning MiB

...

Post by Daniel P. Berrangé

Post by Philipp Hahn

# ./a.out
4041088
# python -c 'import libvirt;c=libvirt.open("qemu:///system");print(c.getInfo()[1])'
3946

Fixes: 197153c6

Not sure why you're quoting that commit has as it is unrelated.
This use of MB is from pretty much day 1 in 506fb7d8

Yes, you're right, that was the wrong commit hash and 506fb7d8 is the
correct one.

Post by Daniel P. Berrangé

Post by Philipp Hahn
---
libvirt-override.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libvirt-override.c b/libvirt-override.c
index f7b2f6b..616fa1c 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -2740,7 +2740,7 @@ libvirt_virNodeGetInfo(PyObject *self ATTRIBUTE_UNUSED,
VIR_PY_LIST_SET_GOTO(py_retval, 0,
libvirt_constcharPtrWrap(&info.model[0]), error);
VIR_PY_LIST_SET_GOTO(py_retval, 1,
- libvirt_longWrap((long) info.memory >> 10), error);
+ libvirt_longWrap((long) info.memory), error);

We can't change this as it would break every single existing user of this
API which have been written to expect this to be a MB value.
102 <function name='virNodeGetInfo' file='python'>
103 <info>Extract hardware information about the Node. Note that the memory size is reported in MiB instead of KiB.</info>

I was only looking at the C-API documentation.
So my patch can go to >/dev/null.

Sorry for the noise.

Philipp