[libvirt] [PATCH] python: Fix emulatorpin API bindings
Guannan Ren
gren at redhat.com
Wed Mar 20 12:50:05 UTC 2013
On 03/20/2013 06:52 PM, Peter Krempa wrote:
> The addition of emulator pinning APIs didn't think of doing the right
> job with python APIs for them. The default generator produced unusable
> code for this.
>
> This patch switches to proper code as in the case of domain Vcpu pining.
> This change can be classified as a python API-breaker but in the state
> the code was before I doubt anyone was able to use it successfully.
> ---
> python/generator.py | 2 +
> python/libvirt-override-api.xml | 18 +++++-
> python/libvirt-override.c | 121 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 139 insertions(+), 2 deletions(-)
>
> diff --git a/python/generator.py b/python/generator.py
> index 6a25c2d..0aeb675 100755
> --- a/python/generator.py
> +++ b/python/generator.py
> @@ -418,6 +418,8 @@ skip_impl = (
> 'virDomainPinVcpu',
> 'virDomainPinVcpuFlags',
> 'virDomainGetVcpuPinInfo',
> + 'virDomainGetEmulatorPinInfo',
> + 'virDomainPinEmulator',
> 'virSecretGetValue',
> 'virSecretSetValue',
> 'virSecretGetUUID',
> diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
> index 5976fb2..c720610 100644
> --- a/python/libvirt-override-api.xml
> +++ b/python/libvirt-override-api.xml
> @@ -237,10 +237,24 @@
> <function name='virDomainGetVcpuPinInfo' file='python'>
> <info>Query the CPU affinity setting of all virtual CPUs of domain</info>
> <return type='unsigned char *' info='the array of cpumap'/>
> - <arg name='domain' type='virDomainPtr' info='pointer to domain object, or NULL for Domain0'/>
> + <arg name='domain' type='virDomainPtr' info='pointer to domain object'/>
> + <arg name='flags' type='int' info='an OR'ed set of virDomainModificationImpact'/>
> + </function>
> + <function name='virDomainGetEmulatorPinInfo' file='python'>
> + <info>Query the CPU affinity setting of the emulator process of domain</info>
> + <return type='unsigned char *' info='the array of cpumap'/>
> + <arg name='domain' type='virDomainPtr' info='pointer to domain object'/>
> <arg name='flags' type='int' info='an OR'ed set of virDomainModificationImpact'/>
> </function>
> - <function name='virDomainSetSchedulerParameters' file='python'>
> + <function name='virDomainPinEmulator' file='python'>
> + <info>Dynamically change the real CPUs which can be allocated to the emulator process of a domain.
> + This function requires privileged access to the hypervisor.</info>
> + <return type='int' info='0 in case of success, -1 in case of failure.'/>
> + <arg name='domain' type='virDomainPtr' info='pointer to domain object, or NULL for Domain0'/>
> + <arg name='cpumap' type='unsigned char *' info='pointer to a bit map of real CPUs (in 8-bit bytes) (IN) Each bit set to 1 means that corresponding CPU is usable. Bytes are stored in little-endian order: CPU0-7, 8-15... In each byte, lowest CPU number is least significant bit.'/>
> + <arg name='flags' type='int' info='flags to specify'/>
> + </function>
> + <function name='virDomainSetSchedulerParameters' file='python'>
> <info>Change the scheduler parameters</info>
> <return type='int' info='-1 in case of error, 0 in case of success.'/>
> <arg name='domain' type='virDomainPtr' info='pointer to domain object'/>
> diff --git a/python/libvirt-override.c b/python/libvirt-override.c
> index 9637598..07e0221 100644
> --- a/python/libvirt-override.c
> +++ b/python/libvirt-override.c
> @@ -1664,6 +1664,125 @@ cleanup:
> return VIR_PY_NONE;
> }
>
> +
> +static PyObject *
> +libvirt_virDomainPinEmulator(PyObject *self ATTRIBUTE_UNUSED,
> + PyObject *args)
> +{
> + virDomainPtr domain;
> + PyObject *pyobj_domain, *pycpumap;
> + PyObject *ret = VIR_PY_INT_FAIL;
> + unsigned char *cpumap = NULL;
> + int cpumaplen, i, tuple_size, cpunum;
> + int i_retval;
> + unsigned int flags;
> +
> + if (!PyArg_ParseTuple(args, (char *)"OOi:virDomainPinVcpu",
> + &pyobj_domain, &pycpumap, &flags))
> + goto cleanup;
> +
> + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
> +
> + if ((cpunum = getPyNodeCPUCount(virDomainGetConnect(domain))) < 0)
> + goto cleanup;
> +
> + cpumaplen = VIR_CPU_MAPLEN(cpunum);
> +
> + if (!PyTuple_Check(pycpumap)) {
> + PyErr_SetString(PyExc_TypeError, "Unexpected type, tuple is required");
> + goto cleanup;
return NULL, because you set an exception by using
PyErr_SetString().
return -1, the upper user will not see the exception.
> + }
> +
> + if ((tuple_size = PyTuple_Size(pycpumap)) == -1)
> + goto cleanup;
If PyTuple_size return -1, the function should return NULL
instead of VIR_PY_INT_FAIL
The NULL will trigger a standard python error info.
Commonly, we only return VIR_PY_INT_FAIL or VIR_PY_NONE
when libvirt API error occurs.
> +
> + if (VIR_ALLOC_N(cpumap, cpumaplen) < 0)
> + goto cleanup;
There, it'd better to throw a python no memory error like follows
if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) {
PyErr_NoMemory();
return NULL;
}
> +
> + for (i = 0; i < tuple_size; i++) {
> + PyObject *flag = PyTuple_GetItem(pycpumap, i);
> + bool b;
> +
> + if (!flag || libvirt_boolUnwrap(flag, &b) < 0)
> + goto cleanup;
> +
> + if (b)
> + VIR_USE_CPU(cpumap, i);
> + else
> + VIR_UNUSE_CPU(cpumap, i);
> + }
> +
> + for (; i < cpunum; i++)
> + VIR_UNUSE_CPU(cpumap, i);
> +
> + LIBVIRT_BEGIN_ALLOW_THREADS;
> + i_retval = virDomainPinEmulator(domain, cpumap, cpumaplen, flags);
> + LIBVIRT_END_ALLOW_THREADS;
> +
> + if (i_retval < 0)
> + goto cleanup;
> +
> + ret = VIR_PY_INT_SUCCESS;
> +
> +cleanup:
> + VIR_FREE(cpumap);
> + return ret;
> +}
> +
> +
> +static PyObject *
> +libvirt_virDomainGetEmulatorPinInfo(PyObject *self ATTRIBUTE_UNUSED,
> + PyObject *args)
> +{
> + virDomainPtr domain;
> + PyObject *pyobj_domain;
> + PyObject *pycpumap = NULL;
> + unsigned char *cpumap = NULL;
> + size_t cpumaplen;
> + size_t pcpu;
> + unsigned int flags;
> + int ret;
> + int cpunum;
> +
> + if (!PyArg_ParseTuple(args, (char *)"Oi:virDomainEmulatorPinInfo",
> + &pyobj_domain, &flags))
> + goto error;
> +
> + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
> +
> + if ((cpunum = getPyNodeCPUCount(virDomainGetConnect(domain))) < 0)
> + goto error;
> +
> + cpumaplen = VIR_CPU_MAPLEN(cpunum);
> + if (VIR_ALLOC_N(cpumap, cpumaplen) < 0)
> + goto error;
the same.
> +
> + LIBVIRT_BEGIN_ALLOW_THREADS;
> + ret = virDomainGetEmulatorPinInfo(domain, cpumap, cpumaplen, flags);
> + LIBVIRT_END_ALLOW_THREADS;
> + if (ret < 0)
> + goto error;
> +
> + if (!(pycpumap = PyTuple_New(cpunum)))
> + goto error;
> +
return NULL is better.
> + for (pcpu = 0; pcpu < cpunum; pcpu++)
> + PyTuple_SetItem(pycpumap, pcpu,
> + PyBool_FromLong(VIR_CPU_USABLE(cpumap, cpumaplen,
> + 0, pcpu)));
> +
> + VIR_FREE(cpumap);
> +
> + return pycpumap;
> +
> +error:
> + VIR_FREE(cpumap);
> + Py_XDECREF(pycpumap);
> +
> + return VIR_PY_NONE;
> +}
> +
> +
> /************************************************************************
> * *
> * Global error handler at the Python level *
> @@ -6705,6 +6824,8 @@ static PyMethodDef libvirtMethods[] = {
> {(char *) "virDomainPinVcpu", libvirt_virDomainPinVcpu, METH_VARARGS, NULL},
> {(char *) "virDomainPinVcpuFlags", libvirt_virDomainPinVcpuFlags, METH_VARARGS, NULL},
> {(char *) "virDomainGetVcpuPinInfo", libvirt_virDomainGetVcpuPinInfo, METH_VARARGS, NULL},
> + {(char *) "virDomainGetEmulatorPinInfo", libvirt_virDomainGetEmulatorPinInfo, METH_VARARGS, NULL},
> + {(char *) "virDomainPinEmulator", libvirt_virDomainPinEmulator, METH_VARARGS, NULL},
> {(char *) "virConnectListStoragePools", libvirt_virConnectListStoragePools, METH_VARARGS, NULL},
> {(char *) "virConnectListDefinedStoragePools", libvirt_virConnectListDefinedStoragePools, METH_VARARGS, NULL},
> {(char *) "virConnectListAllStoragePools", libvirt_virConnectListAllStoragePools, METH_VARARGS, NULL},
There are some python functions which we doesn't check the
return value.
It's okay, but I think checking is always better. Others are
fine to me.
Guannan
More information about the libvir-list
mailing list