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

Royce Lv lvroyce at linux.vnet.ibm.com
Thu Oct 23 07:32:33 UTC 2014


Reviewed-by: Royce Lv<lvroyce at linux.vnet.ibm.com>
On 2014年10月23日 02:34, 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