[libvirt] [PATCH 0/2] Python virNodeGetInfo mess
I'm proposing two contradictionary patches to fix the issue. Long story short, libvirt is mangling the memory size in pythong API binding where the size is reported in MiB instead of KiB: https://www.redhat.com/archives/libvirt-users/2013-September/msg00187.html Which approach do you like better? Michal Privoznik (2): python: Report nodeinfo's memory in KiB python: Document virNodeGetInfo bug python/libvirt-override-api.xml | 2 +- python/libvirt-override.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 1.8.1.5
The python binding to virNodeGetInfo API has this awful bug. The amount of RAM the node has is reported in MiB instead of KiB as we have documented in the struct virNodeInfo description. The problem is, after we obtain the nodeinfo the amount is shifted left ten times (divided by 1024). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- python/libvirt-override.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index e659bae..3069013 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -2843,7 +2843,7 @@ libvirt_virNodeGetInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { return VIR_PY_NONE; py_retval = PyList_New(8); PyList_SetItem(py_retval, 0, libvirt_constcharPtrWrap(&info.model[0])); - PyList_SetItem(py_retval, 1, libvirt_longWrap((long) info.memory >> 10)); + PyList_SetItem(py_retval, 1, libvirt_longWrap((long) info.memory)); PyList_SetItem(py_retval, 2, libvirt_intWrap((int) info.cpus)); PyList_SetItem(py_retval, 3, libvirt_intWrap((int) info.mhz)); PyList_SetItem(py_retval, 4, libvirt_intWrap((int) info.nodes)); -- 1.8.1.5
On Mon, Sep 30, 2013 at 11:30:11AM +0200, Michal Privoznik wrote:
The python binding to virNodeGetInfo API has this awful bug. The amount of RAM the node has is reported in MiB instead of KiB as we have documented in the struct virNodeInfo description. The problem is, after we obtain the nodeinfo the amount is shifted left ten times (divided by 1024).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- python/libvirt-override.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/python/libvirt-override.c b/python/libvirt-override.c index e659bae..3069013 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -2843,7 +2843,7 @@ libvirt_virNodeGetInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { return VIR_PY_NONE; py_retval = PyList_New(8); PyList_SetItem(py_retval, 0, libvirt_constcharPtrWrap(&info.model[0])); - PyList_SetItem(py_retval, 1, libvirt_longWrap((long) info.memory >> 10)); + PyList_SetItem(py_retval, 1, libvirt_longWrap((long) info.memory)); PyList_SetItem(py_retval, 2, libvirt_intWrap((int) info.cpus)); PyList_SetItem(py_retval, 3, libvirt_intWrap((int) info.mhz)); PyList_SetItem(py_retval, 4, libvirt_intWrap((int) info.nodes));
NACK, this is a clear backwards compat problem that will break *every* application using this API Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 30.09.2013 11:37, Daniel P. Berrange wrote:
On Mon, Sep 30, 2013 at 11:30:11AM +0200, Michal Privoznik wrote:
The python binding to virNodeGetInfo API has this awful bug. The amount of RAM the node has is reported in MiB instead of KiB as we have documented in the struct virNodeInfo description. The problem is, after we obtain the nodeinfo the amount is shifted left ten times (divided by 1024).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- python/libvirt-override.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/python/libvirt-override.c b/python/libvirt-override.c index e659bae..3069013 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -2843,7 +2843,7 @@ libvirt_virNodeGetInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { return VIR_PY_NONE; py_retval = PyList_New(8); PyList_SetItem(py_retval, 0, libvirt_constcharPtrWrap(&info.model[0])); - PyList_SetItem(py_retval, 1, libvirt_longWrap((long) info.memory >> 10)); + PyList_SetItem(py_retval, 1, libvirt_longWrap((long) info.memory)); PyList_SetItem(py_retval, 2, libvirt_intWrap((int) info.cpus)); PyList_SetItem(py_retval, 3, libvirt_intWrap((int) info.mhz)); PyList_SetItem(py_retval, 4, libvirt_intWrap((int) info.nodes));
NACK, this is a clear backwards compat problem that will break *every* application using this API
Yeah, I've posted the patch just for completeness-sake while in fact preferring the 2/2. Michal
The memory size in virNodeGetInfo python API binding is reported in MiB instead of KiB (like we have in C struct). However, there already might be applications out there relying on this inconsistence so we can't simply fix it. Document this sad fact as known bug. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- python/libvirt-override-api.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 1bceb05..337d0a0 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -100,7 +100,7 @@ <arg name='flags' type='unsigned int' info='flags, currently unused, pass 0.'/> </function> <function name='virNodeGetInfo' file='python'> - <info>Extract hardware information about the Node.</info> + <info>Extract hardware information about the Node. Note that the memory size is reported in MiB instead of KiB.</info> <return type='char *' info='the list of information or None in case of error'/> <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> </function> -- 1.8.1.5
On Mon, Sep 30, 2013 at 11:30:12AM +0200, Michal Privoznik wrote:
The memory size in virNodeGetInfo python API binding is reported in MiB instead of KiB (like we have in C struct). However, there already might be applications out there relying on this inconsistence so we can't simply fix it. Document this sad fact as known bug.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- python/libvirt-override-api.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 1bceb05..337d0a0 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -100,7 +100,7 @@ <arg name='flags' type='unsigned int' info='flags, currently unused, pass 0.'/> </function> <function name='virNodeGetInfo' file='python'> - <info>Extract hardware information about the Node.</info> + <info>Extract hardware information about the Node. Note that the memory size is reported in MiB instead of KiB.</info> <return type='char *' info='the list of information or None in case of error'/> <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> </function>
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 30.09.2013 11:38, Daniel P. Berrange wrote:
On Mon, Sep 30, 2013 at 11:30:12AM +0200, Michal Privoznik wrote:
The memory size in virNodeGetInfo python API binding is reported in MiB instead of KiB (like we have in C struct). However, there already might be applications out there relying on this inconsistence so we can't simply fix it. Document this sad fact as known bug.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- python/libvirt-override-api.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 1bceb05..337d0a0 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -100,7 +100,7 @@ <arg name='flags' type='unsigned int' info='flags, currently unused, pass 0.'/> </function> <function name='virNodeGetInfo' file='python'> - <info>Extract hardware information about the Node.</info> + <info>Extract hardware information about the Node. Note that the memory size is reported in MiB instead of KiB.</info> <return type='char *' info='the list of information or None in case of error'/> <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> </function>
ACK
Thanks, I've pushed this patch even though we're in freeze as it's just a documentation fix - so it won't break anything [Famous Last Words (TM)] Michal
participants (2)
-
Daniel P. Berrange -
Michal Privoznik