[Kimchi-devel] [project-kimchi] [PATCH v1 1/4] storagepool: dispatch _get_pool_xml() to respective _get_XXX_storagepool_xml() function

[Zhou Zheng Sheng]

on 2013/12/18/ 09:44, Shu Ming wrote:
> 于 2013/12/16 16:01, Zhou Zheng Sheng 写道:
>> 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 at 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 90a9a1b..d22e02d 100644
>> --- a/src/kimchi/model.py
>> +++ b/src/kimchi/model.py
>> @@ -1403,60 +1403,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'/>
> Is it not the intention to replace "nfsserver" with "src_host"?
> 
>> +        <dir path='%(nfspath)s'/>
> 
> Is it not the intention to replace "nfspath" with "src_path"?
> 

Yes. This is in another patch, "storagepool: rename and consolidate
arguments of creating (back end)"

>> +      </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']
> Can we make the path configurable? and make
> 
> '/var/lib/kimchi/*" as the default setting.
> 
> 
I agree, let's do it in another patch.
>> +    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
> 


-- 
Thanks and best regards!

Zhou Zheng Sheng / 周征晟
E-mail: zhshzhou at linux.vnet.ibm.com
Telephone: 86-10-82454397

-- 
project-kimchi mailing list <project-kimchi at googlegroups.com>
https://groups.google.com/forum/#!forum/project-kimchi
--- 
You received this message because you are subscribed to the Google Groups "project-kimchi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to project-kimchi+unsubscribe at googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.