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

Re: [libvirt] [libvirt-python PATCH] override: Implement bindings for virDomainGetFSInfo as domain.fsInfo



On 11/20/14, 7:24 , "Pavel Hrdina" <phrdina redhat com> wrote:

>On 11/20/2014 01:22 PM, Pavel Hrdina wrote:
>> On 11/18/2014 12:29 AM, Tomoki Sekiyama wrote:
>>> Implement the function which returns a list of tuples, that contains
>>> members
>>> of virDomainFSInfo struct.
>>>
>>> Signed-off-by: Tomoki Sekiyama <tomoki sekiyama hds com>
>>> ---
>>>   generator.py             |    5 +++
>>>   libvirt-override-api.xml |    6 ++++
>>>   libvirt-override.c       |   70
>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>   sanitytest.py            |    5 +++
>>>   4 files changed, 85 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/generator.py b/generator.py
>>> index c66c6d4..39ba2f7 100755
>>> --- a/generator.py
>>> +++ b/generator.py
>>> @@ -476,6 +476,7 @@ skip_impl = (
>>>       'virNetworkGetDHCPLeases',
>>>       'virDomainBlockCopy',
>>>       'virNodeAllocPages',
>>> +    'virDomainGetFSInfo',
>>>   )
>>>
>>>   lxc_skip_impl = (
>>> @@ -586,6 +587,7 @@ skip_function = (
>>>
>>>       'virNetworkDHCPLeaseFree', # only useful in C, python code uses
>>> list
>>>       'virDomainStatsRecordListFree', # only useful in C, python uses
>>> dict
>>> +    'virDomainFSInfoFree', # only useful in C, python code uses list
>>>   )
>>>
>>>   lxc_skip_function = (
>>> @@ -1107,6 +1109,9 @@ def nameFixup(name, classe, type, file):
>>>       elif name[0:20] == "virDomainGetCPUStats":
>>>           func = name[9:]
>>>           func = func[0:1].lower() + func[1:]
>>> +    elif name[0:20] == "virDomainGetFSInfo":
>>
>> There definitely must be name[0:18] as John pointed out.
>>
>>> +        func = name[12:]
>>> +        func = func[0:2].lower() + func[2:]
>>>       elif name[0:12] == "virDomainGet":
>>>           func = name[12:]
>>>           func = func[0:1].lower() + func[1:]
>>> diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml
>>> index 4fe3c4d..2e807ba 100644
>>> --- a/libvirt-override-api.xml
>>> +++ b/libvirt-override-api.xml
>>> @@ -658,5 +658,11 @@
>>>         <arg name='flags' type='unsigned int' info='an OR&apos;ed set
>>> of virNodeAllocPagesFlags'/>
>>>         <return type='int' info='the number of nodes successfully
>>> adjusted or -1 in case of an error'/>
>>>       </function>
>>> +    <function name="virDomainGetFSInfo" file='python'>
>>> +      <info>Get a list of mapping informaiton for each mounted file
>>> systems within the specified guest and the disks.</info>
>>> +      <arg name='domain' type='virDomainPtr' info='pointer to domain
>>> object'/>
>>> +      <arg name='flags' type='unsigned int' info='unused, pass 0'/>
>>> +      <return type='char *' info="list of mounted filesystems
>>> information"/>
>>> +    </function>
>>>     </symbols>
>>>   </api>
>>> diff --git a/libvirt-override.c b/libvirt-override.c
>>> index 8895289..bd6321f 100644
>>> --- a/libvirt-override.c
>>> +++ b/libvirt-override.c
>>> @@ -8266,6 +8266,73 @@ libvirt_virNodeAllocPages(PyObject *self
>>> ATTRIBUTE_UNUSED,
>>>   }
>>>   #endif /* LIBVIR_CHECK_VERSION(1, 2, 8) */
>>>
>>> +#if LIBVIR_CHECK_VERSION(1, 2, 11)
>>> +
>>> +static PyObject *
>>> +libvirt_virDomainGetFSInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject
>>> *args) {
>>> +    virDomainPtr domain;
>>> +    PyObject *pyobj_domain;
>>> +    unsigned int flags;
>>> +    virDomainFSInfoPtr *fsinfo = NULL;
>>> +    char **dev;
>>> +    int c_retval, i;
>>> +    PyObject *py_retval;
>>> +
>>> +    if (!PyArg_ParseTuple(args, (char *)"Oi:virDomainFSInfo",
>>> +        &pyobj_domain, &flags))
>>> +        return NULL;
>>> +    domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
>>> +
>>> +    LIBVIRT_BEGIN_ALLOW_THREADS;
>>> +    c_retval = virDomainGetFSInfo(domain, &fsinfo, flags);
>>> +    LIBVIRT_END_ALLOW_THREADS;
>>> +
>>> +    if (c_retval < 0)
>>> +        goto cleanup;
>>> +
>>> +    /* convert to a Python list */
>>> +    if ((py_retval = PyList_New(c_retval)) == NULL)
>>> +        goto cleanup;
>>
>> The PyList_New on success return new reference ... [1]
>>
>>> +
>>> +    for (i = 0; i < c_retval; i++) {
>>> +        virDomainFSInfoPtr fs = fsinfo[i];
>>> +        PyObject *info, *alias;
>>> +
>>> +        if (fs == NULL)
>>> +            goto cleanup;
>>> +        info = PyTuple_New(4);
>>> +    if (info == NULL)
>>
>> Wrong indentation, use spaces instead of tabs.
>>
>>> +            goto cleanup;
>>> +        PyList_SetItem(py_retval, i, info);
>>> +        alias = PyList_New(0);
>>> +    if (alias == NULL)
>>
>> The same wrong indentation.
>>
>>> +            goto cleanup;
>>> +
>>> +        PyTuple_SetItem(info, 0,
>>> libvirt_constcharPtrWrap(fs->mountpoint));
>>> +        PyTuple_SetItem(info, 1, libvirt_constcharPtrWrap(fs->name));
>>> +        PyTuple_SetItem(info, 2, libvirt_constcharPtrWrap(fs->type));
>>> +        PyTuple_SetItem(info, 3, alias);
>>> +
>>> +        for (dev = fs->devAlias; dev && *dev; dev++)
>>> +            if (PyList_Append(alias, libvirt_constcharPtrWrap(*dev))
>>> < 0)
>>> +                goto cleanup;
>>> +    }
>>> +
>>> +    for (i = 0; i < c_retval; i++)
>>> +        virDomainFSInfoFree(fsinfo[i]);
>>> +    VIR_FREE(fsinfo);
>>> +    return py_retval;
>>> +
>>> + cleanup:
>>> +    for (i = 0; i < c_retval; i++)
>>> +        virDomainFSInfoFree(fsinfo[i]);
>>> +    VIR_FREE(fsinfo);
>>> +    Py_DECREF(py_retval);
>>
>> [1] ... there you correctly dereference it, but you should use
>> PY_XDECREF because the py_retval could be NULL.
>>
>>> +    return VIR_PY_NONE;
>>> +}
>>> +
>>> +#endif /* LIBVIR_CHECK_VERSION(1, 2, 11) */
>>> +
>>>
>>> 
>>>/***********************************************************************
>>>*
>>>    *                                    *
>>>    *            The registration stuff                *
>>> @@ -8459,6 +8526,9 @@ static PyMethodDef libvirtMethods[] = {
>>>   #if LIBVIR_CHECK_VERSION(1, 2, 9)
>>>       {(char *) "virNodeAllocPages", libvirt_virNodeAllocPages,
>>> METH_VARARGS, NULL},
>>>   #endif /* LIBVIR_CHECK_VERSION(1, 2, 9) */
>>> +#if LIBVIR_CHECK_VERSION(1, 2, 11)
>>> +    {(char *) "virDomainGetFSInfo", libvirt_virDomainGetFSInfo,
>>> METH_VARARGS, NULL},
>>> +#endif /* LIBVIR_CHECK_VERSION(1, 2, 11) */
>>>       {NULL, NULL, 0, NULL}
>>>   };
>>>
>>> diff --git a/sanitytest.py b/sanitytest.py
>>> index b161696..f5337fc 100644
>>> --- a/sanitytest.py
>>> +++ b/sanitytest.py
>>> @@ -137,6 +137,9 @@ for cname in wantfunctions:
>>>       if name[0:28] == "virDomainStatsRecordListFree":
>>>           continue
>>>
>>> +    if name[0:19] == "virDomainFSInfoFree":
>>> +        continue
>>> +
>>>       if name[0:21] == "virDomainListGetStats":
>>>           name = "virConnectDomainListGetStats"
>>>
>>> @@ -269,7 +272,7 @@ for name in sorted(basicklassmap):
>>>       func = func[0:1].lower() + func[1:]
>>>       if func[0:8] == "nWFilter":
>>>           func = "nwfilter" + func[8:]
>>> -    if func[0:8] == "fSFreeze" or func[0:6] == "fSThaw":
>>> +    if func[0:8] == "fSFreeze" or func[0:6] == "fSThaw" or func[0:6]
>>> == "fSInfo":
>>>           func = "fs" + func[2:]
>>>
>>>       if klass == "virNetwork":
>>>
>>> --
>>> libvir-list mailing list
>>> libvir-list redhat com
>>> https://www.redhat.com/mailman/listinfo/libvir-list
>>>
>>
>> Returning VIR_PY_NONE is safe there as none of the used function sets
>> the python exception and the error is handled in the python wrapper.
>>
>> ACK
>>
>> Pavel
>>
>
>The ACK is of course with the changes mentioned above.
>
>Pavel

Thank you for the review.
I¹ll fix these and re-post the patch v2.

Thanks,
Tomoki Sekiyama




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