[Kimchi-devel] [PATCH v5 2/5] storagepool: rename and consolidate arguments of creating (back-end)
Sheldon
shaohef at linux.vnet.ibm.com
Fri Jan 3 06:07:55 UTC 2014
Reviewed-by: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
On 01/02/2014 05:58 PM, 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.
>
> v2 -> v3
> test_storagepool.py: fix typo, reorganize import.
>
> v3 -> v4
> Update API.json according to the changes in model.py.
>
> v4 -> v5
> No changes.
>
> Signed-off-by: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
> ---
> Makefile.am | 1 +
> docs/API.md | 20 ++++++----
> src/kimchi/API.json | 45 +++++++++++++++++++++++
> src/kimchi/model.py | 39 +++++++++++---------
> tests/Makefile.am | 1 +
> tests/test_storagepool.py | 93 +++++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 173 insertions(+), 26 deletions(-)
> create mode 100644 tests/test_storagepool.py
>
> diff --git a/Makefile.am b/Makefile.am
> index 1fb3502..01efa1b 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -55,6 +55,7 @@ PEP8_WHITELIST = \
> tests/test_model.py \
> tests/test_plugin.py \
> tests/test_rest.py \
> + tests/test_storagepool.py \
> tests/utils.py \
> $(NULL)
>
> diff --git a/docs/API.md b/docs/API.md
> index 9edc551..a8b9ea0 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: 'netfs'.
> + * path: Export path on NFS server for NFS pool.
> + Pool types: 'netfs'.
> + * devices: Array of devices to be used in the Storage Pool
> + Pool types: 'logical'.
>
> ### Resource: Storage Pool
>
> diff --git a/src/kimchi/API.json b/src/kimchi/API.json
> index 7b90826..6519c21 100644
> --- a/src/kimchi/API.json
> +++ b/src/kimchi/API.json
> @@ -4,6 +4,51 @@
> "description": "Json schema for Kimchi API",
> "type": "object",
> "properties": {
> + "storagepools_create": {
> + "type": "object",
> + "properties": {
> + "name": {
> + "description": "The name of the Storage Pool",
> + "type": "string",
> + "minLength": 1,
> + "required": true
> + },
> + "type": {
> + "description": "The type of the defined Storage Pool",
> + "type": "string",
> + "pattern": "^dir|netfs|logical$",
> + "required": true
> + },
> + "path": {
> + "description": "The path of the defined Storage Pool",
> + "type": "string"
> + },
> + "source": {
> + "description": "Dictionary containing source information of the pool",
> + "type": "object",
> + "properties": {
> + "host": {
> + "description": "IP or hostname of server for a pool backed from a remote host",
> + "type": "string"
> + },
> + "path": {
> + "description": "Export path on NFS server for NFS pool",
> + "type": "string"
> + },
> + "devices": {
> + "description": "Array of devices to be used in the Storage Pool",
> + "type": "array",
> + "minItems": 1,
> + "uniqueItems": true,
> + "items": {
> + "description": "Full path of the block device node",
> + "type": "string"
> + }
> + }
> + }
> + }
> + }
> + },
> "vms_create": {
> "type": "object",
> "properties": {
> diff --git a/src/kimchi/model.py b/src/kimchi/model.py
> index 269fc97..2b46121 100644
> --- a/src/kimchi/model.py
> +++ b/src/kimchi/model.py
> @@ -1455,6 +1455,8 @@ class StoragePoolDef(object):
> ''' Subclasses have to override this method to actually generate the
> storage pool XML definition. Should cause no side effect and be
> idempotent'''
> + # TODO: When add new pool type, should also add the related test in
> + # tests/test_storagepool.py
> raise OperationFailed('self.xml is not implemented: %s' % self)
>
>
> @@ -1469,12 +1471,12 @@ class DirPoolDef(StoragePoolDef):
> # path:
> xml = """
> <pool type='dir'>
> - <name>%(name)s</name>
> + <name>{name}</name>
> <target>
> - <path>%(path)s</path>
> + <path>{path}</path>
> </target>
> </pool>
> - """ % self.poolArgs
> + """.format(**self.poolArgs)
> return xml
>
>
> @@ -1494,22 +1496,22 @@ class NetfsPoolDef(StoragePoolDef):
> # Required parameters
> # name:
> # type:
> - # nfsserver:
> - # nfspath:
> + # source[host]:
> + # source[path]:
> poolArgs = copy.deepcopy(self.poolArgs)
> poolArgs['path'] = self.path
> xml = """
> <pool type='netfs'>
> - <name>%(name)s</name>
> + <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
>
>
> @@ -1525,25 +1527,26 @@ class LogicalPoolDef(StoragePoolDef):
> # Required parameters
> # name:
> # type:
> - # devices:
> + # source[devices]:
> poolArgs = copy.deepcopy(self.poolArgs)
> 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': self.path})
> + poolArgs['source']['devices'] = ''.join(devices)
> + poolArgs['path'] = self.path
> +
> xml = """
> <pool type='logical'>
> - <name>%(name)s</name>
> + <name>{name}</name>
> <source>
> - %(devices)s
> + {source[devices]}
> </source>
> <target>
> - <path>%(path)s</path>
> + <path>{path}</path>
> </target>
> </pool>
> - """ % poolArgs
> + """.format(**poolArgs)
> return 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..1a7786f
> --- /dev/null
> +++ b/tests/test_storagepool.py
> @@ -0,0 +1,93 @@
> +#
> +# Project Kimchi
> +#
> +# Copyright IBM, Corp. 2013
> +#
> +# Authors:
> +# Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
> +#
> +# 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 libxml2
> +import unittest
> +
> +
> +import kimchi.model
> +from kimchi.rollbackcontext import RollbackContext
> +
> +
> +class storagepoolTests(unittest.TestCase):
> + 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:
> + defObj = kimchi.model.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())
--
Thanks and best regards!
Sheldon Feng(冯少合)<shaohef at linux.vnet.ibm.com>
IBM Linux Technology Center
More information about the Kimchi-devel
mailing list