[Kimchi-devel] [project-kimchi] [PATCH v1 0/4] Stroage Pool: refactor and add iSCSI support
Sheldon
shaohef at linux.vnet.ibm.com
Mon Dec 23 06:05:47 UTC 2013
wow. cool, Bing Bu.
You should send out this patch easily, then we can avoid so many
repetitions work.
To Aline:
Bing Bu not only implement iscsi and nfs pool, but also implement a test
case for iscsi and nfs pool.
On 12/20/2013 06:54 AM, Bing Bu Cao wrote:
> On 12/16/2013 04:01 PM, Zhou Zheng Sheng wrote:
>> Hi all!
>>
>> I'm implementing iSCSI storage pool support in kimchi. I found that the
>> current design and implementation is not very flexible to add new
>> type of
>> pool.
>>
>> Firstly it uses "if ... elif" to inspect the requested pool type and
>> go into
>> the corresponding branch and create XML. This could be turn into a
>> dispatching
>> dict, with the pool type as the key, and XML generating function as
>> the value.
>>
>> Secondly I think a few arguments for various type of pool can be
>> re-used if
>> they are for the same purpose. For example, both NFS pool and iSCSI
>> pool need
>> a remote host argument, so I renamed the "nfsserver" to "srcHost",
>> and use
>> "srcHost" in both NFS and iSCSI pool. I also change the relative
>> front end
>> code.
>>
>> After the refactoring, I add the iSCSI support. I've tested all the
>> code on
>> Fedora 19 by defining various types of storage pool with various
>> argument.
>>
>> Zhou Zheng Sheng (4):
>> 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
>>
>> docs/API.md | 21 ++--
>> src/kimchi/model.py | 158
>> +++++++++++++++++++++----------
>> ui/js/src/kimchi.storagepool_add_main.js | 9 +-
>> 3 files changed, 128 insertions(+), 60 deletions(-)
>>
> Before I leave kimchi, I written one patch set which refactoring the
> storage pool part.
> But Adam did not allow me to send this patch out because he thought it
> is not important at that moment.
> Whatever, I hope this can give you some other ideas for storage pool
> refactoring. But I am afraid you should make a big rebase because they
> are really so old...
>
>From 6cd5a7900a4f9794c97a1cf86f32128f046bf526 Mon Sep 17 00:00:00 2001
From: Bing Bu Cao <mars at linux.vnet.ibm.com>
Date: Thu, 22 Aug 2013 10:00:00 +0800
Subject: [PATCH 1/4] Generate the storage object XML in storage.py
Signed-off-by: Bing Bu Cao <mars at linux.vnet.ibm.com>
---
src/kimchi/Makefile.am | 1 +
src/kimchi/storage.py | 168
++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 169 insertions(+), 0 deletions(-)
create mode 100644 src/kimchi/storage.py
diff --git a/src/kimchi/Makefile.am b/src/kimchi/Makefile.am
index c5cf5cc..b4e92d4 100644
--- a/src/kimchi/Makefile.am
+++ b/src/kimchi/Makefile.am
@@ -36,6 +36,7 @@ kimchi_PYTHON = \
screenshot.py \
server.py \
sslcert.py \
+ storage.py \
template.py \
vmtemplate.py \
vnc.py \
diff --git a/src/kimchi/storage.py b/src/kimchi/storage.py
new file mode 100644
index 0000000..1114756
--- /dev/null
+++ b/src/kimchi/storage.py
@@ -0,0 +1,168 @@
+#
+# Project Burnet
+#
+# Copyright IBM, Corp. 2013
+#
+# Authors:
+# Bing Bu Cao <mars at linux.vnet.ibm.com>
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
02110-1301 USA
+
+import copy
+import os
+from kimchi.exception import *
+
+STOR_TYPE_POOL = 'pool'
+STOR_TYPE_VOLUME = 'volume'
+STOR_TYPE_LIST = [STOR_TYPE_POOL, STOR_TYPE_VOLUME]
+
+DIR = "dir"
+NETFS = "netfs"
+ISCSI = "iscsi"
+POOL_TYPE_LIST = [DIR, NETFS, ISCSI]
+
+DEFAULT_DIR_TARGET_BASE = "/var/lib/libvirt/images/"
+DEFAULT_SCSI_TARGET = "/dev/disk/by-path"
+
+
+class storage(object):
+ def __init__(self, storage_type, pool_type=None, **kwargs):
+ """
+ @param storage_type: 'pool' or 'volume'
+ @param pool_type: 'pool' or 'volume'
+ @param kwargs:
+ """
+ if storage_type not in STOR_TYPE_LIST:
+ raise InvalidParameter(
+ "Unknown Storage Type: %s" % storage_type)
+ if storage_type == 'volume' and pool_type not in POOL_TYPE_LIST:
+ raise InvalidParameter(
+ "Invalid Pool Type: %s" % pool_type)
+
+ self.pool_type = pool_type
+ self.params = copy.copy(kwargs)
+ self.type = storage_type
+
+ def dumpxml(self):
+ return getattr(self, "_get_%s_xml" % self.type)(self.pool_type)
+
+ def _get_pool_xml(self, pool_type):
+ return getattr(self, "_get_%(type)s_pool_xml" % self.params)()
+
+ def _get_dir_pool_xml(self):
+ # Required parameters
+ # name:
+ # type:
+ # Optional parameters
+ # path:
+ kwargs = self.params
+ kwargs.setdefault('path', DEFAULT_DIR_TARGET_BASE)
+ xml = """
+ <pool type='%(type)s'>
+ <name>%(name)s</name>
+ <target>
+ <path>%(path)s</path>
+ </target>
+ </pool>
+ """ % kwargs
+ return xml
+
+ def _get_netfs_pool_xml(self):
+ # Required parameters
+ # name:
+ # type:
+ # Optional parameters
+ # path:
+ kwargs = self.params
+ formats = ["auto", "nfs", "glusterfs"]
+ kwargs.setdefault('path', os.path.join(DEFAULT_DIR_TARGET_BASE,
+ kwargs['name']))
+ kwargs.setdefault('format', "auto")
+ format = kwargs['format']
+ if not format in formats:
+ raise InvalidParameter(
+ "Unknown Network Filesystem format: %s" %
format)
+ xml = """
+ <pool type='%(type)s'>
+ <name>%(name)s</name>
+ <source>
+ <host name='%(host)s'/>
+ <dir path='%(src_path)s'/>
+ <format type='%(format)s'/>
+ </source>
+ <target>
+ <path>%(path)s</path>
+ </target>
+ </pool>
+ """ % kwargs
+ return xml
+
+ def _get_iscsi_pool_xml(self):
+ # Required parameters
+ # name:
+ # type:
+ # Optional parameters
+ # path:
+ kwargs = self.params
+ kwargs.setdefault('path', DEFAULT_SCSI_TARGET)
+ xml = """
+ <pool type='%(type)s'>
+ <name>%(name)s</name>
+ <source>
+ <host name='%(host)s'/>
+ <device path='%(src_path)s'/>
+ </source>
+ <target>
+ <path>%(path)s</path>
+ </target>
+ </pool>
+ """ % kwargs
+ return xml
+
+ def _get_volume_xml(self, pool_type):
+ # Required parameters
+ # name:
+ # capacity:
+ # path:
+ # Optional parameters
+ # allocation:
+ # format(not used for iscsi pool):
+ if pool_type == ISCSI:
+ raise InvalidOperation(
+ "Creating volume is not supported for iscsi pool")
+ kwargs = self.params
+ kwargs.setdefault('allocation', 0)
+ kwargs.setdefault('format', 'qcow2')
+ target = self._get_vol_target_xml(kwargs.get('format'))
+ kwargs.update({'target': target})
+ xml = """
+ <volume>
+ <name>%(name)s</name>
+ <allocation unit="MiB">%(allocation)s</allocation>
+ <capacity unit="MiB">%(capacity)s</capacity>
+ <source>
+ </source>%(target)s
+ </volume>
+ """ % kwargs
+ return xml
+
+ def _get_vol_target_xml(self, format):
+ target_xml = ""
+ if self.pool_type == DIR:
+ target_xml = """
+ <target>
+ <format type='%s'/>
+ </target>""" % format
+ return target_xml
--
1.7.7.6
>From 93dd8c5d78706d7444ad54cf52845812aa8d7173 Mon Sep 17 00:00:00 2001
From: Bing Bu Cao <mars at linux.vnet.ibm.com>
Date: Thu, 22 Aug 2013 10:02:02 +0800
Subject: [PATCH 2/4] Use new xml generator in model for StoragePool and
StorageVolume operation
Signed-off-by: Bing Bu Cao <mars at linux.vnet.ibm.com>
---
src/kimchi/model.py | 53
+++++++-------------------------------------------
1 files changed, 8 insertions(+), 45 deletions(-)
diff --git a/src/kimchi/model.py b/src/kimchi/model.py
index f05b532..670fb7e 100644
--- a/src/kimchi/model.py
+++ b/src/kimchi/model.py
@@ -54,6 +54,7 @@ from kimchi.exception import *
from kimchi.utils import kimchi_log, is_digit
from kimchi.distroloader import DistroLoader
+from kimchi.storage import storage
ISO_POOL_NAME = u'kimchi_isos'
STATS_INTERVAL = 5
@@ -581,7 +582,8 @@ class Model(object):
name = params['name']
if name in (ISO_POOL_NAME, ):
raise InvalidOperation("StoragePool already exists")
- xml = _get_pool_xml(**params)
+ storpool = storage('pool', **params)
+ xml = storpool.dumpxml()
except KeyError, key:
raise MissingParameter(key)
if name in self.storagepools_get_list():
@@ -689,10 +691,13 @@ class Model(object):
raise
def storagevolumes_create(self, pool, params):
- info = self.storagepool_lookup(pool)
+ pool_obj = self._get_storagepool(pool)
+ pooltype = xmlutils.xpath_get_text(pool_obj.XMLDesc(0),
+ "/pool/@type")[0]
try:
name = params['name']
- xml = _get_volume_xml(**params)
+ storvol = storage('volume', pool_type=pooltype, **params)
+ xml = storvol.dumpxml()
except KeyError, key:
raise MissingParameter(key)
pool = self._get_storagepool(pool)
@@ -826,48 +831,6 @@ class LibvirtVMScreenshot(VMScreenshot):
os.close(fd)
-def _get_pool_xml(**kwargs):
- # Required parameters
- # name:
- # type:
- # path:
- xml = """
- <pool type='%(type)s'>
- <name>%(name)s</name>
- <target>
- <path>%(path)s</path>
- </target>
- </pool>
- """ % kwargs
- return xml
-
-
-def _get_volume_xml(**kwargs):
- # Required parameters
- # name:
- # capacity:
- #
- # Optional:
- # allocation:
- # format:
- kwargs.setdefault('allocation', 0)
- kwargs.setdefault('format', 'qcow2')
-
- xml = """
- <volume>
- <name>%(name)s</name>
- <allocation unit="MiB">%(allocation)s</allocation>
- <capacity unit="MiB">%(capacity)s</capacity>
- <source>
- </source>
- <target>
- <format type='%(format)s'/>
- </target>
- </volume>
- """ % kwargs
- return xml
-
-
class LibvirtConnection(object):
def __init__(self, uri):
self.uri = uri
--
1.7.7.6
>From 21433a52d1b135d696e187d0e29b68d5954749f5 Mon Sep 17 00:00:00 2001
From: Bing Bu Cao <mars at linux.vnet.ibm.com>
Date: Thu, 22 Aug 2013 10:03:39 +0800
Subject: [PATCH 3/4] Add testcases in test_model.py to test the ISCSI
and NFS
storagepool
Signed-off-by: Bing Bu Cao <mars at linux.vnet.ibm.com>
---
tests/test_model.py | 123
+++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 123 insertions(+), 0 deletions(-)
diff --git a/tests/test_model.py b/tests/test_model.py
index dcabbcb..3f6a10e 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -26,6 +26,10 @@ import threading
import os
import time
import tempfile
+import shlex
+import subprocess
+import random
+import shutil
import kimchi.model
import kimchi.objectstore
@@ -256,6 +260,125 @@ class ModelTests(unittest.TestCase):
for key in params.keys():
self.assertEquals(params[key], info[key])
+ def _execute_cmd(self, cmd):
+ print cmd
+ args = shlex.split(cmd)
+ p = subprocess.Popen(args, close_fds=True, stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+ out, err = p.communicate()
+ print out, err
+
+ def _create_iscsi_target(self, path, iqn, capacity):
+ self._execute_cmd("dd if=/dev/zero of=%s bs=1M count=%i" %
(path, capacity))
+ conf = """
+ <target %s>
+ backing-store %s
+ </target>
+ """ % (iqn, path)
+ conf_file = '/etc/tgt/targets.conf'
+ with open(conf_file, 'a') as file:
+ file.write(conf)
+
+ self._execute_cmd("tgt-admin --update %s" % iqn)
+
+ def _get_target(self, iqn):
+ self._execute_cmd("tgt-admin --show")
+
+ def _clean_iscsi_target(self, iqn):
+ self._execute_cmd("tgt-admin --delete %s" % iqn)
+ #remove the lines added before
+ conf = open('/etc/tgt/targets.conf')
+ lines = conf.readlines()
+ conf.close()
+ conf = open('/etc/tgt/targets.conf', 'w')
+ lines = lines[:-4]
+ conf.writelines(lines)
+ conf.close()
+ self._execute_cmd("exportfs -rf")
+
+ @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
+ def test_iscsi_pool(self):
+ inst = kimchi.model.Model('qemu:///system', self.tmp_store)
+ path = '/srv/images'
+ name = 'iscsi-test-pool'
+ img = 'kimchi-%s.img' % random.getrandbits(16)
+ iqn = 'iqn.2013.08.org.foo.bar:%s' % random.getrandbits(16)
+ cap = random.getrandbits(8)
+ if not os.path.exists(path):
+ os.makedirs(path)
+
+ self._create_iscsi_target(os.path.join(path, img), iqn, cap)
+ self._get_target(iqn)
+
+ with utils.RollbackContext() as rollback:
+ args = {'name': name,
+ 'type': 'iscsi',
+ 'host': '127.0.0.1',
+ 'src_path': iqn}
+ inst.storagepools_create(args)
+ rollback.prependDefer(inst.storagepool_delete, name)
+ inst.storagepool_activate(name)
+ rollback.prependDefer(inst.storagepool_deactivate, name)
+ poolinfo = inst.storagepool_lookup(name)
+ self.assertEquals(cap, poolinfo['capacity'])
+
+ self._clean_iscsi_target(iqn)
+ shutil.rmtree(path)
+
+ def _create_nfs(self, path):
+ conf = '%s 127.0.0.1(rw,sync,root_squash)\n' % path
+ conf_file = '/etc/exports'
+ with open(conf_file, 'a') as file:
+ file.write(conf)
+
+ self._execute_cmd("exportfs -rv")
+
+ def _clean_nfs(self, path):
+ #remove the line added in _create_nfs()
+ conf = open('/etc/exports')
+ output = []
+ for line in conf:
+ if not path in line:
+ output.append(line)
+ conf.close()
+ conf = open('/etc/exports', 'w')
+ conf.writelines(output)
+ conf.close()
+ self._execute_cmd("exportfs -rf")
+
+ @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
+ def test_nfs_pool(self):
+ inst = kimchi.model.Model('qemu:///system', self.tmp_store)
+ src_path = '/tmp/nfs-%s' % random.getrandbits(16)
+ src_file = 'kimchi-%s.img' % random.getrandbits(16)
+ name = "pool-nfs-test"
+ cap = 32 * 1024 * 1024
+ if not os.path.exists(src_path):
+ os.makedirs(src_path)
+ fd = open(os.path.join(src_path, src_file), 'w')
+ fd.truncate(cap)
+ self._create_nfs(src_path)
+
+ dst_path = '/mnt/nfs'
+ if not os.path.exists(dst_path):
+ os.makedirs(dst_path)
+ with utils.RollbackContext() as rollback:
+ args = {"type": "netfs",
+ "name": name,
+ "host": "127.0.0.1",
+ "format": "nfs",
+ "src_path": src_path,
+ "path": dst_path}
+ inst.storagepools_create(args)
+ rollback.prependDefer(inst.storagepool_delete, name)
+ inst.storagepool_activate(name)
+ rollback.prependDefer(inst.storagepool_deactivate, name)
+ volinfo = inst.storagevolume_lookup(name, src_file)
+ self.assertEquals(cap >> 20, volinfo['capacity'])
+
+ self._clean_nfs(src_path)
+ shutil.rmtree(src_path)
+
def test_multithreaded_connection(self):
def worker():
for i in xrange(100):
--
1.7.7.6
>From 74c089a3a8174e16f2b03795b5c759334f4ce2ec Mon Sep 17 00:00:00 2001
From: Bing Bu Cao <mars at linux.vnet.ibm.com>
Date: Thu, 22 Aug 2013 10:17:00 +0800
Subject: [PATCH 4/4] Redefine the storagepool 'POST' description in API.md
Signed-off-by: Bing Bu Cao <mars at linux.vnet.ibm.com>
---
docs/API.md | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/docs/API.md b/docs/API.md
index 0c187e0..73d3396 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -163,8 +163,12 @@ 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
+ * path: The target path of the defined Storage Pool
+ * src_path: The source path of the defined Storage pool
* type: The type of the defined Storage Pool
+ * host: The remote server which provide the pool source
+ Only and necessary for netfs and iscsi storage pool
+ * format: The format of the pool, only valid for netfs storage pool
### Resource: Storage Pool
--
1.7.7.6
--
Sheldon Feng(冯少合)<shaohef at linux.vnet.ibm.com>
IBM Linux Technology Center
More information about the Kimchi-devel
mailing list