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

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(a)hds.com&gt; >> --- >> 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(a)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