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

but the Python library does an extra left shift of 10 bits returning MiB instead:
# 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 <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); -- 2.11.0

On Wed, Dec 05, 2018 at 01:01:13PM +0100, Philipp Hahn wrote:
but the Python library does an extra left shift of 10 bits returning MiB instead:
# 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
Signed-off-by: Philipp Hahn <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);
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 -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Hello Daniel, Am 05.12.18 um 13:10 schrieb Daniel P. Berrangé:
On Wed, Dec 05, 2018 at 01:01:13PM +0100, Philipp Hahn wrote:
but the Python library does an extra left shift of 10 bits returning MiB instead: ...
# ./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.
Signed-off-by: Philipp Hahn <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);
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.
Okay. This is already documented in libvirt-override-api.xml:
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
participants (2)
-
Daniel P. Berrangé
-
Philipp Hahn