On 12/14/2013 01:36 PM, Daniel P. Berrange wrote:
> On Sun, Nov 24, 2013 at 10:46:13AM -0600, Doug Goldstein wrote:
>> On Sat, Nov 23, 2013 at 3:15 PM, Don Dugger <n0ano@xxxxxxxxx> wrote:
>>>
>>> This Python interface code is returning a -1 on errors for the
>>> `baselineCPU' API. Since this API is supposed to return a pointer
>>> the error return value should really be VIR_PY_NONE.
>>>
>>> NB: I've checked all the other APIs in this file and this is the
>>> only pointer API that is returning -1.
>>>
>>> Signed-off-by: Don Dugger <donald.d.dugger@xxxxxxxxx>
>>> ---
>>> python/libvirt-override.c | 8 ++++----
>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/python/libvirt-override.c b/python/libvirt-override.c
>>> index c60747d..b471605 100644
>>> --- a/python/libvirt-override.c
>>> +++ b/python/libvirt-override.c
>>> @@ -4471,13 +4471,13 @@ libvirt_virConnectBaselineCPU(PyObject *self ATTRIBUTE_UNUSED,
>>>
>>> ncpus = PyList_Size(list);
>>> if (VIR_ALLOC_N_QUIET(xmlcpus, ncpus) < 0)
>>> - return VIR_PY_INT_FAIL;
>>> + return VIR_PY_NONE;
>>>
>>> for (i = 0; i < ncpus; i++) {
>>> xmlcpus[i] = PyString_AsString(PyList_GetItem(list, i));
>>> if (xmlcpus[i] == NULL) {
>>> VIR_FREE(xmlcpus);
>>> - return VIR_PY_INT_FAIL;
>>> + return VIR_PY_NONE;
>>> }
>>> }
>>> }
>>> @@ -4489,13 +4489,13 @@ libvirt_virConnectBaselineCPU(PyObject *self ATTRIBUTE_UNUSED,
>>> VIR_FREE(xmlcpus);
>>>
>>> if (base_cpu == NULL)
>>> - return VIR_PY_INT_FAIL;
>>> + return VIR_PY_NONE;
>>>
>>> pybase_cpu = PyString_FromString(base_cpu);
>>> VIR_FREE(base_cpu);
>>>
>>> if (pybase_cpu == NULL)
>>> - return VIR_PY_INT_FAIL;
>>> + return VIR_PY_NONE;
>>>
>>> return pybase_cpu;
>>> }
>>> --
>>> 1.7.10.4
>>>
>>
>> ACK. This is correct. But it obviously changes our API so I'm not
>> really sure how we should handle this, (e.g. document the API as is as
>> note that its broken or fix it).
>
> The implicit expectation with python APIs is that they all raise an
> exception if the libvirt call fails. So ACK to this bug fix & we
> should put it in maint branches.
>
> That said, please hold off applying this to GIT. We'll put it into
> the new libvirt-python git I'm about to push instead.
>
I've pushed this to 0.10.2 and 1.0.5 maint branches now.
Thanks,
Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list