4:55 a.m.
Hi,
This patch series is about creating iSCSI storage pool. I firstly
refactored a little bit to make the storage pool creation API nicer,
then add unit test for pool XML generation. The front-end is also
changed to suit the new back-end API. Then I implement iSCSI pool XML
generation and also add a test to actually create a iSCSI pool and
delete it.
v1 -> v2
I take Royce's advice and arrange pool source information under a
"source" key. A pool definition is now like the following.
{'type': 'netfs', 'name': 'XXX,
'source': {'host': 'X.X.X.X', 'path':
'/export/path'}}
or
{'type': 'logical', 'name': 'XXX,
'source': {'devices': ['/dev/sdX', '/dev/sdY']}}
Sheldon thinks the pool XML generation code should be put into a
separated file. I also see Bing Bu's email on refactoring.
Ming says we can make the hard coded path constants configurable.
I think in future we may also add code for validating the pool
source, for example, try to mount an NFS export or login iSCSI
target. All these worth an overall refactor so they are not the
scope of this patch series. I think we can do this after the iSCSI
support is ready.
As Pradeep suggested, I add test to create and delete iSCSI storage
pool in test_model.py. It requires an existing iSCSI target on
localhost. You can use targetcli to create one, or it just skips the
test if it detects no target to use.
Thanks for all your review! I want to hear your comments on the new
API schema, so I can make it stable and save time for front-end work.
Zhou Zheng Sheng (5):
storagepool: dispatch _get_pool_xml() to respective
_get_XXX_storagepool_xml() function
storagepool: rename and consolidate arguments of creating (back end)
storagepool: rename and consolidate arguments of creating (front end)
storagepool: Support Creating iSCSI storagepool in model.py
test_model: test creating iSCSI storage pool
Makefile.am | 2 +
docs/API.md | 24 +++--
src/kimchi/Makefile.am | 1 +
src/kimchi/iscsi.py | 57 +++++++++++
src/kimchi/model.py | 160 +++++++++++++++++++++----------
tests/Makefile.am | 1 +
tests/test_model.py | 91 ++++++++++--------
tests/test_storagepool.py | 111 +++++++++++++++++++++
ui/js/src/kimchi.storagepool_add_main.js | 13 ++-
9 files changed, 361 insertions(+), 99 deletions(-)
create mode 100644 src/kimchi/iscsi.py
create mode 100644 tests/test_storagepool.py
--
1.7.11.7
4:55 a.m.
New subject: [PATCH v2 1/5] storagepool: dispatch _get_pool_xml() to respective _get_XXX_storagepool_xml() function
In src/kimchi/model.py, _get_pool_xml() is to generate libvirt storage
pool XML definition from arguments provided by the POST data (a JSON
dict). It has to support various types of pool such as dir, netfs and
logical. Now it uses "if ... elif ... elif" to check the requested type
of pool and go in to the respective branch and generates the correct
XML.
This is OK for now but in future we'll have more types of pool, so in
this patch we turn "if ... elif" into a dispatching dict, and put the
respective XML generating code to separated functions.
Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
---
src/kimchi/model.py | 121 +++++++++++++++++++++++++++++++---------------------
1 file changed, 73 insertions(+), 48 deletions(-)
diff --git a/src/kimchi/model.py b/src/kimchi/model.py
index 3bc5d6d..0a497b5 100644
--- a/src/kimchi/model.py
+++ b/src/kimchi/model.py
@@ -1430,60 +1430,85 @@ class LibvirtVMScreenshot(VMScreenshot):
finally:
os.close(fd)
-def _get_pool_xml(**kwargs):
+
+def _get_dir_storagepool_xml(poolArgs):
# Required parameters
# name:
# type:
# path:
+ xml = """
+ <pool type='%(type)s'>
+ <name>%(name)s</name>
+ <target>
+ <path>%(path)s</path>
+ </target>
+ </pool>
+ """ % poolArgs
+ return xml
+
+
+def _get_netfs_storagepool_xml(poolArgs):
+ # Required parameters
+ # name:
+ # type:
+ # nfsserver:
# nfspath:
- if kwargs['type'] == 'dir':
- xml = """
- <pool type='%(type)s'>
- <name>%(name)s</name>
- <target>
- <path>%(path)s</path>
- </target>
- </pool>
- """
- elif kwargs['type'] == 'netfs':
- path = '/var/lib/kimchi/nfs_mount/' + kwargs['name'];
- if not os.path.exists(path):
- os.makedirs(path)
- kwargs.update({'path': path})
- xml = """
- <pool type='%(type)s'>
- <name>%(name)s</name>
- <source>
- <host name='%(nfsserver)s'/>
- <dir path='%(nfspath)s'/>
- </source>
- <target>
- <path>%(path)s</path>
- </target>
- </pool>
- """
- elif kwargs['type'] == 'logical':
- path = '/var/lib/kimchi/logical_mount/' + kwargs['name'];
- if not os.path.exists(path):
- os.makedirs(path)
- xml = """
- <pool type='%(type)s'>
- <name>%(name)s</name>
- <source>
- %(devices)s
- </source>
- <target>
- <path>%(path)s</path>
- </target>
- </pool>
- """
- devices = ''
- for device_path in kwargs['devices']:
- devices += "<device path=\""+device_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>
+ <source>
+ <host name='%(nfsserver)s'/>
+ <dir path='%(nfspath)s'/>
+ </source>
+ <target>
+ <path>%(path)s</path>
+ </target>
+ </pool>
+ """ % poolArgs
+ return xml
+
+
+def _get_logical_storagepool_xml(poolArgs):
+ # Required parameters
+ # name:
+ # type:
+ # 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']:
+ devices.append('<device path="%s" />' % device_path)
+
+ poolArgs.update({'devices': ''.join(devices),
+ 'path': path})
+ xml = """
+ <pool type='%(type)s'>
+ <name>%(name)s</name>
+ <source>
+ %(devices)s
+ </source>
+ <target>
+ <path>%(path)s</path>
+ </target>
+ </pool>
+ """ % poolArgs
+ return xml
+
+
+def _get_pool_xml(**kwargs):
+ getPoolXml = {
+ 'dir': _get_dir_storagepool_xml,
+ 'netfs': _get_netfs_storagepool_xml,
+ 'logical': _get_logical_storagepool_xml,
+ }[kwargs['type']]
+ return getPoolXml(kwargs)
- kwargs.update({'devices': devices})
- kwargs.update({'path': path})
- return xml % kwargs
def _get_volume_xml(**kwargs):
# Required parameters
--
1.7.11.7
9:04 a.m.
New subject: [PATCH v2 1/5] storagepool: dispatch _get_pool_xml() to respective _get_XXX_storagepool_xml() function
On 12/22/2013 08:55 AM, Zhou Zheng Sheng wrote:
In src/kimchi/model.py, _get_pool_xml() is to generate libvirt
storage
pool XML definition from arguments provided by the POST data (a JSON
dict). It has to support various types of pool such as dir, netfs and
logical. Now it uses "if ... elif ... elif" to check the requested type
of pool and go in to the respective branch and generates the correct
XML.
This is OK for now but in future we'll have more types of pool, so in
this patch we turn "if ... elif" into a dispatching dict, and put the
respective XML generating code to separated functions.
Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
---
src/kimchi/model.py | 121 +++++++++++++++++++++++++++++++---------------------
1 file changed, 73 insertions(+), 48 deletions(-)
diff --git a/src/kimchi/model.py b/src/kimchi/model.py
index 3bc5d6d..0a497b5 100644
--- a/src/kimchi/model.py
+++ b/src/kimchi/model.py
@@ -1430,60 +1430,85 @@ class LibvirtVMScreenshot(VMScreenshot):
finally:
os.close(fd)
-def _get_pool_xml(**kwargs):
+
+def _get_dir_storagepool_xml(poolArgs):
# Required parameters
# name:
# type:
# path:
+ xml = """
+ <pool type='%(type)s'>
As you separated functions according to type, you can define the type on
xml directly
<pool type='dir'>
+ <name>%(name)s</name>
+ <target>
+ <path>%(path)s</path>
+ </target>
+ </pool>
+ """ % poolArgs
+ return xml
+
+
+def _get_netfs_storagepool_xml(poolArgs):
+ # Required parameters
+ # name:
+ # type:
+ # nfsserver:
# nfspath:
- if kwargs['type'] == 'dir':
- xml = """
- <pool type='%(type)s'>
- <name>%(name)s</name>
- <target>
- <path>%(path)s</path>
- </target>
- </pool>
- """
- elif kwargs['type'] == 'netfs':
- path = '/var/lib/kimchi/nfs_mount/' + kwargs['name'];
- if not os.path.exists(path):
- os.makedirs(path)
- kwargs.update({'path': path})
- xml = """
- <pool type='%(type)s'>
- <name>%(name)s</name>
- <source>
- <host name='%(nfsserver)s'/>
- <dir path='%(nfspath)s'/>
- </source>
- <target>
- <path>%(path)s</path>
- </target>
- </pool>
- """
- elif kwargs['type'] == 'logical':
- path = '/var/lib/kimchi/logical_mount/' + kwargs['name'];
- if not os.path.exists(path):
- os.makedirs(path)
- xml = """
- <pool type='%(type)s'>
- <name>%(name)s</name>
- <source>
- %(devices)s
- </source>
- <target>
- <path>%(path)s</path>
- </target>
- </pool>
- """
- devices = ''
- for device_path in kwargs['devices']:
- devices += "<device path=\""+device_path+"\"
/>"
+ path = '/var/lib/kimchi/nfs_mount/' +
poolArgs['name']
+ if not os.path.exists(path):
+ os.makedirs(path)
I think pool.build() creates everything needed to get pool created so
you don't need the code above.
+ poolArgs['path'] = path
+ xml = """
+ <pool type='%(type)s'>
Same as I commented above about the type
<pool type'netfs'>
+ <name>%(name)s</name>
+ <source>
+ <host name='%(nfsserver)s'/>
+ <dir path='%(nfspath)s'/>
+ </source>
+ <target>
+ <path>%(path)s</path>
+ </target>
+ </pool>
+ """ % poolArgs
+ return xml
+
+
+def _get_logical_storagepool_xml(poolArgs):
+ # Required parameters
+ # name:
+ # type:
+ # devices:
+ path = '/var/lib/kimchi/logical_mount/' +
poolArgs['name']
+ if not os.path.exists(path):
+ os.makedirs(path)
+
poo.build() will care about it
+ devices = []
+ for device_path in poolArgs['devices']:
+ devices.append('<device path="%s" />' % device_path)
+
+ poolArgs.update({'devices': ''.join(devices),
+ 'path': path})
+ xml = """
+ <pool type='%(type)s'>
<pool type='logical'>
+ <name>%(name)s</name>
+ <source>
+ %(devices)s
+ </source>
+ <target>
+ <path>%(path)s</path>
+ </target>
+ </pool>
+ """ % poolArgs
+ return xml
+
+
+def _get_pool_xml(**kwargs):
+ getPoolXml = {
+ 'dir': _get_dir_storagepool_xml,
+ 'netfs': _get_netfs_storagepool_xml,
+ 'logical': _get_logical_storagepool_xml,
+ }[kwargs['type']]
+ return getPoolXml(kwargs)
- kwargs.update({'devices': devices})
- kwargs.update({'path': path})
- return xml % kwargs
def _get_volume_xml(**kwargs):
# Required parameters
7:43 p.m.
New subject: [PATCH v2 1/5] storagepool: dispatch _get_pool_xml() to respective _get_XXX_storagepool_xml() function
on 2013/12/26 23:04, Aline Manera wrote:
On 12/22/2013 08:55 AM, Zhou Zheng Sheng wrote:
> In src/kimchi/model.py, _get_pool_xml() is to generate libvirt storage
> pool XML definition from arguments provided by the POST data (a JSON
> dict). It has to support various types of pool such as dir, netfs and
> logical. Now it uses "if ... elif ... elif" to check the requested type
> of pool and go in to the respective branch and generates the correct
> XML.
>
> This is OK for now but in future we'll have more types of pool, so in
> this patch we turn "if ... elif" into a dispatching dict, and put the
> respective XML generating code to separated functions.
>
> Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
> ---
> src/kimchi/model.py | 121
> +++++++++++++++++++++++++++++++---------------------
> 1 file changed, 73 insertions(+), 48 deletions(-)
>
> diff --git a/src/kimchi/model.py b/src/kimchi/model.py
> index 3bc5d6d..0a497b5 100644
> --- a/src/kimchi/model.py
> +++ b/src/kimchi/model.py
> @@ -1430,60 +1430,85 @@ class LibvirtVMScreenshot(VMScreenshot):
> finally:
> os.close(fd)
>
> -def _get_pool_xml(**kwargs):
> +
> +def _get_dir_storagepool_xml(poolArgs):
> # Required parameters
> # name:
> # type:
> # path:
> + xml = """
> + <pool type='%(type)s'>
As you separated functions according to type, you can define the type on
xml directly
<pool type='dir'>
Correct, I'll change this part for dir, netfs and logical pool in v3.
Thanks very much!
> + <name>%(name)s</name>
> + <target>
> + <path>%(path)s</path>
> + </target>
> + </pool>
> + """ % poolArgs
> + return xml
> +
> +
> +def _get_netfs_storagepool_xml(poolArgs):
> + # Required parameters
> + # name:
> + # type:
> + # nfsserver:
> # nfspath:
> - if kwargs['type'] == 'dir':
> - xml = """
> - <pool type='%(type)s'>
> - <name>%(name)s</name>
> - <target>
> - <path>%(path)s</path>
> - </target>
> - </pool>
> - """
> - elif kwargs['type'] == 'netfs':
> - path = '/var/lib/kimchi/nfs_mount/' + kwargs['name'];
> - if not os.path.exists(path):
> - os.makedirs(path)
> - kwargs.update({'path': path})
> - xml = """
> - <pool type='%(type)s'>
> - <name>%(name)s</name>
> - <source>
> - <host name='%(nfsserver)s'/>
> - <dir path='%(nfspath)s'/>
> - </source>
> - <target>
> - <path>%(path)s</path>
> - </target>
> - </pool>
> - """
> - elif kwargs['type'] == 'logical':
> - path = '/var/lib/kimchi/logical_mount/' + kwargs['name'];
> - if not os.path.exists(path):
> - os.makedirs(path)
> - xml = """
> - <pool type='%(type)s'>
> - <name>%(name)s</name>
> - <source>
> - %(devices)s
> - </source>
> - <target>
> - <path>%(path)s</path>
> - </target>
> - </pool>
> - """
> - devices = ''
> - for device_path in kwargs['devices']:
> - devices += "<device path=\""+device_path+"\"
/>"
> + path = '/var/lib/kimchi/nfs_mount/' + poolArgs['name']
> + if not os.path.exists(path):
> + os.makedirs(path)
I think pool.build() creates everything needed to get pool created so
you don't need the code above.
Yes. This is not introduced by this patch series, it's from the existing
code. I'll test it and see if we can remove the mkdirs call.
> + poolArgs['path'] = path
> + xml = """
> + <pool type='%(type)s'>
Same as I commented above about the type
<pool type'netfs'>
> + <name>%(name)s</name>
> + <source>
> + <host name='%(nfsserver)s'/>
> + <dir path='%(nfspath)s'/>
> + </source>
> + <target>
> + <path>%(path)s</path>
> + </target>
> + </pool>
> + """ % poolArgs
> + return xml
> +
> +
> +def _get_logical_storagepool_xml(poolArgs):
> + # Required parameters
> + # name:
> + # type:
> + # devices:
> + path = '/var/lib/kimchi/logical_mount/' + poolArgs['name']
> + if not os.path.exists(path):
> + os.makedirs(path)
> +
poo.build() will care about it
> + devices = []
> + for device_path in poolArgs['devices']:
> + devices.append('<device path="%s" />' %
device_path)
> +
> + poolArgs.update({'devices': ''.join(devices),
> + 'path': path})
> + xml = """
> + <pool type='%(type)s'>
<pool type='logical'>
> + <name>%(name)s</name>
> + <source>
> + %(devices)s
> + </source>
> + <target>
> + <path>%(path)s</path>
> + </target>
> + </pool>
> + """ % poolArgs
> + return xml
> +
> +
> +def _get_pool_xml(**kwargs):
> + getPoolXml = {
> + 'dir': _get_dir_storagepool_xml,
> + 'netfs': _get_netfs_storagepool_xml,
> + 'logical': _get_logical_storagepool_xml,
> + }[kwargs['type']]
> + return getPoolXml(kwargs)
>
> - kwargs.update({'devices': devices})
> - kwargs.update({'path': path})
> - return xml % kwargs
>
> def _get_volume_xml(**kwargs):
> # Required parameters
--
Thanks and best regards!
Zhou Zheng Sheng / 周征晟
E-mail: zhshzhou(a)linux.vnet.ibm.com
Telephone: 86-10-82454397
4:55 a.m.
New subject: [PATCH v2 2/5] storagepool: rename and consolidate arguments of creating (back end)
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>
---
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'.
+ * path: Export path on NFS server for NFS pool.
+ Pool types: 'nfs'.
+ * 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>
+#
+# 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
+
+import libxml2
+
+import kimchi.model
+import utils
+
+
+class stroagepoolTests(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:
+ 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())
--
1.7.11.7
9:17 a.m.
New subject: [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>
---
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>
+#
+# 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())
7:54 p.m.
New subject: [PATCH v2 2/5] storagepool: rename and consolidate arguments of creating (back end)
于 2013年12月26日 23:17, Aline Manera 写道:
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>
> ---
> 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?
Yes. My mistake. I'll correct it. Thanks very much!
> + * 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>
> +#
> +# 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
Thanks. I think it needs a blank line. According to PEP 8[1], it says,
"""
Imports should be grouped in the following order:
standard library imports
related third party imports
local application/library specific imports
You should put a blank line between each group of imports.
""".
unittest is in standard library, and libxml2 is third party library, so
libxml2 comes after unittest and a blank line is needed. Agree?
[1] http://www.python.org/dev/peps/pep-0008/#imports
> +import libxml2
> +
2 lines here
According to PEP 8 mentioned above, one blank line between import
sections is enough.
Is it a kimchi customized rule to organize imports in such a way that
mix standard and third party imports in alphabetic order, and need two
blank lines between standard library and local imports? If this is the
rule I'll follow it, but in my humble opinion, the rule in PEP 8 seems
more reasonable.
> +import kimchi.model
> +import utils
> +
> +
> +class stroagepoolTests(unittest.TestCase):
typo: stroagepoolTests
Ahh! Thanks! I should have be more careful.
> + 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())
--
Thanks and best regards!
Zhou Zheng Sheng / 周征晟
E-mail: zhshzhou(a)linux.vnet.ibm.com
Telephone: 86-10-82454397
6:32 a.m.
New subject: [PATCH v2 2/5] storagepool: rename and consolidate arguments of creating (back end)
On 12/26/2013 11:54 PM, Zhou Zheng Sheng wrote:
于 2013年12月26日 23:17, Aline Manera 写道:
> 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>
>> ---
>> 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?
>
Yes. My mistake. I'll correct it. Thanks very much!
>> + * 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>
>> +#
>> +# 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
>
Thanks. I think it needs a blank line. According to PEP 8[1], it says,
"""
Imports should be grouped in the following order:
standard library imports
related third party imports
local application/library specific imports
You should put a blank line between each group of imports.
""".
unittest is in standard library, and libxml2 is third party library, so
libxml2 comes after unittest and a blank line is needed. Agree?
[1] http://www.python.org/dev/peps/pep-0008/#imports
>> +import libxml2
>> +
> 2 lines here
>
According to PEP 8 mentioned above, one blank line between import
sections is enough.
Is it a kimchi customized rule to organize imports in such a way that
mix standard and third party imports in alphabetic order, and need two
blank lines between standard library and local imports?
Yes! We are using the following rule:
import ...
import ...
import ...
<2 lines>
from ... import ...
from ... import ...
<2 lines>
import kimchi...
from kimchi import ...
<2 lines>
All those blocks must be in alphabetic order
If this is the
rule I'll follow it, but in my humble opinion, the rule in PEP 8 seems
more reasonable.
>> +import kimchi.model
>> +import utils
>> +
>> +
>> +class stroagepoolTests(unittest.TestCase):
> typo: stroagepoolTests
>
Ahh! Thanks! I should have be more careful.
>> + 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())
4:55 a.m.
New subject: [PATCH v2 3/5] storagepool: rename and consolidate arguments of creating (front end)
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.
nfsserver -> source.host
nfspath -> source.path
devices -> source.devices
This patch contains the front end changes of the consolidation work. It
just changes the formData transferred to kimchi backend, and leaves the
templates and other related scripts untouched.
---
ui/js/src/kimchi.storagepool_add_main.js | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/ui/js/src/kimchi.storagepool_add_main.js
b/ui/js/src/kimchi.storagepool_add_main.js
index b31610a..a154933 100644
--- a/ui/js/src/kimchi.storagepool_add_main.js
+++ b/ui/js/src/kimchi.storagepool_add_main.js
@@ -148,14 +148,21 @@ kimchi.addPool = function(event) {
if (poolType === 'dir') {
formData.path = $('#pathId').val();
} else if (poolType === 'logical') {
+ var source = {};
if (!$.isArray(formData.devices)) {
var deviceObj = [];
deviceObj[0] = formData.devices;
- formData.devices = deviceObj;
+ source.devices = deviceObj;
+ } else {
+ source.devices = formData.devices;
}
+ delete formData.devices;
+ formData.source = source;
} else {
- formData.nfspath = $('#nfspathId').val();
- formData.nfsserver = $('#nfsserverId').val();
+ var source = {};
+ source.path = $('#nfspathId').val();
+ source.host = $('#nfsserverId').val();
+ formData.source = source;
}
if (poolType === 'logical') {
var settings = {
--
1.7.11.7
9:18 a.m.
New subject: [PATCH v2 3/5] storagepool: rename and consolidate arguments of creating (front end)
Reviewed-by: Aline Manera <alinefm(a)linux.vnet.ibm.com>
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.
nfsserver -> source.host
nfspath -> source.path
devices -> source.devices
This patch contains the front end changes of the consolidation work. It
just changes the formData transferred to kimchi backend, and leaves the
templates and other related scripts untouched.
---
ui/js/src/kimchi.storagepool_add_main.js | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/ui/js/src/kimchi.storagepool_add_main.js
b/ui/js/src/kimchi.storagepool_add_main.js
index b31610a..a154933 100644
--- a/ui/js/src/kimchi.storagepool_add_main.js
+++ b/ui/js/src/kimchi.storagepool_add_main.js
@@ -148,14 +148,21 @@ kimchi.addPool = function(event) {
if (poolType === 'dir') {
formData.path = $('#pathId').val();
} else if (poolType === 'logical') {
+ var source = {};
if (!$.isArray(formData.devices)) {
var deviceObj = [];
deviceObj[0] = formData.devices;
- formData.devices = deviceObj;
+ source.devices = deviceObj;
+ } else {
+ source.devices = formData.devices;
}
+ delete formData.devices;
+ formData.source = source;
} else {
- formData.nfspath = $('#nfspathId').val();
- formData.nfsserver = $('#nfsserverId').val();
+ var source = {};
+ source.path = $('#nfspathId').val();
+ source.host = $('#nfsserverId').val();
+ formData.source = source;
}
if (poolType === 'logical') {
var settings = {
4:55 a.m.
New subject: [PATCH v2 4/5] storagepool: Support Creating iSCSI storagepool in model.py
This patch implements creating iSCSI storagepool for libvirt.
Each LUN in iSCSI storagepool can be used as a volume, but iSCSI
storagepool does not provide ability to create volume. For now in kimchi
we create volume for each newly created VM. Next is to implement
attaching existing volume to a new VM.
As we are going to support more types of pool, we want to add pool
source validation, and we should also move the XML definition generation
code along with all other pool related code into one or several dedicated
module. This requires mass refactoring. So this patch just skip this
part and add just code into the current framework.
How to test it manually
Prerequisite
An running iSCSI target on the network.
Create:
curl -u root -H 'Content-type: application/json' \
-H 'Accept: application/json' \
-d '{"source": {
"target": "iqn.YouriSCSITargetIQN.XXX",
"host": "10.0.0.X"},
"type": "iscsi",
"name": "iscsipool"}' \
http://127.0.0.1:8000/storagepools
Show Info:
curl -u root -H 'Accept: application/json' \
http://127.0.0.1:8000/storagepools/iscsipool
Activate:
curl -u root -H 'Content-type: application/json' \
-H 'Accept: application/json' \
-d '' http://127.0.0.1:8000/storagepools/iscsipool/activate
Examine:
iscsiadm -m session
Deactivate:
curl -u root -H 'Content-type: application/json' \
-H 'Accept: application/json' \
-d '' http://127.0.0.1:8000/storagepools/iscsipool/deactivate
Delete:
curl -u root -X DELETE -H 'Accept: application/json' \
http://127.0.0.1:8000/storagepools/iscsipool
Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
---
docs/API.md | 8 ++++++--
src/kimchi/model.py | 34 ++++++++++++++++++++++++++++++++++
tests/test_storagepool.py | 19 +++++++++++++++++++
3 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/docs/API.md b/docs/API.md
index d5c8ff6..c1f3938 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -182,17 +182,21 @@ Represents a snapshot of the Virtual Machine's primary monitor.
* **POST**: Create a new Storage Pool
* name: The name of the Storage Pool.
* type: The type of the defined Storage Pool.
- Supported types: 'dir', 'kimchi-iso', 'netfs',
'logical'
+ Supported types: 'dir', 'kimchi-iso', 'netfs',
'logical', 'iscsi'
* path: The path of the defined Storage Pool.
For 'kimchi-iso' pool refers to targeted deep scan path.
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'.
+ Pool types: 'nfs', 'iscsi'.
* path: Export path on NFS server for NFS pool.
Pool types: 'nfs'.
* devices: Array of devices to be used in the Storage Pool
Pool types: 'logical'.
+ * target: Target IQN of an iSCSI pooVl.
+ Pool types: 'iscsi'.
+ * port *(optional)*: Listening port of a remote storage server.
+ Pool types: 'iscsi'.
### Resource: Storage Pool
diff --git a/src/kimchi/model.py b/src/kimchi/model.py
index a2fe76c..f45fe13 100644
--- a/src/kimchi/model.py
+++ b/src/kimchi/model.py
@@ -1502,6 +1502,39 @@ def _get_logical_storagepool_xml(poolArgs):
return xml
+def _get_iscsi_storagepool_xml(poolArgs):
+ # Required parameters
+ # name:
+ # type:
+ # source[host]:
+ # source[target]:
+ #
+ # Optional parameters
+ # source[port]:
+
+ try:
+ portAttr = "port='%s'" %
poolArgs['source']['port']
+ except KeyError:
+ portAttr = ""
+
+ poolArgs['source']['port'] = portAttr
+ poolArgs['path'] = '/dev/disk/by-id'
+
+ xml = """
+ <pool type='{type}'>
+ <name>{name}</name>
+ <source>
+ <host name='{source[host]}' {source[port]}/>
+ <device path='{source[target]}'/>
+ </source>
+ <target>
+ <path>{path}</path>
+ </target>
+ </pool>
+ """.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
@@ -1509,6 +1542,7 @@ def _get_pool_xml(**kwargs):
'dir': _get_dir_storagepool_xml,
'netfs': _get_netfs_storagepool_xml,
'logical': _get_logical_storagepool_xml,
+ 'iscsi': _get_iscsi_storagepool_xml,
}[kwargs['type']]
return getPoolXml(kwargs)
diff --git a/tests/test_storagepool.py b/tests/test_storagepool.py
index 5d5a7cd..9e6f6e3 100644
--- a/tests/test_storagepool.py
+++ b/tests/test_storagepool.py
@@ -78,6 +78,25 @@ class stroagepoolTests(unittest.TestCase):
<path>/var/lib/kimchi/logical_mount/unitTestLogicalPool</path>
</target>
</pool>
+ """},
+ {'def':
+ {'type': 'iscsi',
+ 'name': 'unitTestISCSIPool',
+ 'source': {
+ 'host': '127.0.0.1',
+ 'target':
'iqn.2003-01.org.linux-iscsi.localhost'}},
+ 'xml':
+ """
+ <pool type='iscsi'>
+ <name>unitTestISCSIPool</name>
+ <source>
+ <host name='127.0.0.1' />
+ <device path='iqn.2003-01.org.linux-iscsi.localhost'/>
+ </source>
+ <target>
+ <path>/dev/disk/by-id</path>
+ </target>
+ </pool>
"""}]
for poolDef in poolDefs:
--
1.7.11.7
9:22 a.m.
New subject: [PATCH v2 4/5] storagepool: Support Creating iSCSI storagepool in model.py
On 12/22/2013 08:55 AM, Zhou Zheng Sheng wrote:
This patch implements creating iSCSI storagepool for libvirt.
Each LUN in iSCSI storagepool can be used as a volume, but iSCSI
storagepool does not provide ability to create volume. For now in kimchi
we create volume for each newly created VM. Next is to implement
attaching existing volume to a new VM.
As we are going to support more types of pool, we want to add pool
source validation, and we should also move the XML definition generation
code along with all other pool related code into one or several dedicated
module. This requires mass refactoring. So this patch just skip this
part and add just code into the current framework.
How to test it manually
Prerequisite
An running iSCSI target on the network.
Create:
curl -u root -H 'Content-type: application/json' \
-H 'Accept: application/json' \
-d '{"source": {
"target": "iqn.YouriSCSITargetIQN.XXX",
"host": "10.0.0.X"},
"type": "iscsi",
"name": "iscsipool"}' \
http://127.0.0.1:8000/storagepools
Show Info:
curl -u root -H 'Accept: application/json' \
http://127.0.0.1:8000/storagepools/iscsipool
Activate:
curl -u root -H 'Content-type: application/json' \
-H 'Accept: application/json' \
-d '' http://127.0.0.1:8000/storagepools/iscsipool/activate
Examine:
iscsiadm -m session
Deactivate:
curl -u root -H 'Content-type: application/json' \
-H 'Accept: application/json' \
-d '' http://127.0.0.1:8000/storagepools/iscsipool/deactivate
Delete:
curl -u root -X DELETE -H 'Accept: application/json' \
http://127.0.0.1:8000/storagepools/iscsipool
Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
---
docs/API.md | 8 ++++++--
src/kimchi/model.py | 34 ++++++++++++++++++++++++++++++++++
tests/test_storagepool.py | 19 +++++++++++++++++++
3 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/docs/API.md b/docs/API.md
index d5c8ff6..c1f3938 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -182,17 +182,21 @@ Represents a snapshot of the Virtual Machine's primary
monitor.
* **POST**: Create a new Storage Pool
* name: The name of the Storage Pool.
* type: The type of the defined Storage Pool.
- Supported types: 'dir', 'kimchi-iso', 'netfs',
'logical'
+ Supported types: 'dir', 'kimchi-iso', 'netfs',
'logical', 'iscsi'
* path: The path of the defined Storage Pool.
For 'kimchi-iso' pool refers to targeted deep scan path.
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'.
+ Pool types: 'nfs', 'iscsi'.
* path: Export path on NFS server for NFS pool.
Pool types: 'nfs'.
* devices: Array of devices to be used in the Storage Pool
Pool types: 'logical'.
+ * target: Target IQN of an iSCSI pooVl.
+ Pool types: 'iscsi'.
+ * port *(optional)*: Listening port of a remote storage server.
+ Pool types: 'iscsi'.
### Resource: Storage Pool
diff --git a/src/kimchi/model.py b/src/kimchi/model.py
index a2fe76c..f45fe13 100644
--- a/src/kimchi/model.py
+++ b/src/kimchi/model.py
@@ -1502,6 +1502,39 @@ def _get_logical_storagepool_xml(poolArgs):
return xml
+def _get_iscsi_storagepool_xml(poolArgs):
+ # Required parameters
+ # name:
+ # type:
+ # source[host]:
+ # source[target]:
+ #
+ # Optional parameters
+ # source[port]:
+
+ try:
+ portAttr = "port='%s'" %
poolArgs['source']['port']
+ except KeyError:
+ portAttr = ""
+
+ poolArgs['source']['port'] = portAttr
+ poolArgs['path'] = '/dev/disk/by-id'
+
+ xml = """
+ <pool type='{type}'>
Same as I commented on previous patch.
As you separated functions by type you can use type directly in xml
<pool type='iscsi'>
+ <name>{name}</name>
+ <source>
+ <host name='{source[host]}' {source[port]}/>
+ <device path='{source[target]}'/>
+ </source>
+ <target>
+ <path>{path}</path>
+ </target>
+ </pool>
+ """.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
@@ -1509,6 +1542,7 @@ def _get_pool_xml(**kwargs):
'dir': _get_dir_storagepool_xml,
'netfs': _get_netfs_storagepool_xml,
'logical': _get_logical_storagepool_xml,
+ 'iscsi': _get_iscsi_storagepool_xml,
}[kwargs['type']]
return getPoolXml(kwargs)
diff --git a/tests/test_storagepool.py b/tests/test_storagepool.py
index 5d5a7cd..9e6f6e3 100644
--- a/tests/test_storagepool.py
+++ b/tests/test_storagepool.py
@@ -78,6 +78,25 @@ class stroagepoolTests(unittest.TestCase):
<path>/var/lib/kimchi/logical_mount/unitTestLogicalPool</path>
</target>
</pool>
+ """},
+ {'def':
+ {'type': 'iscsi',
+ 'name': 'unitTestISCSIPool',
+ 'source': {
+ 'host': '127.0.0.1',
+ 'target':
'iqn.2003-01.org.linux-iscsi.localhost'}},
+ 'xml':
+ """
+ <pool type='iscsi'>
+ <name>unitTestISCSIPool</name>
+ <source>
+ <host name='127.0.0.1' />
+ <device path='iqn.2003-01.org.linux-iscsi.localhost'/>
+ </source>
+ <target>
+ <path>/dev/disk/by-id</path>
+ </target>
+ </pool>
"""}]
Add a test with 'port' parameter too.
for poolDef in poolDefs:
7:56 p.m.
New subject: [PATCH v2 4/5] storagepool: Support Creating iSCSI storagepool in model.py
on 201312/26 23:22, Aline Manera wrote:
On 12/22/2013 08:55 AM, Zhou Zheng Sheng wrote:
> This patch implements creating iSCSI storagepool for libvirt.
> Each LUN in iSCSI storagepool can be used as a volume, but iSCSI
> storagepool does not provide ability to create volume. For now in kimchi
> we create volume for each newly created VM. Next is to implement
> attaching existing volume to a new VM.
>
> As we are going to support more types of pool, we want to add pool
> source validation, and we should also move the XML definition generation
> code along with all other pool related code into one or several dedicated
> module. This requires mass refactoring. So this patch just skip this
> part and add just code into the current framework.
>
> How to test it manually
>
> Prerequisite
> An running iSCSI target on the network.
>
> Create:
> curl -u root -H 'Content-type: application/json' \
> -H 'Accept: application/json' \
> -d '{"source": {
> "target": "iqn.YouriSCSITargetIQN.XXX",
> "host": "10.0.0.X"},
> "type": "iscsi",
> "name": "iscsipool"}' \
> http://127.0.0.1:8000/storagepools
>
> Show Info:
> curl -u root -H 'Accept: application/json' \
> http://127.0.0.1:8000/storagepools/iscsipool
>
> Activate:
> curl -u root -H 'Content-type: application/json' \
> -H 'Accept: application/json' \
> -d '' http://127.0.0.1:8000/storagepools/iscsipool/activate
>
> Examine:
> iscsiadm -m session
>
> Deactivate:
> curl -u root -H 'Content-type: application/json' \
> -H 'Accept: application/json' \
> -d '' http://127.0.0.1:8000/storagepools/iscsipool/deactivate
>
> Delete:
> curl -u root -X DELETE -H 'Accept: application/json' \
> http://127.0.0.1:8000/storagepools/iscsipool
>
> Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
> ---
> docs/API.md | 8 ++++++--
> src/kimchi/model.py | 34 ++++++++++++++++++++++++++++++++++
> tests/test_storagepool.py | 19 +++++++++++++++++++
> 3 files changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/docs/API.md b/docs/API.md
> index d5c8ff6..c1f3938 100644
> --- a/docs/API.md
> +++ b/docs/API.md
> @@ -182,17 +182,21 @@ Represents a snapshot of the Virtual Machine's
> primary monitor.
> * **POST**: Create a new Storage Pool
> * name: The name of the Storage Pool.
> * type: The type of the defined Storage Pool.
> - Supported types: 'dir', 'kimchi-iso', 'netfs',
'logical'
> + Supported types: 'dir', 'kimchi-iso', 'netfs',
'logical',
> 'iscsi'
> * path: The path of the defined Storage Pool.
> For 'kimchi-iso' pool refers to targeted deep scan path.
> 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'.
> + Pool types: 'nfs', 'iscsi'.
> * path: Export path on NFS server for NFS pool.
> Pool types: 'nfs'.
> * devices: Array of devices to be used in the Storage Pool
> Pool types: 'logical'.
> + * target: Target IQN of an iSCSI pooVl.
> + Pool types: 'iscsi'.
> + * port *(optional)*: Listening port of a remote storage server.
> + Pool types: 'iscsi'.
>
> ### Resource: Storage Pool
>
> diff --git a/src/kimchi/model.py b/src/kimchi/model.py
> index a2fe76c..f45fe13 100644
> --- a/src/kimchi/model.py
> +++ b/src/kimchi/model.py
> @@ -1502,6 +1502,39 @@ def _get_logical_storagepool_xml(poolArgs):
> return xml
>
>
> +def _get_iscsi_storagepool_xml(poolArgs):
> + # Required parameters
> + # name:
> + # type:
> + # source[host]:
> + # source[target]:
> + #
> + # Optional parameters
> + # source[port]:
> +
> + try:
> + portAttr = "port='%s'" %
poolArgs['source']['port']
> + except KeyError:
> + portAttr = ""
> +
> + poolArgs['source']['port'] = portAttr
> + poolArgs['path'] = '/dev/disk/by-id'
> +
> + xml = """
> + <pool type='{type}'>
Same as I commented on previous patch.
As you separated functions by type you can use type directly in xml
<pool type='iscsi'>
Yes. I'll do it. Thanks for mentioning it!
> + <name>{name}</name>
> + <source>
> + <host name='{source[host]}' {source[port]}/>
> + <device path='{source[target]}'/>
> + </source>
> + <target>
> + <path>{path}</path>
> + </target>
> + </pool>
> + """.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
> @@ -1509,6 +1542,7 @@ def _get_pool_xml(**kwargs):
> 'dir': _get_dir_storagepool_xml,
> 'netfs': _get_netfs_storagepool_xml,
> 'logical': _get_logical_storagepool_xml,
> + 'iscsi': _get_iscsi_storagepool_xml,
> }[kwargs['type']]
> return getPoolXml(kwargs)
>
> diff --git a/tests/test_storagepool.py b/tests/test_storagepool.py
> index 5d5a7cd..9e6f6e3 100644
> --- a/tests/test_storagepool.py
> +++ b/tests/test_storagepool.py
> @@ -78,6 +78,25 @@ class stroagepoolTests(unittest.TestCase):
>
> <path>/var/lib/kimchi/logical_mount/unitTestLogicalPool</path>
> </target>
> </pool>
> + """},
> + {'def':
> + {'type': 'iscsi',
> + 'name': 'unitTestISCSIPool',
> + 'source': {
> + 'host': '127.0.0.1',
> + 'target':
> 'iqn.2003-01.org.linux-iscsi.localhost'}},
> + 'xml':
> + """
> + <pool type='iscsi'>
> + <name>unitTestISCSIPool</name>
> + <source>
> + <host name='127.0.0.1' />
> + <device
path='iqn.2003-01.org.linux-iscsi.localhost'/>
> + </source>
> + <target>
> + <path>/dev/disk/by-id</path>
> + </target>
> + </pool>
> """}]
Add a test with 'port' parameter too.
Correct. I'll add it.
> for poolDef in poolDefs:
--
Thanks and best regards!
Zhou Zheng Sheng / 周征晟
E-mail: zhshzhou(a)linux.vnet.ibm.com
Telephone: 86-10-82454397
9:24 a.m.
New subject: [PATCH v2 4/5] storagepool: Support Creating iSCSI storagepool in model.py
On 12/22/2013 08:55 AM, Zhou Zheng Sheng wrote:
This patch implements creating iSCSI storagepool for libvirt.
Each LUN in iSCSI storagepool can be used as a volume, but iSCSI
storagepool does not provide ability to create volume. For now in kimchi
we create volume for each newly created VM. Next is to implement
attaching existing volume to a new VM.
As we are going to support more types of pool, we want to add pool
source validation, and we should also move the XML definition generation
code along with all other pool related code into one or several dedicated
module. This requires mass refactoring.
I am planning to do that during refactoring backend task.
So this patch just skip this
part and add just code into the current framework.
How to test it manually
Prerequisite
An running iSCSI target on the network.
Create:
curl -u root -H 'Content-type: application/json' \
-H 'Accept: application/json' \
-d '{"source": {
"target": "iqn.YouriSCSITargetIQN.XXX",
"host": "10.0.0.X"},
"type": "iscsi",
"name": "iscsipool"}' \
http://127.0.0.1:8000/storagepools
Show Info:
curl -u root -H 'Accept: application/json' \
http://127.0.0.1:8000/storagepools/iscsipool
Activate:
curl -u root -H 'Content-type: application/json' \
-H 'Accept: application/json' \
-d '' http://127.0.0.1:8000/storagepools/iscsipool/activate
Examine:
iscsiadm -m session
Deactivate:
curl -u root -H 'Content-type: application/json' \
-H 'Accept: application/json' \
-d '' http://127.0.0.1:8000/storagepools/iscsipool/deactivate
Delete:
curl -u root -X DELETE -H 'Accept: application/json' \
http://127.0.0.1:8000/storagepools/iscsipool
Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
---
docs/API.md | 8 ++++++--
src/kimchi/model.py | 34 ++++++++++++++++++++++++++++++++++
tests/test_storagepool.py | 19 +++++++++++++++++++
3 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/docs/API.md b/docs/API.md
index d5c8ff6..c1f3938 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -182,17 +182,21 @@ Represents a snapshot of the Virtual Machine's primary
monitor.
* **POST**: Create a new Storage Pool
* name: The name of the Storage Pool.
* type: The type of the defined Storage Pool.
- Supported types: 'dir', 'kimchi-iso', 'netfs',
'logical'
+ Supported types: 'dir', 'kimchi-iso', 'netfs',
'logical', 'iscsi'
* path: The path of the defined Storage Pool.
For 'kimchi-iso' pool refers to targeted deep scan path.
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'.
+ Pool types: 'nfs', 'iscsi'.
* path: Export path on NFS server for NFS pool.
Pool types: 'nfs'.
* devices: Array of devices to be used in the Storage Pool
Pool types: 'logical'.
+ * target: Target IQN of an iSCSI pooVl.
+ Pool types: 'iscsi'.
+ * port *(optional)*: Listening port of a remote storage server.
+ Pool types: 'iscsi'.
### Resource: Storage Pool
diff --git a/src/kimchi/model.py b/src/kimchi/model.py
index a2fe76c..f45fe13 100644
--- a/src/kimchi/model.py
+++ b/src/kimchi/model.py
@@ -1502,6 +1502,39 @@ def _get_logical_storagepool_xml(poolArgs):
return xml
+def _get_iscsi_storagepool_xml(poolArgs):
+ # Required parameters
+ # name:
+ # type:
+ # source[host]:
+ # source[target]:
+ #
+ # Optional parameters
+ # source[port]:
+
+ try:
+ portAttr = "port='%s'" %
poolArgs['source']['port']
+ except KeyError:
+ portAttr = ""
+
+ poolArgs['source']['port'] = portAttr
+ poolArgs['path'] = '/dev/disk/by-id'
+
+ xml = """
+ <pool type='{type}'>
+ <name>{name}</name>
+ <source>
+ <host name='{source[host]}' {source[port]}/>
+ <device path='{source[target]}'/>
+ </source>
+ <target>
+ <path>{path}</path>
+ </target>
+ </pool>
+ """.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
@@ -1509,6 +1542,7 @@ def _get_pool_xml(**kwargs):
'dir': _get_dir_storagepool_xml,
'netfs': _get_netfs_storagepool_xml,
'logical': _get_logical_storagepool_xml,
+ 'iscsi': _get_iscsi_storagepool_xml,
}[kwargs['type']]
return getPoolXml(kwargs)
diff --git a/tests/test_storagepool.py b/tests/test_storagepool.py
index 5d5a7cd..9e6f6e3 100644
--- a/tests/test_storagepool.py
+++ b/tests/test_storagepool.py
@@ -78,6 +78,25 @@ class stroagepoolTests(unittest.TestCase):
<path>/var/lib/kimchi/logical_mount/unitTestLogicalPool</path>
</target>
</pool>
+ """},
+ {'def':
+ {'type': 'iscsi',
+ 'name': 'unitTestISCSIPool',
+ 'source': {
+ 'host': '127.0.0.1',
+ 'target':
'iqn.2003-01.org.linux-iscsi.localhost'}},
+ 'xml':
+ """
+ <pool type='iscsi'>
+ <name>unitTestISCSIPool</name>
+ <source>
+ <host name='127.0.0.1' />
+ <device path='iqn.2003-01.org.linux-iscsi.localhost'/>
+ </source>
+ <target>
+ <path>/dev/disk/by-id</path>
+ </target>
+ </pool>
"""}]
for poolDef in poolDefs:
4:55 a.m.
New subject: [PATCH v2 5/5] test_model: test creating iSCSI storage pool
This patch adds iSCSI storage pool test to test_modle.test_storagepool.
It firstly checks if there exists an iSCSI target named
"iqn.2013-12.localhost.kimchiUnitTest" on local host. If not, skip the
test; if yes, continue to run the test already defined in test_storagepool.
On RedHat family distributions, libiscsi comes with a Python binding,
but not on other distributions. So in this patch it invokes iscsiadm to
check iSCSI target.
Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
---
Makefile.am | 1 +
src/kimchi/Makefile.am | 1 +
src/kimchi/iscsi.py | 57 +++++++++++++++++++++++++++++++
tests/test_model.py | 91 ++++++++++++++++++++++++++++----------------------
4 files changed, 111 insertions(+), 39 deletions(-)
create mode 100644 src/kimchi/iscsi.py
diff --git a/Makefile.am b/Makefile.am
index cbdc128..d832fcd 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -41,6 +41,7 @@ PEP8_WHITELIST = \
src/kimchi/asynctask.py \
src/kimchi/config.py.in \
src/kimchi/disks.py \
+ src/kimchi/iscsi.py \
src/kimchi/server.py \
plugins/__init__.py \
plugins/sample/__init__.py \
diff --git a/src/kimchi/Makefile.am b/src/kimchi/Makefile.am
index 47a3bd2..02b6fee 100644
--- a/src/kimchi/Makefile.am
+++ b/src/kimchi/Makefile.am
@@ -28,6 +28,7 @@ kimchi_PYTHON = \
distroloader.py \
exception.py \
__init__.py \
+ iscsi.py \
isoinfo.py \
netinfo.py \
network.py \
diff --git a/src/kimchi/iscsi.py b/src/kimchi/iscsi.py
new file mode 100644
index 0000000..96c77fa
--- /dev/null
+++ b/src/kimchi/iscsi.py
@@ -0,0 +1,57 @@
+#
+# Project Kimchi
+#
+# Copyright IBM, Corp. 2013
+#
+# Authors:
+# Zhou Zheng Sheng <zhshzhou(a)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-1301USA
+
+import subprocess
+
+from kimchi.exception import OperationFailed
+
+
+class IscsiAdmin(object):
+ def __init__(self, host, port=None):
+ self.portal = host + ("" if port is None else ":%s" % port)
+
+ def _run_op(self, mode, target, op):
+ iscsiadm = subprocess.Popen(
+ ['iscsiadm', '--mode', mode, '--targetname', target,
+ '--portal', self.portal, '--' + op],
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+ out, err = iscsiadm.communicate()
+ if iscsiadm.returncode != 0:
+ raise OperationFailed('Error executing iscsiadm: %s' % err)
+ return out
+
+ def login(self, target):
+ self._run_op('node', target, 'login')
+
+ def logout(self, target):
+ self._run_op('node', target, 'logout')
+
+
+def validate_iscsi_target(target, host, port=None):
+ adm = IscsiAdmin(host, port)
+ try:
+ adm.login(target)
+ except OperationFailed:
+ return False
+
+ adm.logout(target)
+ return True
diff --git a/tests/test_model.py b/tests/test_model.py
index fb7d6dd..82fa57d 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -34,6 +34,7 @@ import kimchi.model
import kimchi.objectstore
from kimchi.exception import *
from kimchi import netinfo
+from kimchi.iscsi import validate_iscsi_target
import utils
import iso_gen
@@ -112,48 +113,60 @@ class ModelTests(unittest.TestCase):
def test_storagepool(self):
inst = kimchi.model.Model('qemu:///system', self.tmp_store)
- with utils.RollbackContext() as rollback:
- path = '/tmp/kimchi-images'
- name = 'test-pool'
- if not os.path.exists(path):
- os.mkdir(path)
+ poolDefs = [
+ {'type': 'dir',
+ 'name': 'unitTestDirPool',
+ 'path': '/tmp/kimchi-images'},
+ {'type': 'iscsi',
+ 'name': 'unitTestISCSIPool',
+ 'source': {'host': '127.0.0.1',
+ 'target':
'iqn.2013-12.localhost.kimchiUnitTest'}}]
- pools = inst.storagepools_get_list()
- num = len(pools) + 1
+ for poolDef in poolDefs:
+ with utils.RollbackContext() as rollback:
+ path = poolDef.get('path')
+ name = poolDef['name']
+ if not (path is None or os.path.exists(path)):
+ os.mkdir(path)
+
+ if poolDef['type'] == 'iscsi':
+ if not validate_iscsi_target(**poolDef['source']):
+ continue
+
+ pools = inst.storagepools_get_list()
+ num = len(pools) + 1
+
+ inst.storagepools_create(poolDef)
+ rollback.prependDefer(inst.storagepool_delete, name)
+
+ pools = inst.storagepools_get_list()
+ self.assertEquals(num, len(pools))
+
+ poolinfo = inst.storagepool_lookup(name)
+ if path is not None:
+ self.assertEquals(path, poolinfo['path'])
+ self.assertEquals('inactive', poolinfo['state'])
+ if poolinfo['type'] == 'dir':
+ self.assertEquals(True, poolinfo['autostart'])
+ else:
+ self.assertEquals(False, poolinfo['autostart'])
+
+ inst.storagepool_activate(name)
+ rollback.prependDefer(inst.storagepool_deactivate, name)
- args = {'name': name,
- 'path': path,
- 'type': 'dir'}
- inst.storagepools_create(args)
- rollback.prependDefer(inst.storagepool_delete, name)
-
- pools = inst.storagepools_get_list()
- self.assertEquals(num, len(pools))
-
- poolinfo = inst.storagepool_lookup(name)
- self.assertEquals(path, poolinfo['path'])
- self.assertEquals('inactive', poolinfo['state'])
- if poolinfo['type'] == 'dir':
- self.assertEquals(True, poolinfo['autostart'])
- else:
- self.assertEquals(False, poolinfo['autostart'])
-
- inst.storagepool_activate(name)
- rollback.prependDefer(inst.storagepool_deactivate, name)
-
- poolinfo = inst.storagepool_lookup(name)
- self.assertEquals('active', poolinfo['state'])
-
- autostart = poolinfo['autostart']
- ori_params = {'autostart': True} if autostart else
{'autostart': False}
- for i in [True, False]:
- params = {'autostart': i}
- inst.storagepool_update(name, params)
- rollback.prependDefer(inst.storagepool_update, name,
- ori_params)
poolinfo = inst.storagepool_lookup(name)
- self.assertEquals(i, poolinfo['autostart'])
- inst.storagepool_update(name, ori_params)
+ self.assertEquals('active', poolinfo['state'])
+
+ autostart = poolinfo['autostart']
+ ori_params = {'autostart': True} if autostart else
{'autostart': False}
+ for i in [True, False]:
+ params = {'autostart': i}
+ inst.storagepool_update(name, params)
+ rollback.prependDefer(inst.storagepool_update, name,
+ ori_params)
+ poolinfo = inst.storagepool_lookup(name)
+ self.assertEquals(i, poolinfo['autostart'])
+ inst.storagepool_update(name, ori_params)
pools = inst.storagepools_get_list()
self.assertIn('default', pools)
--
1.7.11.7
9:35 a.m.
New subject: [PATCH v2 5/5] test_model: test creating iSCSI storage pool
On 12/22/2013 08:55 AM, Zhou Zheng Sheng wrote:
This patch adds iSCSI storage pool test to
test_modle.test_storagepool.
typo: test_modle
It firstly checks if there exists an iSCSI target named
"iqn.2013-12.localhost.kimchiUnitTest" on local host. If not, skip the
test; if yes, continue to run the test already defined in test_storagepool.
On RedHat family distributions, libiscsi comes with a Python binding,
but not on other distributions. So in this patch it invokes iscsiadm to
check iSCSI target.
Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
---
Makefile.am | 1 +
src/kimchi/Makefile.am | 1 +
src/kimchi/iscsi.py | 57 +++++++++++++++++++++++++++++++
tests/test_model.py | 91 ++++++++++++++++++++++++++++----------------------
4 files changed, 111 insertions(+), 39 deletions(-)
create mode 100644 src/kimchi/iscsi.py
diff --git a/Makefile.am b/Makefile.am
index cbdc128..d832fcd 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -41,6 +41,7 @@ PEP8_WHITELIST = \
src/kimchi/asynctask.py \
src/kimchi/config.py.in \
src/kimchi/disks.py \
+ src/kimchi/iscsi.py \
src/kimchi/server.py \
plugins/__init__.py \
plugins/sample/__init__.py \
diff --git a/src/kimchi/Makefile.am b/src/kimchi/Makefile.am
index 47a3bd2..02b6fee 100644
--- a/src/kimchi/Makefile.am
+++ b/src/kimchi/Makefile.am
@@ -28,6 +28,7 @@ kimchi_PYTHON = \
distroloader.py \
exception.py \
__init__.py \
+ iscsi.py \
isoinfo.py \
netinfo.py \
network.py \
diff --git a/src/kimchi/iscsi.py b/src/kimchi/iscsi.py
new file mode 100644
index 0000000..96c77fa
--- /dev/null
+++ b/src/kimchi/iscsi.py
It is only used for tests so move it under /tests
@@ -0,0 +1,57 @@
+#
+# Project Kimchi
+#
+# Copyright IBM, Corp. 2013
+#
+# Authors:
+# Zhou Zheng Sheng <zhshzhou(a)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-1301USA
+
+import subprocess
+
2 lines
+from kimchi.exception import OperationFailed
+
+
+class IscsiAdmin(object):
+ def __init__(self, host, port=None):
+ self.portal = host + ("" if port is None else ":%s" % port)
+
+ def _run_op(self, mode, target, op):
+ iscsiadm = subprocess.Popen(
+ ['iscsiadm', '--mode', mode, '--targetname',
target,
+ '--portal', self.portal, '--' + op],
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+ out, err = iscsiadm.communicate()
+ if iscsiadm.returncode != 0:
+ raise OperationFailed('Error executing iscsiadm: %s' % err)
+ return out
+
+ def login(self, target):
+ self._run_op('node', target, 'login')
+
+ def logout(self, target):
+ self._run_op('node', target, 'logout')
+
+
+def validate_iscsi_target(target, host, port=None):
+ adm = IscsiAdmin(host, port)
+ try:
+ adm.login(target)
+ except OperationFailed:
+ return False
+
+ adm.logout(target)
+ return True
Let's do a python module with classes or functions - never both
That way we can easily identify modules which implements instances types
or use functions
The function validate_iscsi_target can be into /tests/utils.py module.
diff --git a/tests/test_model.py b/tests/test_model.py
index fb7d6dd..82fa57d 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -34,6 +34,7 @@ import kimchi.model
import kimchi.objectstore
from kimchi.exception import *
from kimchi import netinfo
+from kimchi.iscsi import validate_iscsi_target
import utils
import iso_gen
@@ -112,48 +113,60 @@ class ModelTests(unittest.TestCase):
def test_storagepool(self):
inst = kimchi.model.Model('qemu:///system', self.tmp_store)
- with utils.RollbackContext() as rollback:
- path = '/tmp/kimchi-images'
- name = 'test-pool'
- if not os.path.exists(path):
- os.mkdir(path)
+ poolDefs = [
+ {'type': 'dir',
+ 'name': 'unitTestDirPool',
+ 'path': '/tmp/kimchi-images'},
+ {'type': 'iscsi',
+ 'name': 'unitTestISCSIPool',
+ 'source': {'host': '127.0.0.1',
+ 'target':
'iqn.2013-12.localhost.kimchiUnitTest'}}]
- pools = inst.storagepools_get_list()
- num = len(pools) + 1
+ for poolDef in poolDefs:
+ with utils.RollbackContext() as rollback:
+ path = poolDef.get('path')
+ name = poolDef['name']
+ if not (path is None or os.path.exists(path)):
+ os.mkdir(path)
The backend should be able to create the path if it doesn't exist by
pool.build()
+
+ if poolDef['type'] == 'iscsi':
+ if not validate_iscsi_target(**poolDef['source']):
+ continue
+
+ pools = inst.storagepools_get_list()
+ num = len(pools) + 1
+
+ inst.storagepools_create(poolDef)
+ rollback.prependDefer(inst.storagepool_delete, name)
+
+ pools = inst.storagepools_get_list()
+ self.assertEquals(num, len(pools))
+
+ poolinfo = inst.storagepool_lookup(name)
+ if path is not None:
+ self.assertEquals(path, poolinfo['path'])
+ self.assertEquals('inactive', poolinfo['state'])
+ if poolinfo['type'] == 'dir':
+ self.assertEquals(True, poolinfo['autostart'])
+ else:
+ self.assertEquals(False, poolinfo['autostart'])
+
+ inst.storagepool_activate(name)
+ rollback.prependDefer(inst.storagepool_deactivate, name)
- args = {'name': name,
- 'path': path,
- 'type': 'dir'}
- inst.storagepools_create(args)
- rollback.prependDefer(inst.storagepool_delete, name)
-
- pools = inst.storagepools_get_list()
- self.assertEquals(num, len(pools))
-
- poolinfo = inst.storagepool_lookup(name)
- self.assertEquals(path, poolinfo['path'])
- self.assertEquals('inactive', poolinfo['state'])
- if poolinfo['type'] == 'dir':
- self.assertEquals(True, poolinfo['autostart'])
- else:
- self.assertEquals(False, poolinfo['autostart'])
-
- inst.storagepool_activate(name)
- rollback.prependDefer(inst.storagepool_deactivate, name)
-
- poolinfo = inst.storagepool_lookup(name)
- self.assertEquals('active', poolinfo['state'])
-
- autostart = poolinfo['autostart']
- ori_params = {'autostart': True} if autostart else
{'autostart': False}
- for i in [True, False]:
- params = {'autostart': i}
- inst.storagepool_update(name, params)
- rollback.prependDefer(inst.storagepool_update, name,
- ori_params)
poolinfo = inst.storagepool_lookup(name)
- self.assertEquals(i, poolinfo['autostart'])
- inst.storagepool_update(name, ori_params)
+ self.assertEquals('active', poolinfo['state'])
+
+ autostart = poolinfo['autostart']
+ ori_params = {'autostart': True} if autostart else
{'autostart': False}
+ for i in [True, False]:
+ params = {'autostart': i}
+ inst.storagepool_update(name, params)
+ rollback.prependDefer(inst.storagepool_update, name,
+ ori_params)
+ poolinfo = inst.storagepool_lookup(name)
+ self.assertEquals(i, poolinfo['autostart'])
+ inst.storagepool_update(name, ori_params)
pools = inst.storagepools_get_list()
self.assertIn('default', pools)
10:07 p.m.
New subject: [PATCH v2 5/5] test_model: test creating iSCSI storage pool
on 2013/12/26 23:35, Aline Manera wrote:
On 12/22/2013 08:55 AM, Zhou Zheng Sheng wrote:
> This patch adds iSCSI storage pool test to test_modle.test_storagepool.
typo: test_modle
Get it. Thanks!
> It firstly checks if there exists an iSCSI target named
> "iqn.2013-12.localhost.kimchiUnitTest" on local host. If not, skip the
> test; if yes, continue to run the test already defined in
> test_storagepool.
>
> On RedHat family distributions, libiscsi comes with a Python binding,
> but not on other distributions. So in this patch it invokes iscsiadm to
> check iSCSI target.
>
> Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
> ---
> Makefile.am | 1 +
> src/kimchi/Makefile.am | 1 +
> src/kimchi/iscsi.py | 57 +++++++++++++++++++++++++++++++
> tests/test_model.py | 91
> ++++++++++++++++++++++++++++----------------------
> 4 files changed, 111 insertions(+), 39 deletions(-)
> create mode 100644 src/kimchi/iscsi.py
>
> diff --git a/Makefile.am b/Makefile.am
> index cbdc128..d832fcd 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -41,6 +41,7 @@ PEP8_WHITELIST = \
> src/kimchi/asynctask.py \
> src/kimchi/config.py.in \
> src/kimchi/disks.py \
> + src/kimchi/iscsi.py \
> src/kimchi/server.py \
> plugins/__init__.py \
> plugins/sample/__init__.py \
> diff --git a/src/kimchi/Makefile.am b/src/kimchi/Makefile.am
> index 47a3bd2..02b6fee 100644
> --- a/src/kimchi/Makefile.am
> +++ b/src/kimchi/Makefile.am
> @@ -28,6 +28,7 @@ kimchi_PYTHON = \
> distroloader.py \
> exception.py \
> __init__.py \
> + iscsi.py \
> isoinfo.py \
> netinfo.py \
> network.py \
> diff --git a/src/kimchi/iscsi.py b/src/kimchi/iscsi.py
> new file mode 100644
> index 0000000..96c77fa
> --- /dev/null
> +++ b/src/kimchi/iscsi.py
It is only used for tests so move it under /tests
I plan to validate iscsi backend before creating the iscsi pool. It will
be used in model.py, so in next patch set I'd like to still put iscsi.py
in src/kimchi.
> @@ -0,0 +1,57 @@
> +#
> +# Project Kimchi
> +#
> +# Copyright IBM, Corp. 2013
> +#
> +# Authors:
> +# Zhou Zheng Sheng <zhshzhou(a)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-1301USA
> +
> +import subprocess
> +
2 lines
Get it. Thanks!
> +from kimchi.exception import OperationFailed
> +
> +
> +class IscsiAdmin(object):
> + def __init__(self, host, port=None):
> + self.portal = host + ("" if port is None else ":%s" %
port)
> +
> + def _run_op(self, mode, target, op):
> + iscsiadm = subprocess.Popen(
> + ['iscsiadm', '--mode', mode, '--targetname',
target,
> + '--portal', self.portal, '--' + op],
> + stdout=subprocess.PIPE, stderr=subprocess.PIPE)
> + out, err = iscsiadm.communicate()
> + if iscsiadm.returncode != 0:
> + raise OperationFailed('Error executing iscsiadm: %s' % err)
> + return out
> +
> + def login(self, target):
> + self._run_op('node', target, 'login')
> +
> + def logout(self, target):
> + self._run_op('node', target, 'logout')
> +
> +
> +def validate_iscsi_target(target, host, port=None):
> + adm = IscsiAdmin(host, port)
> + try:
> + adm.login(target)
> + except OperationFailed:
> + return False
> +
> + adm.logout(target)
> + return True
Let's do a python module with classes or functions - never both
That way we can easily identify modules which implements instances types
or use functions
The function validate_iscsi_target can be into /tests/utils.py module.
I do not agree with you. Code should be divided according to the things
they do, not according to they are classes or functions. All iscsi
related things should be put in iscsi.py, regardless of classes or
functions. In Python standard library, there are many examples that a
module contains both classes and functions, as long as they serve
related purpose. Putting all functions in utils.py is a bad idea. You
get a big module with a lot of unrelated functions, and since each
function do different things, you have to import a lot of unrelated
modules in utils.py. This is an anti-pattern of modulization. Ideally,
most classes and functions are divided into modules according to their
logical/functional semantics. Only those small helper functions that can
not fit into any module can be put in utils.py. Having a bit utils.py is
a sign of bad engineered project.
> diff --git a/tests/test_model.py b/tests/test_model.py
> index fb7d6dd..82fa57d 100644
> --- a/tests/test_model.py
> +++ b/tests/test_model.py
> @@ -34,6 +34,7 @@ import kimchi.model
> import kimchi.objectstore
> from kimchi.exception import *
> from kimchi import netinfo
> +from kimchi.iscsi import validate_iscsi_target
> import utils
> import iso_gen
>
> @@ -112,48 +113,60 @@ class ModelTests(unittest.TestCase):
> def test_storagepool(self):
> inst = kimchi.model.Model('qemu:///system', self.tmp_store)
>
> - with utils.RollbackContext() as rollback:
> - path = '/tmp/kimchi-images'
> - name = 'test-pool'
> - if not os.path.exists(path):
> - os.mkdir(path)
> + poolDefs = [
> + {'type': 'dir',
> + 'name': 'unitTestDirPool',
> + 'path': '/tmp/kimchi-images'},
> + {'type': 'iscsi',
> + 'name': 'unitTestISCSIPool',
> + 'source': {'host': '127.0.0.1',
> + 'target':
> 'iqn.2013-12.localhost.kimchiUnitTest'}}]
>
> - pools = inst.storagepools_get_list()
> - num = len(pools) + 1
> + for poolDef in poolDefs:
> + with utils.RollbackContext() as rollback:
> + path = poolDef.get('path')
> + name = poolDef['name']
> + if not (path is None or os.path.exists(path)):
> + os.mkdir(path)
The backend should be able to create the path if it doesn't exist by
pool.build()
OK. Thanks!
> +
> + if poolDef['type'] == 'iscsi':
> + if not validate_iscsi_target(**poolDef['source']):
> + continue
> +
> + pools = inst.storagepools_get_list()
> + num = len(pools) + 1
> +
> + inst.storagepools_create(poolDef)
> + rollback.prependDefer(inst.storagepool_delete, name)
> +
> + pools = inst.storagepools_get_list()
> + self.assertEquals(num, len(pools))
> +
> + poolinfo = inst.storagepool_lookup(name)
> + if path is not None:
> + self.assertEquals(path, poolinfo['path'])
> + self.assertEquals('inactive', poolinfo['state'])
> + if poolinfo['type'] == 'dir':
> + self.assertEquals(True, poolinfo['autostart'])
> + else:
> + self.assertEquals(False, poolinfo['autostart'])
> +
> + inst.storagepool_activate(name)
> + rollback.prependDefer(inst.storagepool_deactivate, name)
>
> - args = {'name': name,
> - 'path': path,
> - 'type': 'dir'}
> - inst.storagepools_create(args)
> - rollback.prependDefer(inst.storagepool_delete, name)
> -
> - pools = inst.storagepools_get_list()
> - self.assertEquals(num, len(pools))
> -
> - poolinfo = inst.storagepool_lookup(name)
> - self.assertEquals(path, poolinfo['path'])
> - self.assertEquals('inactive', poolinfo['state'])
> - if poolinfo['type'] == 'dir':
> - self.assertEquals(True, poolinfo['autostart'])
> - else:
> - self.assertEquals(False, poolinfo['autostart'])
> -
> - inst.storagepool_activate(name)
> - rollback.prependDefer(inst.storagepool_deactivate, name)
> -
> - poolinfo = inst.storagepool_lookup(name)
> - self.assertEquals('active', poolinfo['state'])
> -
> - autostart = poolinfo['autostart']
> - ori_params = {'autostart': True} if autostart else
> {'autostart': False}
> - for i in [True, False]:
> - params = {'autostart': i}
> - inst.storagepool_update(name, params)
> - rollback.prependDefer(inst.storagepool_update, name,
> - ori_params)
> poolinfo = inst.storagepool_lookup(name)
> - self.assertEquals(i, poolinfo['autostart'])
> - inst.storagepool_update(name, ori_params)
> + self.assertEquals('active', poolinfo['state'])
> +
> + autostart = poolinfo['autostart']
> + ori_params = {'autostart': True} if autostart else
> {'autostart': False}
> + for i in [True, False]:
> + params = {'autostart': i}
> + inst.storagepool_update(name, params)
> + rollback.prependDefer(inst.storagepool_update, name,
> + ori_params)
> + poolinfo = inst.storagepool_lookup(name)
> + self.assertEquals(i, poolinfo['autostart'])
> + inst.storagepool_update(name, ori_params)
>
> pools = inst.storagepools_get_list()
> self.assertIn('default', pools)
--
Thanks and best regards!
Zhou Zheng Sheng / 周征晟
E-mail: zhshzhou(a)linux.vnet.ibm.com
Telephone: 86-10-82454397
6:39 a.m.
New subject: [PATCH v2 5/5] test_model: test creating iSCSI storage pool
On 12/27/2013 02:07 AM, Zhou Zheng Sheng wrote:
on 2013/12/26 23:35, Aline Manera wrote:
> On 12/22/2013 08:55 AM, Zhou Zheng Sheng wrote:
>> This patch adds iSCSI storage pool test to test_modle.test_storagepool.
> typo: test_modle
>
Get it. Thanks!
>> It firstly checks if there exists an iSCSI target named
>> "iqn.2013-12.localhost.kimchiUnitTest" on local host. If not, skip the
>> test; if yes, continue to run the test already defined in
>> test_storagepool.
>>
>> On RedHat family distributions, libiscsi comes with a Python binding,
>> but not on other distributions. So in this patch it invokes iscsiadm to
>> check iSCSI target.
>>
>> Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
>> ---
>> Makefile.am | 1 +
>> src/kimchi/Makefile.am | 1 +
>> src/kimchi/iscsi.py | 57 +++++++++++++++++++++++++++++++
>> tests/test_model.py | 91
>> ++++++++++++++++++++++++++++----------------------
>> 4 files changed, 111 insertions(+), 39 deletions(-)
>> create mode 100644 src/kimchi/iscsi.py
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index cbdc128..d832fcd 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -41,6 +41,7 @@ PEP8_WHITELIST = \
>> src/kimchi/asynctask.py \
>> src/kimchi/config.py.in \
>> src/kimchi/disks.py \
>> + src/kimchi/iscsi.py \
>> src/kimchi/server.py \
>> plugins/__init__.py \
>> plugins/sample/__init__.py \
>> diff --git a/src/kimchi/Makefile.am b/src/kimchi/Makefile.am
>> index 47a3bd2..02b6fee 100644
>> --- a/src/kimchi/Makefile.am
>> +++ b/src/kimchi/Makefile.am
>> @@ -28,6 +28,7 @@ kimchi_PYTHON = \
>> distroloader.py \
>> exception.py \
>> __init__.py \
>> + iscsi.py \
>> isoinfo.py \
>> netinfo.py \
>> network.py \
>> diff --git a/src/kimchi/iscsi.py b/src/kimchi/iscsi.py
>> new file mode 100644
>> index 0000000..96c77fa
>> --- /dev/null
>> +++ b/src/kimchi/iscsi.py
> It is only used for tests so move it under /tests
>
I plan to validate iscsi backend before creating the iscsi pool. It will
be used in model.py, so in next patch set I'd like to still put iscsi.py
in src/kimchi.
Ok. Leave it in /src/kimchi
>> @@ -0,0 +1,57 @@
>> +#
>> +# Project Kimchi
>> +#
>> +# Copyright IBM, Corp. 2013
>> +#
>> +# Authors:
>> +# Zhou Zheng Sheng <zhshzhou(a)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-1301USA
>> +
>> +import subprocess
>> +
> 2 lines
>
Get it. Thanks!
>> +from kimchi.exception import OperationFailed
>> +
>> +
>> +class IscsiAdmin(object):
>> + def __init__(self, host, port=None):
>> + self.portal = host + ("" if port is None else ":%s"
% port)
>> +
>> + def _run_op(self, mode, target, op):
>> + iscsiadm = subprocess.Popen(
>> + ['iscsiadm', '--mode', mode, '--targetname',
target,
>> + '--portal', self.portal, '--' + op],
>> + stdout=subprocess.PIPE, stderr=subprocess.PIPE)
>> + out, err = iscsiadm.communicate()
>> + if iscsiadm.returncode != 0:
>> + raise OperationFailed('Error executing iscsiadm: %s' % err)
>> + return out
>> +
>> + def login(self, target):
>> + self._run_op('node', target, 'login')
>> +
>> + def logout(self, target):
>> + self._run_op('node', target, 'logout')
>> +
>> +
>> +def validate_iscsi_target(target, host, port=None):
>> + adm = IscsiAdmin(host, port)
>> + try:
>> + adm.login(target)
>> + except OperationFailed:
>> + return False
>> +
>> + adm.logout(target)
>> + return True
> Let's do a python module with classes or functions - never both
> That way we can easily identify modules which implements instances types
> or use functions
>
> The function validate_iscsi_target can be into /tests/utils.py module.
>
I do not agree with you. Code should be divided according to the things
they do, not according to they are classes or functions. All iscsi
related things should be put in iscsi.py, regardless of classes or
functions. In Python standard library, there are many examples that a
module contains both classes and functions, as long as they serve
related purpose. Putting all functions in utils.py is a bad idea. You
get a big module with a lot of unrelated functions, and since each
function do different things, you have to import a lot of unrelated
modules in utils.py. This is an anti-pattern of modulization. Ideally,
most classes and functions are divided into modules according to their
logical/functional semantics. Only those small helper functions that can
not fit into any module can be put in utils.py. Having a bit utils.py is
a sign of bad engineered project.
So if you want to put all related to iscsi in iscsi.py so move the
function inside the class.
But let's do a module with class or functions.
>> diff --git a/tests/test_model.py b/tests/test_model.py
>> index fb7d6dd..82fa57d 100644
>> --- a/tests/test_model.py
>> +++ b/tests/test_model.py
>> @@ -34,6 +34,7 @@ import kimchi.model
>> import kimchi.objectstore
>> from kimchi.exception import *
>> from kimchi import netinfo
>> +from kimchi.iscsi import validate_iscsi_target
>> import utils
>> import iso_gen
>>
>> @@ -112,48 +113,60 @@ class ModelTests(unittest.TestCase):
>> def test_storagepool(self):
>> inst = kimchi.model.Model('qemu:///system', self.tmp_store)
>>
>> - with utils.RollbackContext() as rollback:
>> - path = '/tmp/kimchi-images'
>> - name = 'test-pool'
>> - if not os.path.exists(path):
>> - os.mkdir(path)
>> + poolDefs = [
>> + {'type': 'dir',
>> + 'name': 'unitTestDirPool',
>> + 'path': '/tmp/kimchi-images'},
>> + {'type': 'iscsi',
>> + 'name': 'unitTestISCSIPool',
>> + 'source': {'host': '127.0.0.1',
>> + 'target':
>> 'iqn.2013-12.localhost.kimchiUnitTest'}}]
>>
>> - pools = inst.storagepools_get_list()
>> - num = len(pools) + 1
>> + for poolDef in poolDefs:
>> + with utils.RollbackContext() as rollback:
>> + path = poolDef.get('path')
>> + name = poolDef['name']
>> + if not (path is None or os.path.exists(path)):
>> + os.mkdir(path)
> The backend should be able to create the path if it doesn't exist by
> pool.build()
>
OK. Thanks!
>> +
>> + if poolDef['type'] == 'iscsi':
>> + if not validate_iscsi_target(**poolDef['source']):
>> + continue
>> +
>> + pools = inst.storagepools_get_list()
>> + num = len(pools) + 1
>> +
>> + inst.storagepools_create(poolDef)
>> + rollback.prependDefer(inst.storagepool_delete, name)
>> +
>> + pools = inst.storagepools_get_list()
>> + self.assertEquals(num, len(pools))
>> +
>> + poolinfo = inst.storagepool_lookup(name)
>> + if path is not None:
>> + self.assertEquals(path, poolinfo['path'])
>> + self.assertEquals('inactive',
poolinfo['state'])
>> + if poolinfo['type'] == 'dir':
>> + self.assertEquals(True, poolinfo['autostart'])
>> + else:
>> + self.assertEquals(False, poolinfo['autostart'])
>> +
>> + inst.storagepool_activate(name)
>> + rollback.prependDefer(inst.storagepool_deactivate, name)
>>
>> - args = {'name': name,
>> - 'path': path,
>> - 'type': 'dir'}
>> - inst.storagepools_create(args)
>> - rollback.prependDefer(inst.storagepool_delete, name)
>> -
>> - pools = inst.storagepools_get_list()
>> - self.assertEquals(num, len(pools))
>> -
>> - poolinfo = inst.storagepool_lookup(name)
>> - self.assertEquals(path, poolinfo['path'])
>> - self.assertEquals('inactive', poolinfo['state'])
>> - if poolinfo['type'] == 'dir':
>> - self.assertEquals(True, poolinfo['autostart'])
>> - else:
>> - self.assertEquals(False, poolinfo['autostart'])
>> -
>> - inst.storagepool_activate(name)
>> - rollback.prependDefer(inst.storagepool_deactivate, name)
>> -
>> - poolinfo = inst.storagepool_lookup(name)
>> - self.assertEquals('active', poolinfo['state'])
>> -
>> - autostart = poolinfo['autostart']
>> - ori_params = {'autostart': True} if autostart else
>> {'autostart': False}
>> - for i in [True, False]:
>> - params = {'autostart': i}
>> - inst.storagepool_update(name, params)
>> - rollback.prependDefer(inst.storagepool_update, name,
>> - ori_params)
>> poolinfo = inst.storagepool_lookup(name)
>> - self.assertEquals(i, poolinfo['autostart'])
>> - inst.storagepool_update(name, ori_params)
>> + self.assertEquals('active', poolinfo['state'])
>> +
>> + autostart = poolinfo['autostart']
>> + ori_params = {'autostart': True} if autostart else
>> {'autostart': False}
>> + for i in [True, False]:
>> + params = {'autostart': i}
>> + inst.storagepool_update(name, params)
>> + rollback.prependDefer(inst.storagepool_update, name,
>> + ori_params)
>> + poolinfo = inst.storagepool_lookup(name)
>> + self.assertEquals(i, poolinfo['autostart'])
>> + inst.storagepool_update(name, ori_params)
>>
>> pools = inst.storagepools_get_list()
>> self.assertIn('default', pools)
3775
days inactive
3780
days old
17 comments
2 participants
participants (2)
-
Aline Manera -
Zhou Zheng Sheng