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?

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

Philipp
-- 
Philipp Hahn
Open Source Software Engineer

Univention GmbH
be open.
Mary-Somerville-Str. 1
D-28359 Bremen
Tel.: +49 421 22232-0
Fax : +49 421 22232-99
h...@univention.de

http://www.univention.de/
Geschäftsführer: Peter H. Ganten
HRB 20755 Amtsgericht Bremen
Steuer-Nr.: 71-597-02876
From b58456ac8cab8d84c40f8ac8222832b0ecbd6771 Mon Sep 17 00:00:00 2001
Message-Id: <b58456ac8cab8d84c40f8ac8222832b0ecbd6771.1542784226.git.h...@univention.de>
From: Philipp Hahn <h...@univention.de>
Date: Wed, 21 Nov 2018 07:55:57 +0100
Subject: [PATCH libvirt-python 1/3] snap: pass snapshot reference to exception
To: libvir-list@redhat.com

instead of passing it as a connection reference.

Signed-off-by: Philipp Hahn <h...@univention.de>
---
 libvirt-override-virDomainSnapshot.py | 2 +-
 libvirt-override.py                   | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/libvirt-override-virDomainSnapshot.py b/libvirt-override-virDomainSnapshot.py
index 722cee3..ce59b09 100644
--- a/libvirt-override-virDomainSnapshot.py
+++ b/libvirt-override-virDomainSnapshot.py
@@ -12,7 +12,7 @@
         """List all child snapshots and returns a list of snapshot objects"""
         ret = libvirtmod.virDomainSnapshotListAllChildren(self._o, flags)
         if ret is None:
-            raise libvirtError("virDomainSnapshotListAllChildren() failed", conn=self)
+            raise libvirtError("virDomainSnapshotListAllChildren() failed", snap=self)
 
         retlist = list()
         for snapptr in ret:
diff --git a/libvirt-override.py b/libvirt-override.py
index c0c61eb..9c18d58 100644
--- a/libvirt-override.py
+++ b/libvirt-override.py
@@ -29,7 +29,8 @@ class libvirtError(Exception):
                  dom=None,  # type: Optional[virDomain]
                  net=None,  # type: Optional[virNetwork]
                  pool=None,  # type: Optional[virStoragePool]
-                 vol=None  # type: Optional[virStorageVol]
+                 vol=None,  # type: Optional[virStorageVol]
+                 snap=None  # type: Optional[virDomainSnapshot]
                  ):
 
         # Never call virConnGetLastError().
-- 
2.11.0

From 7ccc762b5f44d6c1b613852092fc747675a3936a Mon Sep 17 00:00:00 2001
Message-Id: <7ccc762b5f44d6c1b613852092fc747675a3936a.1542784226.git.h...@univention.de>
In-Reply-To: <b58456ac8cab8d84c40f8ac8222832b0ecbd6771.1542784226.git.h...@univention.de>
References: <b58456ac8cab8d84c40f8ac8222832b0ecbd6771.1542784226.git.h...@univention.de>
From: Philipp Hahn <h...@univention.de>
Date: Wed, 21 Nov 2018 08:07:23 +0100
Subject: [PATCH libvirt-python 2/3] snap: pass domain reference to exception
To: libvir-list@redhat.com

instead of passing it as a connection reference.

Signed-off-by: Philipp Hahn <h...@univention.de>
---
 libvirt-override-virDomain.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt-override-virDomain.py b/libvirt-override-virDomain.py
index 590d5c0..fb3f305 100644
--- a/libvirt-override-virDomain.py
+++ b/libvirt-override-virDomain.py
@@ -4,7 +4,7 @@
         """List all snapshots and returns a list of snapshot objects"""
         ret = libvirtmod.virDomainListAllSnapshots(self._o, flags)
         if ret is None:
-            raise libvirtError("virDomainListAllSnapshots() failed", conn=self)
+            raise libvirtError("virDomainListAllSnapshots() failed", dom=self)
 
         retlist = list()
         for snapptr in ret:
-- 
2.11.0

From cccf432d1bdbe625c88b91695e124deca7dd1c11 Mon Sep 17 00:00:00 2001
Message-Id: <cccf432d1bdbe625c88b91695e124deca7dd1c11.1542784226.git.h...@univention.de>
In-Reply-To: <b58456ac8cab8d84c40f8ac8222832b0ecbd6771.1542784226.git.h...@univention.de>
References: <b58456ac8cab8d84c40f8ac8222832b0ecbd6771.1542784226.git.h...@univention.de>
From: Philipp Hahn <h...@univention.de>
Date: Wed, 21 Nov 2018 08:09:15 +0100
Subject: [PATCH libvirt-python 3/3] snap: pass pool reference to exception
To: libvir-list@redhat.com

instead of passing it as a connection reference.

Signed-off-by: Philipp Hahn <h...@univention.de>
---
 libvirt-override-virStoragePool.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt-override-virStoragePool.py b/libvirt-override-virStoragePool.py
index 1f7c41e..807b099 100644
--- a/libvirt-override-virStoragePool.py
+++ b/libvirt-override-virStoragePool.py
@@ -4,7 +4,7 @@
         """List all storage volumes and returns a list of storage volume objects"""
         ret = libvirtmod.virStoragePoolListAllVolumes(self._o, flags)
         if ret is None:
-            raise libvirtError("virStoragePoolListAllVolumes() failed", conn=self)
+            raise libvirtError("virStoragePoolListAllVolumes() failed", pool=self)
 
         retlist = list()
         for volptr in ret:
-- 
2.11.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to