[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]