Re: [PATCH python 1/1] Fix type extracting from PyDict

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 02/07/2018 01:56 PM, Daniel P. Berrangé wrote:
On Wed, Feb 07, 2018 at 01:46:00PM +0300, Edgar Kaziakhmedov wrote:
Ping


On 01/31/2018 07:34 PM, Edgar Kaziakhmedov wrote:
PyInt_Check returns value whether or not an input object is the integer
type. The existing implementation of extracting leads to the wrong
type interpretation in the following code:

params = {libvirt.VIR_MIGRATE_PARAM_DISKS_PORT : 50123}
...
dom.migrate3(%option1, params, %option3)

where libvirt reports:

libvirt.libvirtError: invalid argument: invalid type 'ullong' for
parameter 'disks_port', expected 'int'

So, this patch fixes that bug and allows casting to the VIR_TYPED_PARAM_INT
type.

Signed-off-by: Edgar Kaziakhmedov <edgar.kaziakhmedov@xxxxxxxxxxxxx>
---
   libvirt-utils.c | 5 +----
   1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/libvirt-utils.c b/libvirt-utils.c
index f7b4478..3a00e9f 100644
--- a/libvirt-utils.c
+++ b/libvirt-utils.c
@@ -434,10 +434,7 @@ virPyDictToTypedParamOne(virTypedParameterPtr *params,
                   type = VIR_TYPED_PARAM_ULLONG;
   #if PY_MAJOR_VERSION < 3
           } else if (PyInt_Check(value)) {
-            if (PyInt_AS_LONG(value) < 0)
-                type = VIR_TYPED_PARAM_LLONG;
-            else
-                type = VIR_TYPED_PARAM_ULLONG;
+            type = VIR_TYPED_PARAM_INT;
   #endif
           } else if (PyFloat_Check(value)) {
               type = VIR_TYPED_PARAM_DOUBLE;
I very much doubt this is the right fix. This virPyDictToTypedParamOne
method is used by several callers and many different parameters, and
your change affects all of them. IOW, every typed parameter that is
currently sent as a LLONG gets turned into an INT now.
Why? Before PyInt_Check there is a PyLong_Check which prevents
case you described.
Before my changes it worked that whether it is an INT or LONG INT it gets turned
into LLONG/ULLONG which is wrong behaviour.
This is good
for the "disks_port" field but bad for the "bandwidth" field for
example, so you've just moved where the brokeness is AFAICT.

This method is simply broken by design IMHO. You cann't reliably identify
which libvirt integer type is required by introspecting the python type.
The Python glue code needs to explicitly say which type to use for each
named parameter, based on the documented requirements in the libvirt
header file. eg the python needs to take the same approach as done in
the Perl binding here:

https://libvirt.org/git/?p=libvirt-perl.git;a=blob;f=Virt.xs;h=f19880f800bf477b939b24376fef30ec34af5abf;hb=HEAD#l5161


Regards,
Daniel

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux