[Kimchi-devel] [PATCH v2 4/5] storagepool: Support Creating iSCSI storagepool in model.py

[Aline Manera]

On 12/22/2013 08:55 AM, Zhou Zheng Sheng wrote:
> This patch implements creating iSCSI storagepool for libvirt.
> Each LUN in iSCSI storagepool can be used as a volume, but iSCSI
> storagepool does not provide ability to create volume. For now in kimchi
> we create volume for each newly created VM. Next is to implement
> attaching existing volume to a new VM.
>
> As we are going to support more types of pool, we want to add pool
> source validation, and we should also move the XML definition generation
> code along with all other pool related code into one or several dedicated
> module. This requires mass refactoring. So this patch just skip this
> part and add just code into the current framework.
>
> How to test it manually
>
> Prerequisite
> An running iSCSI target on the network.
>
> Create:
> curl -u root -H 'Content-type: application/json' \
>    -H 'Accept: application/json' \
>    -d '{"source": {
>            "target": "iqn.YouriSCSITargetIQN.XXX",
>            "host": "10.0.0.X"},
>         "type": "iscsi",
>         "name": "iscsipool"}' \
>    http://127.0.0.1:8000/storagepools
>
> Show Info:
> curl -u root -H 'Accept: application/json' \
>    http://127.0.0.1:8000/storagepools/iscsipool
>
> Activate:
> curl -u root -H 'Content-type: application/json' \
>    -H 'Accept: application/json' \
>    -d '' http://127.0.0.1:8000/storagepools/iscsipool/activate
>
> Examine:
> iscsiadm -m session
>
> Deactivate:
> curl -u root -H 'Content-type: application/json' \
>    -H 'Accept: application/json' \
>    -d '' http://127.0.0.1:8000/storagepools/iscsipool/deactivate
>
> Delete:
> curl -u root -X DELETE -H 'Accept: application/json' \
>    http://127.0.0.1:8000/storagepools/iscsipool
>
> Signed-off-by: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
> ---
>   docs/API.md               |  8 ++++++--
>   src/kimchi/model.py       | 34 ++++++++++++++++++++++++++++++++++
>   tests/test_storagepool.py | 19 +++++++++++++++++++
>   3 files changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/docs/API.md b/docs/API.md
> index d5c8ff6..c1f3938 100644
> --- a/docs/API.md
> +++ b/docs/API.md
> @@ -182,17 +182,21 @@ Represents a snapshot of the Virtual Machine's primary monitor.
>   * **POST**: Create a new Storage Pool
>       * name: The name of the Storage Pool.
>       * type: The type of the defined Storage Pool.
> -            Supported types: 'dir', 'kimchi-iso', 'netfs', 'logical'
> +            Supported types: 'dir', 'kimchi-iso', 'netfs', 'logical', 'iscsi'
>       * path: The path of the defined Storage Pool.
>               For 'kimchi-iso' pool refers to targeted deep scan path.
>               Pool types: 'dir', 'kimchi-iso'.
>       * source: Dictionary containing source information of the pool.
>           * host: IP or hostname of server for a pool backed from a remote host.
> -                Pool types: 'nfs'.
> +                Pool types: 'nfs', 'iscsi'.
>           * path: Export path on NFS server for NFS pool.
>                   Pool types: 'nfs'.
>           * devices: Array of devices to be used in the Storage Pool
>                      Pool types: 'logical'.
> +        * target: Target IQN of an iSCSI pooVl.
> +                  Pool types: 'iscsi'.
> +        * port *(optional)*: Listening port of a remote storage server.
> +                             Pool types: 'iscsi'.
>
>   ### Resource: Storage Pool
>
> diff --git a/src/kimchi/model.py b/src/kimchi/model.py
> index a2fe76c..f45fe13 100644
> --- a/src/kimchi/model.py
> +++ b/src/kimchi/model.py
> @@ -1502,6 +1502,39 @@ def _get_logical_storagepool_xml(poolArgs):
>       return xml
>
>
> +def _get_iscsi_storagepool_xml(poolArgs):
> +    # Required parameters
> +    # name:
> +    # type:
> +    # source[host]:
> +    # source[target]:
> +    #
> +    # Optional parameters
> +    # source[port]:
> +
> +    try:
> +        portAttr = "port='%s'" % poolArgs['source']['port']
> +    except KeyError:
> +        portAttr = ""
> +
> +    poolArgs['source']['port'] = portAttr
> +    poolArgs['path'] = '/dev/disk/by-id'
> +
> +    xml = """
> +    <pool type='{type}'>

Same as I commented on previous patch.
As you separated functions by type you can use type directly in xml

<pool type='iscsi'>

> +      <name>{name}</name>
> +      <source>
> +        <host name='{source[host]}' {source[port]}/>
> +        <device path='{source[target]}'/>
> +      </source>
> +      <target>
> +        <path>{path}</path>
> +      </target>
> +    </pool>
> +    """.format(**poolArgs)
> +    return xml
> +
> +
>   def _get_pool_xml(**kwargs):
>       # REMIND: When add new pool type, also add the related test in
>       # tests/test_storagepool.py
> @@ -1509,6 +1542,7 @@ def _get_pool_xml(**kwargs):
>           'dir': _get_dir_storagepool_xml,
>           'netfs': _get_netfs_storagepool_xml,
>           'logical': _get_logical_storagepool_xml,
> +        'iscsi': _get_iscsi_storagepool_xml,
>           }[kwargs['type']]
>       return getPoolXml(kwargs)
>
> diff --git a/tests/test_storagepool.py b/tests/test_storagepool.py
> index 5d5a7cd..9e6f6e3 100644
> --- a/tests/test_storagepool.py
> +++ b/tests/test_storagepool.py
> @@ -78,6 +78,25 @@ class stroagepoolTests(unittest.TestCase):
>                    <path>/var/lib/kimchi/logical_mount/unitTestLogicalPool</path>
>                </target>
>                </pool>
> +             """},
> +            {'def':
> +                {'type': 'iscsi',
> +                 'name': 'unitTestISCSIPool',
> +                 'source': {
> +                     'host': '127.0.0.1',
> +                     'target': 'iqn.2003-01.org.linux-iscsi.localhost'}},
> +             'xml':
> +             """
> +             <pool type='iscsi'>
> +               <name>unitTestISCSIPool</name>
> +               <source>
> +                 <host name='127.0.0.1' />
> +                 <device path='iqn.2003-01.org.linux-iscsi.localhost'/>
> +               </source>
> +               <target>
> +                 <path>/dev/disk/by-id</path>
> +               </target>
> +             </pool>
>                """}]

Add a test with 'port' parameter too.

>           for poolDef in poolDefs: