[libvirt] [BUG][PATCH][RRFC][libvirt-python] libvirtError(..., conn=, dom=. vol=, pool=, snap=)

[Michal Privoznik]

On 11/21/18 8:17 AM, Philipp Hahn wrote:
> Hi,
> 
> while working on the Python type annotations for the Python libvirt
> binding I noticed the following code in
> libvirt-override-virDomainSnapshot.py:
> 
>>     def listAllChildren(self, flags=0):
>>         """List all child snapshots and returns a list of snapshot objects"""
> ...
>>             raise libvirtError("..., conn=self)
> 
> "self" is an instance of virDomainSnapshot here.
> 
> I found two similar cases where "conn" was not a "virConnect" instance
> in listAllSnapshots() and listAllVolumes().
> 
> Compare that with the declaration of libvirtError in libvirt-override.py:
> 
>> class libvirtError(Exception):
>>     def __init__(self, defmsg, conn=None, dom=None, net=None, pool=None, vol=None):
> 
> Looking further at the implementation of that method only "defmsg" is
> used; all other references are not accessed and never stored in an
> instance variable.
> 
> Should I add a new "snap" argument to libvirtError.__init__() or should
> we stop passing those references to the constructor altogether?

I think we should pass whatever object the error occurred on. Your patch
#2 demonstrates this beautifully - with the current code conn will point
to something that is not instance of virConnect rather than virDomain class.

> 
> Patch 2 and 3 might be applied to the current branch already, patch 1
> currently depends on my other work.

ACK to them, do you want me to apply them now or should I wait (e.g.
will you send them in some series?)

Michal