On Mon, Mar 11, 2019 at 12:00 PM Daniel P. Berrangé <berrange@xxxxxxxxxx> wrote:
On Sun, Mar 10, 2019 at 03:03:25AM +0200, Nir Soffer wrote:
> Commit 2b4bd07e0a22 (Add check for params, nparams being a dictionary)
> changed the way the optional params argument is treated. If
> libvirt.virDomain.blockCopy() is called without specifying params,
> params is None, and the call will fail with:
>
> TypeError: block params must be a dictionary
>
> This is wrong as params is defined as kwarg, breaking existing libvirt
> users like oVirt. Remove the check, restoring the previous behaviour.
>
> Resolves: https://bugzilla.redhat.com/1687114
>
> Signed-off-by: Nir Soffer <nsoffer@xxxxxxxxxx>
> ---
> libvirt-override.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/libvirt-override.c b/libvirt-override.c
> index 857c018..127d71c 100644
> --- a/libvirt-override.c
> +++ b/libvirt-override.c
> @@ -8829,19 +8829,17 @@ libvirt_virDomainBlockCopy(PyObject *self ATTRIBUTE_UNUSED,
>
> if (!PyArg_ParseTuple(args, (char *) "Ozz|OI:virDomainBlockCopy",
> &pyobj_dom, &disk, &destxml, &pyobj_dict, &flags))
> return NULL;
>
> + /* Note: params is optional in libvirt.py */
> if (PyDict_Check(pyobj_dict)) {
> if (virPyDictToTypedParams(pyobj_dict, ¶ms, &nparams,
> virPyDomainBlockCopyParams,
> VIR_N_ELEMENTS(virPyDomainBlockCopyParams)) < 0) {
> return NULL;
> }
> - } else {
> - PyErr_Format(PyExc_TypeError, "block params must be a dictionary");
> - return NULL;
> }
Rather than removing this else branch, I think we should have an explict
check for None first
if (pyobj_dict == Py_None) {
...no op..
} else if (PyDict_Check(....)) {
} else {
.... error ...
}
Can you do that & validate that it works with oVirt.
Sure, I'll send v2.
Nir
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
