[Kimchi-devel] [PATCH v4 1/5] storagepool: refactor _get_pool_xml()
Zhou Zheng Sheng
zhshzhou at linux.vnet.ibm.com
Tue Dec 31 06:39:55 UTC 2013
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. It also performs some preparations such as create target directory.
This is OK for now but in future we'll have more types of pool, and add
more validation and preparation code. So it needs a little refactor
In this patch we define StoragePoolDef object along with a factory
methoed named "create". The factory method instantiates DirPoolDef,
NetfsPoolDef or LogicalPoolDef according to the requested pool type.
Then we can call defObj.prepare() to check NFS mount and iSCSI
login/password. We can inspect defObj.xml to get the generated XML for
the respective poolType.
v2 -> v3
Call pool.build() for netfs, dir and logical pool. So do not need to
create dirs any longer. Since NFS and iSCSI pool need validation,
extract a StoragePoolDef object with "prepare" and "get_xml" methods.
v3 -> v4
Turn 'defObj.get_xml()' into a lazily calculated property 'defObj.xml'.
Signed-off-by: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
---
src/kimchi/model.py | 124 +++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 94 insertions(+), 30 deletions(-)
diff --git a/src/kimchi/model.py b/src/kimchi/model.py
index a6790b8..107aa6c 100644
--- a/src/kimchi/model.py
+++ b/src/kimchi/model.py
@@ -994,7 +994,9 @@ class Model(object):
if params['type'] == 'kimchi-iso':
task_id = self._do_deep_scan(params)
- xml = _get_pool_xml(**params)
+ poolDef = StoragePoolDef.create(params)
+ poolDef.prepare()
+ xml = poolDef.xml
except KeyError, key:
raise MissingParameter(key)
@@ -1009,7 +1011,7 @@ class Model(object):
return name
pool = conn.storagePoolDefineXML(xml, 0)
- if params['type'] in ['logical', 'dir']:
+ if params['type'] in ['logical', 'dir', 'netfs']:
pool.build(libvirt.VIR_STORAGE_POOL_BUILD_NEW)
# autostart dir and logical storage pool created from kimchi
pool.setAutostart(1)
@@ -1430,28 +1432,77 @@ class LibvirtVMScreenshot(VMScreenshot):
finally:
os.close(fd)
-def _get_pool_xml(**kwargs):
- # Required parameters
- # name:
- # type:
- # path:
- # nfspath:
- if kwargs['type'] == 'dir':
+
+class StoragePoolDef(object):
+ @classmethod
+ def create(cls, poolArgs):
+ for klass in cls.__subclasses__():
+ if poolArgs['type'] == klass.poolType:
+ return klass(poolArgs)
+ raise OperationFailed('Unsupported pool type: %s' % poolArgs['type'])
+
+ def __init__(self, poolArgs):
+ self.poolArgs = poolArgs
+ self._xml = None
+
+ def prepare(self):
+ ''' Validate pool arguments and perform preparations. Operation which
+ would cause side effect should be put here. Subclasses can optionally
+ override this method, or it always succeeds by default. '''
+ pass
+
+ @property
+ def xml(self):
+ if self._xml is None:
+ self._xml = self._get_xml()
+ return self._xml
+
+ def _get_xml(self):
+ ''' Subclasses have to override this method to actually generate the
+ storage pool XML definition. '''
+ raise OperationFailed('_get_xml is not implemented: %s' % self)
+
+
+class DirPoolDef(StoragePoolDef):
+ poolType = 'dir'
+
+ def _get_xml(self):
+ # Required parameters
+ # name:
+ # type:
+ # path:
xml = """
- <pool type='%(type)s'>
+ <pool type='dir'>
<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})
+ """ % self.poolArgs
+ return xml
+
+
+class NetfsPoolDef(StoragePoolDef):
+ poolType = 'netfs'
+
+ def __init__(self, poolArgs):
+ super(NetfsPoolDef, self).__init__(poolArgs)
+ self.path = '/var/lib/kimchi/nfs_mount/' + self.poolArgs['name']
+
+ def prepare(self):
+ # TODO Verify the NFS export can be actually mounted.
+ pass
+
+ def _get_xml(self):
+ # Required parameters
+ # name:
+ # type:
+ # nfsserver:
+ # nfspath:
+ poolArgs = copy.deepcopy(self.poolArgs)
+ poolArgs['path'] = self.path
xml = """
- <pool type='%(type)s'>
+ <pool type='netfs'>
<name>%(name)s</name>
<source>
<host name='%(nfsserver)s'/>
@@ -1461,13 +1512,31 @@ def _get_pool_xml(**kwargs):
<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)
+ """ % poolArgs
+ return xml
+
+
+class LogicalPoolDef(StoragePoolDef):
+ poolType = 'logical'
+
+ def __init__(self, poolArgs):
+ super(LogicalPoolDef, self).__init__(poolArgs)
+ self.path = '/var/lib/kimchi/logical_mount/' + self.poolArgs['name']
+
+ def _get_xml(self):
+ # Required parameters
+ # name:
+ # type:
+ # devices:
+ poolArgs = copy.deepcopy(self.poolArgs)
+ devices = []
+ for device_path in poolArgs['devices']:
+ devices.append('<device path="%s" />' % device_path)
+
+ poolArgs.update({'devices': ''.join(devices),
+ 'path': self.path})
xml = """
- <pool type='%(type)s'>
+ <pool type='logical'>
<name>%(name)s</name>
<source>
%(devices)s
@@ -1476,14 +1545,9 @@ def _get_pool_xml(**kwargs):
<path>%(path)s</path>
</target>
</pool>
- """
- devices = ''
- for device_path in kwargs['devices']:
- devices += "<device path=\""+device_path+"\" />"
+ """ % poolArgs
+ return xml
- kwargs.update({'devices': devices})
- kwargs.update({'path': path})
- return xml % kwargs
def _get_volume_xml(**kwargs):
# Required parameters
--
1.7.11.7
More information about the Kimchi-devel
mailing list