[Kimchi-devel] [PATCH] Update libvirtstoragepool.py to use lxml.builder

22 Oct 2014

Instead of using plan XML text, we should use lxml.builder to avoid any
issue while generating the XML.

Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com>
---
 src/kimchi/model/libvirtstoragepool.py | 177 +++++++++++++--------------------
 tests/test_storagepool.py              |  16 ++-
 2 files changed, 77 insertions(+), 116 deletions(-)

diff --git a/src/kimchi/model/libvirtstoragepool.py b/src/kimchi/model/libvirtstoragepool.py
index d7b49e2..c6deafc 100644
--- a/src/kimchi/model/libvirtstoragepool.py
+++ b/src/kimchi/model/libvirtstoragepool.py
@@ -17,9 +17,10 @@
 # License along with this library; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
 
-import copy
 import os
 import tempfile
+import lxml.etree as ET
+from lxml.builder import E
 
 import libvirt
 
@@ -65,15 +66,10 @@ class DirPoolDef(StoragePoolDef):
         # name:
         # type:
         # path:
-        xml = u"""
-        <pool type='dir'>
-          <name>{name}</name>
-          <target>
-            <path>{path}</path>
-          </target>
-        </pool>
-        """.format(**self.poolArgs)
-        return xml
+        pool = E.pool(type='dir')
+        pool.append(E.name(self.poolArgs['name']))
+        pool.append(E.target(E.path(self.poolArgs['path'])))
+        return ET.tostring(pool, encoding='unicode', pretty_print=True)
 
 
 class NetfsPoolDef(StoragePoolDef):
@@ -120,21 +116,16 @@ class NetfsPoolDef(StoragePoolDef):
         # type:
         # source[host]:
         # source[path]:
-        poolArgs = copy.deepcopy(self.poolArgs)
-        poolArgs['path'] = self.path
-        xml = u"""
-        <pool type='netfs'>
-          <name>{name}</name>
-          <source>
-            <host name='{source[host]}'/>
-            <dir path='{source[path]}'/>
-          </source>
-          <target>
-            <path>{path}</path>
-          </target>
-        </pool>
-        """.format(**poolArgs)
-        return xml
+        pool = E.pool(type='netfs')
+        pool.append(E.name(self.poolArgs['name']))
+
+        source = E.source()
+        source.append(E.host(name=self.poolArgs['source']['host']))
+        source.append(E.dir(path=self.poolArgs['source']['path']))
+
+        pool.append(source)
+        pool.append(E.target(E.path(self.path)))
+        return ET.tostring(pool, encoding='unicode', pretty_print=True)
 
 
 class LogicalPoolDef(StoragePoolDef):
@@ -150,26 +141,16 @@ class LogicalPoolDef(StoragePoolDef):
         # name:
         # type:
         # source[devices]:
-        poolArgs = copy.deepcopy(self.poolArgs)
-        devices = []
-        for device_path in poolArgs['source']['devices']:
-            devices.append('<device path="%s" />' % device_path)
-
-        poolArgs['source']['devices'] = ''.join(devices)
-        poolArgs['path'] = self.path
-
-        xml = u"""
-        <pool type='logical'>
-        <name>{name}</name>
-            <source>
-            {source[devices]}
-            </source>
-        <target>
-            <path>{path}</path>
-        </target>
-        </pool>
-        """.format(**poolArgs)
-        return xml
+        pool = E.pool(type='logical')
+        pool.append(E.name(self.poolArgs['name']))
+
+        source = E.source()
+        for device_path in self.poolArgs['source']['devices']:
+            source.append(E.device(path=device_path))
+
+        pool.append(source)
+        pool.append(E.target(E.path(self.path)))
+        return ET.tostring(pool, encoding='unicode', pretty_print=True)
 
 
 class ScsiPoolDef(StoragePoolDef):
@@ -199,22 +180,17 @@ class ScsiPoolDef(StoragePoolDef):
         # source[adapter][wwnn]:
         # source[adapter][wwpn]:
         # path:
+        pool = E.pool(type='scsi')
+        pool.append(E.name(self.poolArgs['name']))
+
+        adapter = E.adapter(type=self.poolArgs['source']['adapter']['type'])
+        adapter.set('name', self.poolArgs['source']['name'])
+        adapter.set('wwnn', self.poolArgs['source']['adapter']['wwnn'])
+        adapter.set('wwpn', self.poolArgs['source']['adapter']['wwpn'])
 
-        xml = """
-        <pool type='scsi'>
-          <name>{name}</name>
-          <source>
-            <adapter type='{source[adapter][type]}'\
-                     name='{source[name]}'\
-                     wwnn='{source[adapter][wwnn]}'\
-                     wwpn='{source[adapter][wwpn]}'/>
-          </source>
-          <target>
-            <path>{path}</path>
-          </target>
-        </pool>
-        """.format(**self.poolArgs)
-        return xml
+        pool.append(E.source(adapter))
+        pool.append(E.target(E.path(self.poolArgs['path'])))
+        return ET.tostring(pool, encoding='unicode', pretty_print=True)
 
 
 class IscsiPoolDef(StoragePoolDef):
