Reviewed-by: Daniel Barboza <danielhb(a)linux.vnet.ibm.com>
On 10/22/2014 04:34 PM, Aline Manera wrote:
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(a)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))