[libvirt] [PATCH python 1/1] Fix type extracting from PyDict

[Edgar Kaziakhmedov]

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 at virtuozzo.com>
>>> ---
>>>    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