@@ -237,36 +213,19 @@ class IscsiPoolDef(StoragePoolDef):
             virSecret = conn.secretLookupByUsage(
                 libvirt.VIR_SECRET_USAGE_TYPE_ISCSI, self.poolArgs['name'])
         except libvirt.libvirtError:
-            xml = '''
-            <secret ephemeral='no' private='yes'>
-              <description>Secret for iSCSI storage pool {name}</description>
-              <auth type='chap' username='{username}'/>
-              <usage type='iscsi'>
-                <target>{name}</target>
-              </usage>
-            </secret>'''.format(name=self.poolArgs['name'],
-                                username=auth['username'])
-            virSecret = conn.secretDefineXML(xml)
+            secret = E.secret(ephemeral='no', private='yes')
 
-        virSecret.setValue(auth['password'])
-
-    def _format_port(self, poolArgs):
-        try:
-            port = poolArgs['source']['port']
-        except KeyError:
-            return ""
-        return "port='%s'" % port
+            description = E.description('Secret for iSCSI storage pool %s' %
+                                        self.poolArgs['name'])
+            secret.append(description)
+            secret.append(E.auth(type='chap', username=auth['username']))
 
-    def _format_auth(self, poolArgs):
-        try:
-            auth = poolArgs['source']['auth']
-        except KeyError:
-            return ""
+            usage = E.usage(type='iscsi')
+            usage.append(E.target(self.poolArgs['name']))
+            secret.append(usage)
+            virSecret = conn.secretDefineXML(ET.tostring(secret))
 
-        return '''
-        <auth type='chap' username='{username}'>
-          <secret type='iscsi' usage='{name}'/>
-        </auth>'''.format(name=poolArgs['name'], username=auth['username'])
+        virSecret.setValue(auth['password'])
 
     @property
     def xml(self):
@@ -278,22 +237,28 @@ class IscsiPoolDef(StoragePoolDef):
         #
         # Optional parameters
         # source[port]:
-        poolArgs = copy.deepcopy(self.poolArgs)
-        poolArgs['source'].update({'port': self._format_port(poolArgs),
-                                   'auth': self._format_auth(poolArgs)})
-        poolArgs['path'] = '/dev/disk/by-id'
-
-        xml = u"""
-        <pool type='iscsi'>
-          <name>{name}</name>
-          <source>
-            <host name='{source[host]}' {source[port]}/>
-            <device path='{source[target]}'/>
-            {source[auth]}
-          </source>
-          <target>
-            <path>{path}</path>
-          </target>
-        </pool>
-        """.format(**poolArgs)
-        return xml
+        pool = E.pool(type='iscsi')
+        pool.append(E.name(self.poolArgs['name']))
+
+        host = E.host(name=self.poolArgs['source']['host'])
+        port = self.poolArgs['source'].get('port')
+        if port is not None:
+            host.set('port', str(port))
+
+        source = E.source(host)
+        source.append(E.device(path=self.poolArgs['source']['target']))
+
+        source_auth = self.poolArgs['source'].get('auth')
+        if source_auth is not None:
+            auth = E.auth(type='chap')
+            auth.set('username', source_auth['username'])
+
+            secret = E.secret(type='iscsi')
+            secret.set('usage', self.poolArgs['name'])
+            auth.append(secret)
+
+            source.append(auth)
+
+        pool.append(source)
+        pool.append(E.target(E.path('/dev/disk/by-id')))
+        return ET.tostring(pool, encoding='unicode', pretty_print=True)
diff --git a/tests/test_storagepool.py b/tests/test_storagepool.py
index 3fbeed0..ee30e9a 100644
--- a/tests/test_storagepool.py
+++ b/tests/test_storagepool.py
@@ -17,12 +17,11 @@
 # License along with this library; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
-import libxml2
+import lxml.etree as ET
 import unittest
 
 
 from kimchi.model.libvirtstoragepool import StoragePoolDef
-from kimchi.rollbackcontext import RollbackContext
 
 
 class storagepoolTests(unittest.TestCase):
@@ -166,11 +165,8 @@ class storagepoolTests(unittest.TestCase):
         for poolDef in poolDefs:
             defObj = StoragePoolDef.create(poolDef['def'])
             xmlStr = defObj.xml
-            with RollbackContext() as rollback:
-                t1 = libxml2.readDoc(xmlStr, URL='', encoding='UTF-8',
-                                     options=libxml2.XML_PARSE_NOBLANKS)
-                rollback.prependDefer(t1.freeDoc)
-                t2 = libxml2.readDoc(poolDef['xml'], URL='', encoding='UTF-8',
-                                     options=libxml2.XML_PARSE_NOBLANKS)
-                rollback.prependDefer(t2.freeDoc)
-                self.assertEquals(t1.serialize(), t2.serialize())
+
+            parser = ET.XMLParser(remove_blank_text=True)
+            t1 = ET.fromstring(xmlStr, parser)
+            t2 = ET.fromstring(poolDef['xml'], parser)
+            self.assertEquals(ET.tostring(t1), ET.tostring(t2))
-- 
1.9.3

    