10:04 a.m.
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