[libvirt] [PATCH 0/5] Python fixes

The following series fixes some functional (and cosmetic) issues with the python bindings. Thanks, Cole Cole Robinson (5): python: Don't generate conflicting conn.createXML functions. python: Add a newline between virConnect and virNode for readability. python: Don't generate bindings for vir*Ref python: Use a pure python implementation of 'vir*GetConnect' python: Fix generated virInterface method names python/generator.py | 40 +++++++++++++++++++++++++++++++++++++--- python/virConnect.py | 1 + 2 files changed, 38 insertions(+), 3 deletions(-)

A special case in the generator wasn't doing its job, and duplicate conn.createXML functions were being generated. The bindings diff is: $ diff -rup ../tmp libvirt.py --- ../tmp 2009-09-23 11:57:11.649203000 -0400 +++ libvirt.py 2009-09-23 11:58:15.165391000 -0400 @@ -1079,14 +1079,6 @@ class virConnect: return __tmp def createXML(self, xmlDesc, flags): - """Create a new device on the VM host machine, for example, - virtual HBAs created using vport_create. """ - ret = libvirtmod.virNodeDeviceCreateXML(self._o, xmlDesc, flags) - if ret is None:raise libvirtError('virNodeDeviceCreateXML() failed', conn=self) - __tmp = virNodeDevice(self, _obj=ret) - return __tmp - - def createXML(self, xmlDesc, flags): """Launch a new guest domain, based on an XML description similar to the one returned by virDomainGetXMLDesc() This function may requires privileged access to the hypervisor. @@ -1327,6 +1319,14 @@ class virConnect: __tmp = virNetwork(self, _obj=ret) return __tmp + def nodeDeviceCreateXML(self, xmlDesc, flags): + """Create a new device on the VM host machine, for example, + virtual HBAs created using vport_create. """ + ret = libvirtmod.virNodeDeviceCreateXML(self._o, xmlDesc, flags) + if ret is None:raise libvirtError('virNodeDeviceCreateXML() failed', conn=self) + __tmp = virNodeDevice(self, _obj=ret) + return __tmp + def nodeDeviceLookupByName(self, name): """Lookup a node device by its name. """ ret = libvirtmod.virNodeDeviceLookupByName(self._o, name) Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/python/generator.py b/python/generator.py index ad9c544..2754be0 100755 --- a/python/generator.py +++ b/python/generator.py @@ -790,7 +790,7 @@ def nameFixup(name, classe, type, file): elif name[0:13] == "virNodeDevice": if name[13:16] == "Get": func = string.lower(name[16]) + name[17:] - elif name[13:19] == "Lookup" or name[13:] == "Create": + elif name[13:19] == "Lookup" or name[13:19] == "Create": func = string.lower(name[3]) + name[4:] else: func = string.lower(name[13]) + name[14:] -- 1.6.4.4

In the generated bindings, they are squashed on top of each other. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/virConnect.py | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/python/virConnect.py b/python/virConnect.py index 1fdf548..096a1ba 100644 --- a/python/virConnect.py +++ b/python/virConnect.py @@ -41,3 +41,4 @@ return 0 except AttributeError: pass + -- 1.6.4.4

On Wed, Sep 23, 2009 at 01:01:25PM -0400, Cole Robinson wrote:
In the generated bindings, they are squashed on top of each other.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/virConnect.py | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/python/virConnect.py b/python/virConnect.py index 1fdf548..096a1ba 100644 --- a/python/virConnect.py +++ b/python/virConnect.py @@ -41,3 +41,4 @@ return 0 except AttributeError: pass +
That's probably going to cause make syntax check to complain about a trailing newline. You might be better making the generator insert the newline instead. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On 09/23/2009 01:09 PM, Daniel P. Berrange wrote:
On Wed, Sep 23, 2009 at 01:01:25PM -0400, Cole Robinson wrote:
In the generated bindings, they are squashed on top of each other.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/virConnect.py | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/python/virConnect.py b/python/virConnect.py index 1fdf548..096a1ba 100644 --- a/python/virConnect.py +++ b/python/virConnect.py @@ -41,3 +41,4 @@ return 0 except AttributeError: pass +
That's probably going to cause make syntax check to complain about a trailing newline. You might be better making the generator insert the newline instead.
Daniel
Yes, good call, I didn't think to run syntax-check, and it does complain. I'll work up another patch. Thanks, Cole

