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