[libvirt] [PATCH python 1/1] Fix type extracting from PyDict
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@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; -- 2.11.0
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@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; -- 2.11.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list .
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@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. 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=f19880f800bf4... 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 :|
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@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=f19880f800bf4...
Regards, Daniel
On Wed, Feb 07, 2018 at 03:23:58PM +0300, Edgar Kaziakhmedov wrote:
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@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.
The "disks_port" value could be stored in a PyLong, or the bandwidth parameter could be stored in a "PyInt", depending on just how the mgmt app using libvirt has got hold of the data. The app shouldn't be expected to know about different python integer sized types to be able to use libvirt. 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 :|
On 02/07/2018 03:26 PM, Daniel P. Berrangé wrote:
On Wed, Feb 07, 2018 at 03:23:58PM +0300, Edgar Kaziakhmedov wrote:
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@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.
The "disks_port" value could be stored in a PyLong, or the bandwidth parameter could be stored in a "PyInt", depending on just how the mgmt app using libvirt has got hold of the data. The app shouldn't be expected to know about different python integer sized types to be able to use libvirt. Get your idea. We need to fill params array with predefined type for each option and then while converting cast python object to the specified type.
Regards, Daniel
On 02/07/2018 03:26 PM, Daniel P. Berrangé wrote:
On Wed, Feb 07, 2018 at 03:23:58PM +0300, Edgar Kaziakhmedov wrote:
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@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.
The "disks_port" value could be stored in a PyLong, or the bandwidth parameter could be stored in a "PyInt", depending on just how the mgmt app using libvirt has got hold of the data. The app shouldn't be expected to know about different python integer sized types to be able to use libvirt. So, fixed and sent out a new solution
Regards, Daniel
participants (2)
-
Daniel P. Berrangé -
Edgar Kaziakhmedov