Bug 1687114
| Summary: | blockCopy fail with bogus error: "TypeError: block params must be a dictionary" | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Nir Soffer <nsoffer> | ||||
| Component: | libvirt-python | Assignee: | Daniel Berrangé <berrange> | ||||
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
| Severity: | urgent | Docs Contact: | |||||
| Priority: | unspecified | ||||||
| Version: | 30 | CC: | berrange, crobinso, jferlan, pkrempa, veillard | ||||
| Target Milestone: | --- | ||||||
| Target Release: | --- | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Whiteboard: | |||||||
| Fixed In Version: | libvirt-python-5.1.0-2.fc30 | Doc Type: | If docs needed, set a value | ||||
| Doc Text: | Story Points: | --- | |||||
| Clone Of: | |||||||
| : | 1687712 (view as bug list) | Environment: | |||||
| Last Closed: | 2019-07-09 00:55:45 UTC | Type: | Bug | ||||
| Regression: | --- | Mount Type: | --- | ||||
| Documentation: | --- | CRM: | |||||
| Verified Versions: | Category: | --- | |||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||
| Embargoed: | |||||||
| Bug Depends On: | |||||||
| Bug Blocks: | 1687712 | ||||||
| Attachments: |
|
||||||
John, can you take a look? Created attachment 1542644 [details] Proposed patch I sent the patch to libvirt mailing list, but for some reason I don't see it in the archive: https://www.redhat.com/archives/libvir-list/2019-March/date.html I see Daniel has reviewed your patch already. Still to "answer" your question - the referenced commit was part of other changes to more consistently handle the arguments - something done as a result of code review comments from my initial posting of the series of patches (see 3/3 from the v1 https://www.redhat.com/archives/libvir-list/2018-November/msg00772.html). Seems I must have missed that "...|O..." means optional object argument, sorry about that. Broken in:
$ git desc --contains 2b4bd07e0a22
v4.10.0~4
Will be fixed in 5.2.0
commit 5d6228d4171c0a9d1299ad742dc878092287675a (HEAD -> master, origin/master, origin/HEAD)
Author: Nir Soffer <nirsof>
Date: Mon Mar 11 23:34:50 2019 +0200
Fix handling of optional params in blockCopy()
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. Add a check for Py_None, so we accept either a dict or
None and fail with TypeError with anything else.
Resolves: https://bugzilla.redhat.com/1687114
Signed-off-by: Nir Soffer <nsoffer>
libvirt-python-5.2.0-1.fc31 is in rawhide now, but we can backport to fedora 30 too FEDORA-2019-0a6b4bee90 has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-0a6b4bee90 libvirt-python-5.1.0-2.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-0a6b4bee90 libvirt-python-5.1.0-2.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report. |
Description of problem: When blockCopy is called without specifying the params argument, the call fails with: TypeError: block params must be a dictionary blockCopy is defined as: def blockCopy(self, disk, destxml, params=None, flags=0): """Copy the guest-visible contents of a disk image to a new file described by destxml """ ret = libvirtmod.virDomainBlockCopy(self._o, disk, destxml, params, flags) if ret == -1: raise libvirtError ('virDomainBlockCopy() failed', dom=self) return ret So when calling it without the params kwarg, params is None. Vdsm calls blockCopy like this: def _startDriveReplication(self, drive): destxml = xmlutils.tostring(drive.getReplicaXML()) self.log.debug("Replicating drive %s to %s", drive.name, destxml) flags = (libvirt.VIR_DOMAIN_BLOCK_COPY_SHALLOW | libvirt.VIR_DOMAIN_BLOCK_COPY_REUSE_EXT | libvirt.VIR_DOMAIN_BLOCK_COPY_TRANSIENT_JOB) self._dom.blockCopy(drive.name, destxml, flags=flags) Version-Release number of selected component (if applicable): $ rpm -q python2-libvirt python2-libvirt-5.0.0-2.fc28.x86_64 $ rpm -q libvirt-daemon libvirt-daemon-5.0.0-3.fc28.x86_64 How reproducible: Always Additional info: I think this was broken by this patch: commit 2b4bd07e0a2239cd9a4da49b9bd5229ac46e7875 Author: John Ferlan <jferlan> Date: Tue Nov 20 11:56:47 2018 -0500 Add check for params, nparams being a dictionary If PyDict_Check fails, we should force an error rather than blindly continuing on. Signed-off-by: John Ferlan <jferlan> Reviewed-by: Pavel Hrdina <phrdina> Adding this check: 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; } The correct check should allow dict or None.