Re: [Kimchi-devel] [PATCH v2 2/5] storagepool: rename and consolidate arguments of creating (back end)

On 12/22/2013 08:55 AM, Zhou Zheng Sheng wrote: > As we are adding support to new type of storage pool, the current naming > scheme of the storage pool creating arguments should be rearranged to be > more extendable. This patch renames some arguments and consolidates the > argument of the same purposes as follow. This patch moves storage source > related arguments under "source" key, while "source" itself is a > dictionary. > > nfsserver -> source.host > This is because in future patches, iSCSI pool can use this host info as > well. Other network backed storage pool can also make use of this > argument. > > nfspath -> source.path > This is because other netfs pool can also make use of this argument. > > devices -> source.devices > To differentiate source arguments from the target arguments, we move > this argument under "source" key. > > The patch also adds unit test to check the generated XML is correct. > > Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt; > --- > Makefile.am | 1 + > docs/API.md | 20 ++++++----- > src/kimchi/model.py | 45 ++++++++++++----------- > tests/Makefile.am | 1 + > tests/test_storagepool.py | 92 > +++++++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 130 insertions(+), 29 deletions(-) > create mode 100644 tests/test_storagepool.py > > diff --git a/Makefile.am b/Makefile.am > index e57d3b6..cbdc128 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -46,6 +46,7 @@ PEP8_WHITELIST = \ > plugins/sample/__init__.py \ > plugins/sample/model.py \ > tests/test_plugin.py \ > + tests/test_storagepool.py \ > $(NULL) > > check-local: > diff --git a/docs/API.md b/docs/API.md > index 9edc551..d5c8ff6 100644 > --- a/docs/API.md > +++ b/docs/API.md > @@ -180,15 +180,19 @@ Represents a snapshot of the Virtual Machine's > primary monitor. > > * **GET**: Retrieve a summarized list of all defined Storage Pools > * **POST**: Create a new Storage Pool > - * name: The name of the Storage Pool > - * path: The path of the defined Storage Pool, > + * name: The name of the Storage Pool. > + * type: The type of the defined Storage Pool. > + Supported types: 'dir', 'kimchi-iso', 'netfs', 'logical' > + * path: The path of the defined Storage Pool. > For 'kimchi-iso' pool refers to targeted deep scan path. > - * type: The type of the defined Storage Pool, > - Supported types: 'dir', 'kimchi-iso', 'netfs' > - * nfsserver: IP or hostname of NFS server to create NFS pool. > - * nfspath: export path on nfs server for NFS pool. > - * devices: Array of devices to be used in the Storage Pool > - Exclusive to the 'logical' storage pool type. > + Pool types: 'dir', 'kimchi-iso'. > + * source: Dictionary containing source information of the pool. > + * host: IP or hostname of server for a pool backed from a > remote host. > + Pool types: 'nfs'. It should be 'netfs', ritgh? > + * path: Export path on NFS server for NFS pool. > + Pool types: 'nfs'. It should be 'netfs', ritgh?

> + * devices: Array of devices to be used in the Storage Pool > + Pool types: 'logical'. > > ### Resource: Storage Pool > > diff --git a/src/kimchi/model.py b/src/kimchi/model.py > index 0a497b5..a2fe76c 100644 > --- a/src/kimchi/model.py > +++ b/src/kimchi/model.py > @@ -1437,13 +1437,13 @@ def _get_dir_storagepool_xml(poolArgs): > # type: > # path: > xml = """ > - <pool type='%(type)s'> > - <name>%(name)s</name> > + <pool type='{type}'> > + <name>{name}</name> > <target> > - <path>%(path)s</path> > + <path>{path}</path> > </target> > </pool> > - """ % poolArgs > + """.format(**poolArgs) > return xml > > > @@ -1451,24 +1451,24 @@ def _get_netfs_storagepool_xml(poolArgs): > # Required parameters > # name: > # type: > - # nfsserver: > - # nfspath: > + # source[host]: > + # source[path]: > path = '/var/lib/kimchi/nfs_mount/' + poolArgs['name'] > if not os.path.exists(path): > os.makedirs(path) > poolArgs['path'] = path > xml = """ > - <pool type='%(type)s'> > - <name>%(name)s</name> > + <pool type='{type}'> > + <name>{name}</name> > <source> > - <host name='%(nfsserver)s'/> > - <dir path='%(nfspath)s'/> > + <host name='{source[host]}'/> > + <dir path='{source[path]}'/> > </source> > <target> > - <path>%(path)s</path> > + <path>{path}</path> > </target> > </pool> > - """ % poolArgs > + """.format(**poolArgs) > return xml > > > @@ -1476,32 +1476,35 @@ def _get_logical_storagepool_xml(poolArgs): > # Required parameters > # name: > # type: > - # devices: > + # source[devices]: > path = '/var/lib/kimchi/logical_mount/' + poolArgs['name'] > if not os.path.exists(path): > os.makedirs(path) > > devices = [] > - for device_path in poolArgs['devices']: > + for device_path in poolArgs['source']['devices']: > devices.append('<device path="%s" />' % device_path) > > - poolArgs.update({'devices': ''.join(devices), > - 'path': path}) > + poolArgs['source']['devices'] = ''.join(devices) > + poolArgs['path'] = path > + > xml = """ > - <pool type='%(type)s'> > - <name>%(name)s</name> > + <pool type='{type}'> > + <name>{name}</name> > <source> > - %(devices)s > + {source[devices]} > </source> > <target> > - <path>%(path)s</path> > + <path>{path}</path> > </target> > </pool> > - """ % poolArgs > + """.format(**poolArgs) > return xml > > > def _get_pool_xml(**kwargs): > + # REMIND: When add new pool type, also add the related test in > + # tests/test_storagepool.py > getPoolXml = { > 'dir': _get_dir_storagepool_xml, > 'netfs': _get_netfs_storagepool_xml, > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 5fb00c0..55d3ab4 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -32,6 +32,7 @@ EXTRA_DIST = \ > test_plugin.py \ > test_rest.py \ > test_server.py \ > + test_storagepool.py \ > test_vmtemplate.py \ > utils.py \ > $(NULL) > diff --git a/tests/test_storagepool.py b/tests/test_storagepool.py > new file mode 100644 > index 0000000..5d5a7cd > --- /dev/null > +++ b/tests/test_storagepool.py > @@ -0,0 +1,92 @@ > +# > +# Project Kimchi > +# > +# Copyright IBM, Corp. 2013 > +# > +# Authors: > +# Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt; > +# > +# This library is free software; you can redistribute it and/or > +# modify it under the terms of the GNU Lesser General Public > +# License as published by the Free Software Foundation; either > +# version 2.1 of the License, or (at your option) any later version. > +# > +# This library is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > +# Lesser General Public License for more details. > +# > +# You should have received a copy of the GNU Lesser General Public > +# 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 unittest > + This blank line isn't needed Also organize it in alphabetic order

> +import libxml2 > + 2 lines here

> +import kimchi.model > +import utils > + > + > +class stroagepoolTests(unittest.TestCase): typo: stroagepoolTests

> + def test_get_storagepool_xml(self): > + poolDefs = [ > + {'def': > + {'type': 'dir', > + 'name': 'unitTestDirPool', > + 'path': '/var/temp/images'}, > + 'xml': > + """ > + <pool type='dir'> > + <name>unitTestDirPool</name> > + <target> > + <path>/var/temp/images</path> > + </target> > + </pool> > + """}, > + {'def': > + {'type': 'netfs', > + 'name': 'unitTestNFSPool', > + 'source': {'host': '127.0.0.1', > + 'path': '/var/export'}}, > + 'xml': > + """ > + <pool type='netfs'> > + <name>unitTestNFSPool</name> > + <source> > + <host name='127.0.0.1'/> > + <dir path='/var/export'/> > + </source> > + <target> > + <path>/var/lib/kimchi/nfs_mount/unitTestNFSPool</path> > + </target> > + </pool> > + """}, > + {'def': > + {'type': 'logical', > + 'name': 'unitTestLogicalPool', > + 'source': {'devices': ['/dev/hda', '/dev/hdb']}}, > + 'xml': > + """ > + <pool type='logical'> > + <name>unitTestLogicalPool</name> > + <source> > + <device path="/dev/hda" /> > + <device path="/dev/hdb" /> > + </source> > + <target> > + > <path>/var/lib/kimchi/logical_mount/unitTestLogicalPool</path> > + </target> > + </pool> > + """}] > + > + for poolDef in poolDefs: > + xmlstr = kimchi.model._get_pool_xml(**poolDef['def']) > + with utils.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())