Re: [libvirt] [PATCH v2] python: Fix bindings for virDomainSnapshotGet{Domain, Connect}

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.