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

Re: [libvirt] [PATCHv2] python: Fix emulatorpin API bindings



On 03/20/2013 08:48 AM, 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       | 118 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 136 insertions(+), 2 deletions(-)

> +++ 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&apos;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&apos;ed set of virDomainModificationImpact'/>

I'd LOVE to make python treat flags as an optional argument that
defaults to 0 when not present, instead of a mandatory argument, but I
don't know enough about python bindings to provide a patch.  That would
be a separate cleanup for a lot more than just this function, though.

>      </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'>

Looks like an unintentional whitespace change (turning 4 spaces into 5)
snuck in here.


> +static PyObject *
> +libvirt_virDomainGetEmulatorPinInfo(PyObject *self ATTRIBUTE_UNUSED,
> +                                    PyObject *args)
> +{

> +
> +    for (pcpu = 0; pcpu < cpunum; pcpu++)
> +        PyTuple_SET_ITEM(pycpumap, pcpu,
> +                         PyBool_FromLong(VIR_CPU_USABLE(cpumap, cpumaplen,
> +                                                        0, pcpu)));

The counterpart of libvirt_virDomainGetVpcpuPinInfo() used
PyTuple_SetItem instead of PyTuple_SET_ITEM; any reason?

At any rate, this looks sane modulo the whitespace tweak, so:

ACK.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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