[Kimchi-devel] [PATCH] Update libvirtstoragepool.py to use lxml.builder
Daniel H Barboza
danielhb at linux.vnet.ibm.com
Tue Oct 28 09:54:09 UTC 2014
Reviewed-by: Daniel Barboza <danielhb at 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 at 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))
More information about the Kimchi-devel
mailing list