They are only for use in implementing the bindings, so shouldn't be exposed to regular API users. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/python/generator.py b/python/generator.py index 2754be0..aea2120 100755 --- a/python/generator.py +++ b/python/generator.py @@ -366,6 +366,16 @@ skip_function = ( 'virConnectDomainEventDeregister', # overridden in virConnect.py 'virSaveLastError', # We have our own python error wrapper 'virFreeError', # Only needed if we use virSaveLastError + + # These have no use for bindings users. + "virConnectRef", + "virDomainRef", + "virInterfaceRef", + "virNetworkRef", + "virNodeDeviceRef", + "virSecretRef", + "virStoragePoolRef", + "virStorageVolRef", ) -- 1.6.4.4

The API docs explictly warn that we shouldn't use the C vir*GetConnect calls in bindings: doing so can close the internal connection pointer and cause things to get screwy. Implement these calls in python. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-) diff --git a/python/generator.py b/python/generator.py index aea2120..f87540b 100755 --- a/python/generator.py +++ b/python/generator.py @@ -376,6 +376,16 @@ skip_function = ( "virSecretRef", "virStoragePoolRef", "virStorageVolRef", + + # This functions shouldn't be called via the bindings (and even the docs + # contain an explicit warning to that effect). The equivalent should be + # implemented in pure python for each class + "virDomainGetConnect", + "virInterfaceGetConnect", + "virNetworkGetConnect", + "virSecretGetConnect", + "virStoragePoolGetConnect", + "virStorageVolGetConnect", ) @@ -669,6 +679,11 @@ classes_destructors = { "virSecret": "virSecretFree" } +class_skip_connect_impl = { + "virConnect" : True +} + + functions_noexcept = { 'virDomainGetID': True, 'virDomainGetName': True, @@ -1080,6 +1095,12 @@ def buildWrappers(): classes_destructors[classname]); classes.write(" self._o = None\n\n"); destruct=classes_destructors[classname] + + if not class_skip_connect_impl.has_key(classname): + # Build python safe 'connect' method + classes.write(" def connect(self):\n") + classes.write(" return self._conn\n\n") + flist = function_classes[classname] flist.sort(functionCompare) oldfile = "" -- 1.6.4.4

A mistake in the generator was causing virInterface methods to be generated with unpredicatable names ('ceUndefine', instead of just 'undefine'). This fixes the method names to match existing convention. Does anyone care if we are breaking API compat? My guess is that no one is using the python interface bindings yet. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/python/generator.py b/python/generator.py index f87540b..b63465b 100755 --- a/python/generator.py +++ b/python/generator.py @@ -789,10 +789,10 @@ def nameFixup(name, classe, type, file): func = name[10:] func = string.lower(func[0:1]) + func[1:] elif name[0:15] == "virInterfaceGet": - func = name[13:] + func = name[15:] func = string.lower(func[0:1]) + func[1:] elif name[0:12] == "virInterface": - func = name[10:] + func = name[12:] func = string.lower(func[0:1]) + func[1:] elif name[0:12] == 'virSecretGet': func = name[12:] @@ -840,6 +840,9 @@ def nameFixup(name, classe, type, file): func = "OSType" if func == "xMLDesc": func = "XMLDesc" + if func == "mACString": + func = "MACString" + return func -- 1.6.4.4

On Wed, Sep 23, 2009 at 01:01:28PM -0400, Cole Robinson wrote:
A mistake in the generator was causing virInterface methods to be generated with unpredicatable names ('ceUndefine', instead of just 'undefine'). This fixes the method names to match existing convention.
Does anyone care if we are breaking API compat? My guess is that no one is using the python interface bindings yet.
Not at all - the current generated names are just plain broken.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/python/generator.py b/python/generator.py index f87540b..b63465b 100755 --- a/python/generator.py +++ b/python/generator.py @@ -789,10 +789,10 @@ def nameFixup(name, classe, type, file): func = name[10:] func = string.lower(func[0:1]) + func[1:] elif name[0:15] == "virInterfaceGet": - func = name[13:] + func = name[15:] func = string.lower(func[0:1]) + func[1:] elif name[0:12] == "virInterface": - func = name[10:] + func = name[12:] func = string.lower(func[0:1]) + func[1:] elif name[0:12] == 'virSecretGet': func = name[12:] @@ -840,6 +840,9 @@ def nameFixup(name, classe, type, file): func = "OSType" if func == "xMLDesc": func = "XMLDesc" + if func == "mACString": + func = "MACString" + return func
Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (2)
-
Cole Robinson
-
Daniel P. Berrange