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

Re: [libvirt] [PATCH] libvirt-override: fix flags mutually exclusize



On Fri, Feb 17, 2017 at 07:19:25PM +0800, Jie Wang wrote:

Just a nit that we tend to prefix the patch with [libvirt-python] instead of
just [libvirt] so it's absolutely clear which repository you're sending the
patch against and in this case it's a python bindings fix.

> As virDomainGetBlkioParameters is called in libvirt_virDomainSetBlkioParameters,
> it will result in the two flags 'VIR_DOMAIN_AFFECT_LIVE' and 'VIR_DOMAIN_AFFECT_CONFIG'
> are mutually exclusive in libvirt_virDomainSetBlkioParameters, it's unreasonable,
> So ues this patch to fix it.
> 
> Signed-off-by: Jie Wang <wangjie88 huawei com>
> ---
>  libvirt-override.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libvirt-override.c b/libvirt-override.c
> index 9e40f00..7bdc09c 100644
> --- a/libvirt-override.c
> +++ b/libvirt-override.c
> @@ -727,7 +727,7 @@ libvirt_virDomainSetBlkioParameters(PyObject *self ATTRIBUTE_UNUSED,
>      }
>  
>      LIBVIRT_BEGIN_ALLOW_THREADS;
> -    i_retval = virDomainGetBlkioParameters(domain, NULL, &nparams, flags);
> +    i_retval = virDomainGetBlkioParameters(domain, NULL, &nparams, 0);
>      LIBVIRT_END_ALLOW_THREADS;
>  
>      if (i_retval < 0)
> @@ -743,7 +743,7 @@ libvirt_virDomainSetBlkioParameters(PyObject *self ATTRIBUTE_UNUSED,
>          return PyErr_NoMemory();
>  
>      LIBVIRT_BEGIN_ALLOW_THREADS;
> -    i_retval = virDomainGetBlkioParameters(domain, params, &nparams, flags);
> +    i_retval = virDomainGetBlkioParameters(domain, params, &nparams, 0);
>      LIBVIRT_END_ALLOW_THREADS;
>  

IMHO the correct fix would be to get rid of the getters completely as that is
something the python API adds as an 'extra' compared to the plain C API - it
never performed such a check and it never will since it's been done on the
server side for quite some time. So, if someone used the old C API with a new
client and an old server using some unsupported typed params, the old server
would happily interpret all the ones it knew and fail at the first unknown one,
however, there was no rollback involved. Similarly, the python API should behave
exactly the same, irregardless of the correctness of the old server's behaviour.

CC'ing Dan what he thinks about the python API doing some extra checking to
compensate for the old server's behaviour.

Regards,
Erik


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