Re: [Kimchi-devel] [project-kimchi] [PATCH v1 0/4] Stroage Pool: refactor and add iSCSI support

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: From: Bing Bu Cao <mars(a)linux.vnet.ibm.com&gt; 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(a)linux.vnet.ibm.com&gt; --- 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(a)linux.vnet.ibm.com&gt; +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +import 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: Bing Bu Cao <mars(a)linux.vnet.ibm.com&gt; 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(a)linux.vnet.ibm.com&gt; --- 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: Bing Bu Cao <mars(a)linux.vnet.ibm.com&gt; 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(a)linux.vnet.ibm.com&gt; --- 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: Bing Bu Cao <mars(a)linux.vnet.ibm.com&gt; 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(a)linux.vnet.ibm.com&gt; --- 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(冯少合)&lt;shaohef(a)linux.vnet.ibm.com&gt; IBM Linux Technology Center