[libvirt] [PATCH v2] python: Fix bindings for virDomainSnapshotGet{Domain, Connect}
https://bugzilla.redhat.com/show_bug.cgi?id=895882 virDomainSnapshot.getDomain() and virDomainSnapshot.getConnect() wrappers around virDomainSnapshotGet{Domain,Connect} were not supposed to be ever implemented. The class should contain proper domain() and connect() accessors that fetch python objects stored internally within the class. While domain() was already provided, connect() was missing. This patch adds connect() method to virDomainSnapshot class and reimplements getDomain() and getConnect() methods as aliases to domain() and connect() for backward compatibility. --- python/generator.py | 4 +++- python/libvirt-override-virDomainSnapshot.py | 8 ++++++++ src/libvirt.c | 10 ++++++++-- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/python/generator.py b/python/generator.py index f853d77..5894616 100755 --- a/python/generator.py +++ b/python/generator.py @@ -527,6 +527,8 @@ skip_function = ( "virNWFilterGetConnect", "virStoragePoolGetConnect", "virStorageVolGetConnect", + "virDomainSnapshotGetConnect", + "virDomainSnapshotGetDomain", # only useful in C code, python code uses dict for typed parameters "virTypedParamsAddBoolean", @@ -953,7 +955,6 @@ classes_destructors = { class_skip_connect_impl = { "virConnect" : True, - "virDomainSnapshot": True, } class_domain_impl = { @@ -1436,6 +1437,7 @@ def buildWrappers(module): " self._conn = conn._conn\n") elif classname in [ "virDomainSnapshot" ]: classes.write(" self._dom = dom\n") + classes.write(" self._conn = dom.connect()\n") classes.write(" if _obj != None:self._o = _obj;return\n") classes.write(" self._o = None\n\n"); destruct=None diff --git a/python/libvirt-override-virDomainSnapshot.py b/python/libvirt-override-virDomainSnapshot.py index 3da7bfd..bf708a5 100644 --- a/python/libvirt-override-virDomainSnapshot.py +++ b/python/libvirt-override-virDomainSnapshot.py @@ -1,3 +1,11 @@ + def getConnect(self): + """Get the connection that owns the domain that a snapshot was created for""" + return self.connect() + + def getDomain(self): + """Get the domain that a snapshot was created for""" + return self.domain() + def listAllChildren(self, flags): """List all child snapshots and returns a list of snapshot objects""" ret = libvirtmod.virDomainSnapshotListAllChildren(self._o, flags) diff --git a/src/libvirt.c b/src/libvirt.c index d5d561c..fa39881 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17850,7 +17850,10 @@ virDomainSnapshotGetName(virDomainSnapshotPtr snapshot) * virDomainSnapshotGetDomain: * @snapshot: a snapshot object * - * Get the domain that a snapshot was created for + * Get the domain that a snapshot was created for. + * + * WARNING: When writing libvirt bindings in other languages, do not use this + * function. Instead, store the domain and the snapshot object together. * * Returns the domain or NULL. */ @@ -17874,7 +17877,10 @@ virDomainSnapshotGetDomain(virDomainSnapshotPtr snapshot) * virDomainSnapshotGetConnect: * @snapshot: a snapshot object * - * Get the connection that owns the domain that a snapshot was created for + * Get the connection that owns the domain that a snapshot was created for. + * + * WARNING: When writing libvirt bindings in other languages, do not use this + * function. Instead, store the connection and the snapshot object together. * * Returns the connection or NULL. */ -- 1.8.1.1
On 01/23/2013 04:26 AM, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=895882
virDomainSnapshot.getDomain() and virDomainSnapshot.getConnect() wrappers around virDomainSnapshotGet{Domain,Connect} were not supposed to be ever implemented. The class should contain proper domain() and connect() accessors that fetch python objects stored internally within the class. While domain() was already provided, connect() was missing.
This patch adds connect() method to virDomainSnapshot class and reimplements getDomain() and getConnect() methods as aliases to domain() and connect() for backward compatibility. --- python/generator.py | 4 +++- python/libvirt-override-virDomainSnapshot.py | 8 ++++++++ src/libvirt.c | 10 ++++++++-- 3 files changed, 19 insertions(+), 3 deletions(-)
* virDomainSnapshotGetDomain: * @snapshot: a snapshot object * - * Get the domain that a snapshot was created for + * Get the domain that a snapshot was created for. + *
Missing the following (based on copy-and-paste from virDomainGetConnect): * Provides the domain pointer associated with a snapshot. The * reference counter on the domain is not increased by this * call.
+ * WARNING: When writing libvirt bindings in other languages, do not use this + * function. Instead, store the domain and the snapshot object together. * * Returns the domain or NULL. */ @@ -17874,7 +17877,10 @@ virDomainSnapshotGetDomain(virDomainSnapshotPtr snapshot) * virDomainSnapshotGetConnect: * @snapshot: a snapshot object * - * Get the connection that owns the domain that a snapshot was created for + * Get the connection that owns the domain that a snapshot was created for. + *
Likewise: * Provides the connection pointer associated with a snapshot. The * reference counter on the connection is not increased by this * call.
+ * WARNING: When writing libvirt bindings in other languages, do not use this + * function. Instead, store the connection and the snapshot object together. * * Returns the connection or NULL. */
ACK with those additions; no need to see a v3. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Jan 24, 2013 at 11:26:35 -0700, Eric Blake wrote:
On 01/23/2013 04:26 AM, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=895882
virDomainSnapshot.getDomain() and virDomainSnapshot.getConnect() wrappers around virDomainSnapshotGet{Domain,Connect} were not supposed to be ever implemented. The class should contain proper domain() and connect() accessors that fetch python objects stored internally within the class. While domain() was already provided, connect() was missing.
This patch adds connect() method to virDomainSnapshot class and reimplements getDomain() and getConnect() methods as aliases to domain() and connect() for backward compatibility. --- python/generator.py | 4 +++- python/libvirt-override-virDomainSnapshot.py | 8 ++++++++ src/libvirt.c | 10 ++++++++-- 3 files changed, 19 insertions(+), 3 deletions(-)
* virDomainSnapshotGetDomain: * @snapshot: a snapshot object * - * Get the domain that a snapshot was created for + * Get the domain that a snapshot was created for. + *
Missing the following (based on copy-and-paste from virDomainGetConnect):
* Provides the domain pointer associated with a snapshot. The * reference counter on the domain is not increased by this * call.
You're right, it looks better this way. ...
ACK with those additions; no need to see a v3.
Thanks, I fixed the comments and pushed. Jirka
On 01/23/2013 06:26 AM, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=895882
virDomainSnapshot.getDomain() and virDomainSnapshot.getConnect() wrappers around virDomainSnapshotGet{Domain,Connect} were not supposed to be ever implemented. The class should contain proper domain() and connect() accessors that fetch python objects stored internally within the class. While domain() was already provided, connect() was missing.
This patch adds connect() method to virDomainSnapshot class and reimplements getDomain() and getConnect() methods as aliases to domain() and connect() for backward compatibility. --- python/generator.py | 4 +++- python/libvirt-override-virDomainSnapshot.py | 8 ++++++++ src/libvirt.c | 10 ++++++++-- 3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/python/libvirt-override-virDomainSnapshot.py b/python/libvirt-override-virDomainSnapshot.py index 3da7bfd..bf708a5 100644 --- a/python/libvirt-override-virDomainSnapshot.py +++ b/python/libvirt-override-virDomainSnapshot.py @@ -1,3 +1,11 @@ + def getConnect(self): + """Get the connection that owns the domain that a snapshot was created for""" + return self.connect() + + def getDomain(self): + """Get the domain that a snapshot was created for""" + return self.domain() + def listAllChildren(self, flags): """List all child snapshots and returns a list of snapshot objects""" ret = libvirtmod.virDomainSnapshotListAllChildren(self._o, flags)
Not a big deal, but I think this chunk should be reverted. None of the other classes provide getConnect, and it is inconsistent with how virDomainGetInfo() is converted to virDomain.info() Adding the doc string is useful certainly, but ideally it would be done for all connect() impls. - Cole
On Thu, Jan 24, 2013 at 17:37:31 -0500, Cole Robinson wrote:
On 01/23/2013 06:26 AM, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=895882
virDomainSnapshot.getDomain() and virDomainSnapshot.getConnect() wrappers around virDomainSnapshotGet{Domain,Connect} were not supposed to be ever implemented. The class should contain proper domain() and connect() accessors that fetch python objects stored internally within the class. While domain() was already provided, connect() was missing.
This patch adds connect() method to virDomainSnapshot class and reimplements getDomain() and getConnect() methods as aliases to domain() and connect() for backward compatibility. --- python/generator.py | 4 +++- python/libvirt-override-virDomainSnapshot.py | 8 ++++++++ src/libvirt.c | 10 ++++++++-- 3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/python/libvirt-override-virDomainSnapshot.py b/python/libvirt-override-virDomainSnapshot.py index 3da7bfd..bf708a5 100644 --- a/python/libvirt-override-virDomainSnapshot.py +++ b/python/libvirt-override-virDomainSnapshot.py @@ -1,3 +1,11 @@ + def getConnect(self): + """Get the connection that owns the domain that a snapshot was created for""" + return self.connect() + + def getDomain(self): + """Get the domain that a snapshot was created for""" + return self.domain() + def listAllChildren(self, flags): """List all child snapshots and returns a list of snapshot objects""" ret = libvirtmod.virDomainSnapshotListAllChildren(self._o, flags)
Not a big deal, but I think this chunk should be reverted. None of the other classes provide getConnect, and it is inconsistent with how virDomainGetInfo() is converted to virDomain.info()
You are right, but we have getConnect and getDomain methods in virDomainSnapshot since 0.9.5 (although their implementation was not exactly correct) and thus we need to keep them for backward compatibility. Jirka
On 01/25/2013 11:35 AM, Jiri Denemark wrote:
On Thu, Jan 24, 2013 at 17:37:31 -0500, Cole Robinson wrote:
On 01/23/2013 06:26 AM, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=895882
virDomainSnapshot.getDomain() and virDomainSnapshot.getConnect() wrappers around virDomainSnapshotGet{Domain,Connect} were not supposed to be ever implemented. The class should contain proper domain() and connect() accessors that fetch python objects stored internally within the class. While domain() was already provided, connect() was missing.
This patch adds connect() method to virDomainSnapshot class and reimplements getDomain() and getConnect() methods as aliases to domain() and connect() for backward compatibility. --- python/generator.py | 4 +++- python/libvirt-override-virDomainSnapshot.py | 8 ++++++++ src/libvirt.c | 10 ++++++++-- 3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/python/libvirt-override-virDomainSnapshot.py b/python/libvirt-override-virDomainSnapshot.py index 3da7bfd..bf708a5 100644 --- a/python/libvirt-override-virDomainSnapshot.py +++ b/python/libvirt-override-virDomainSnapshot.py @@ -1,3 +1,11 @@ + def getConnect(self): + """Get the connection that owns the domain that a snapshot was created for""" + return self.connect() + + def getDomain(self): + """Get the domain that a snapshot was created for""" + return self.domain() + def listAllChildren(self, flags): """List all child snapshots and returns a list of snapshot objects""" ret = libvirtmod.virDomainSnapshotListAllChildren(self._o, flags)
Not a big deal, but I think this chunk should be reverted. None of the other classes provide getConnect, and it is inconsistent with how virDomainGetInfo() is converted to virDomain.info()
You are right, but we have getConnect and getDomain methods in virDomainSnapshot since 0.9.5 (although their implementation was not exactly correct) and thus we need to keep them for backward compatibility.
I'm pretty sure using getConnect and getDomain would actually cause crashes, at least that was my experience when I patched out other getConnect instances in the past. If that's the case then they were never usable anyways and IMO can just be dropped, but it's a minor point. Thanks, Cole
participants (3)
-
Cole Robinson -
Eric Blake -
Jiri Denemark