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