[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