[Kimchi-devel] [PATCH V2 1/7] (WIP) Storagepool SCSI/FC: SCSI/Fibre Channel backend implementation

Aline Manera alinefm at linux.vnet.ibm.com
Sat Jan 25 22:26:56 UTC 2014


On 01/23/2014 10:29 PM, Rodrigo Trujillo wrote:
> This patch creates basic functions that allow kimchi users to create
> an libvirt SCSI storagepool using the rest API.
>
> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
> ---
>   docs/API.md         |  6 ++++++
>   src/kimchi/API.json | 19 +++++++++++++++++-
>   src/kimchi/model.py | 57 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>   3 files changed, 76 insertions(+), 6 deletions(-)
>
> diff --git a/docs/API.md b/docs/API.md
> index a86b884..d00cdd5 100644
> --- a/docs/API.md
> +++ b/docs/API.md
> @@ -242,6 +242,12 @@ Represents a snapshot of the Virtual Machine's primary monitor.
>                                Pool types: 'iscsi'.
>               * username: Login username of the iSCSI target.
>               * password: Login password of the iSCSI target.

> +        * adapter_type: Host adapter type: 'scsi_host' or 'fc_host'.
> +        * adapter_name: Scsi host name.
> +                        Required if 'scsi_host' type is selected.
> +        * wwnn: Word Wide Node Name.
> +        * wwpn: Word Wide Port Name.
> +

 From my view, only adapter_name is required to create a scsi pool. So 
all others should be marked as optional
The user just need to provide the scsi host and kimchi will get all 
those info based on that

>   ### Resource: Storage Pool
>
> diff --git a/src/kimchi/API.json b/src/kimchi/API.json
> index e942824..b7eb27b 100644
> --- a/src/kimchi/API.json
> +++ b/src/kimchi/API.json
> @@ -37,7 +37,8 @@
>                   "type": {
>                       "description": "The type of the defined Storage Pool",
>                       "type": "string",

> -                    "pattern": "^dir|netfs|logical|kimchi-iso$",
> +                    "pattern": "^dir|netfs|logical|kimchi-iso|scsi$",
> +                    "pattern": "^dir|netfs|logical|scsi$",

Why are there 2 pattern entries there?

>                       "required": true
>                   },
>                   "path": {
> @@ -76,6 +77,22 @@
>                               "minimum": 1,
>                               "maximum": 65535
>                           },
> +                        "adapter_type": {
> +                            "description": "Host adapter type: 'scsi_host' or 'fc_host'",
> +                            "type": "string"
> +                        },

If there are only 2 possible values you should use 'pattern' to only 
allow them (instead of string)

> +                        "adapter_name": {
> +                            "description": "SCSI host name",
> +                            "type": "string"
> +                        },
> +                        "wwnn": {
> +                            "description": "Fibre channel Word Wide Node Name",
> +                            "type": "string"
> +                        },
> +                        "wwpn": {
> +                            "description": "Fibre channel Word Wide Port Name",
> +                            "type": "string"
> +                        },
>                           "auth": {
>                               "description": "Storage back-end authentication information",
>                               "type": "object",
> diff --git a/src/kimchi/model.py b/src/kimchi/model.py
> index 51a4427..0eb8cc4 100644
> --- a/src/kimchi/model.py
> +++ b/src/kimchi/model.py
> @@ -75,8 +75,9 @@ from kimchi.objectstore import ObjectStore
>   from kimchi.rollbackcontext import RollbackContext
>   from kimchi.scan import Scanner
>   from kimchi.screenshot import VMScreenshot
> -from kimchi.utils import get_enabled_plugins, is_digit, kimchi_log
> -from kimchi.utils import run_command, parse_cmd_output, patch_find_nfs_target
> +from kimchi.utils import get_enabled_plugins, is_digit
> +from kimchi.utils import is_libvirt_version_lesser, kimchi_log, parse_cmd_output
> +from kimchi.utils import patch_find_nfs_target, run_command
>   from kimchi.vmtemplate import VMTemplate
>
>
> @@ -86,7 +87,11 @@ HOST_STATS_INTERVAL = 1
>   VM_STATIC_UPDATE_PARAMS = {'name': './name'}
>   VM_LIVE_UPDATE_PARAMS = {}
>   STORAGE_SOURCES = {'netfs': {'addr': '/pool/source/host/@name',
> -                             'path': '/pool/source/dir/@path'}}
> +                             'path': '/pool/source/dir/@path'},
> +                   'scsi': {'adapter_type': '/pool/source/adapter/@type',
> +                            'adapter_name': '/pool/source/adapter/@name',
> +                            'wwnn': '/pool/source/adapter/@wwnn',
> +                            'wwpn': '/pool/source/adapter/@wwpn'}}
>
>
>   def _uri_to_name(collection, uri):
> @@ -1095,9 +1100,10 @@ class Model(object):
>                   return name
>
>               pool = conn.storagePoolDefineXML(xml, 0)
> -            if params['type'] in ['logical', 'dir', 'netfs']:
> +            if params['type'] in ['logical', 'dir', 'netfs', 'scsi']:
>                   pool.build(libvirt.VIR_STORAGE_POOL_BUILD_NEW)
> -                # autostart dir and logical storage pool created from kimchi
> +                # autostart dir, logical, netfs and scsi storage pools created
> +                # from kimchi
>                   pool.setAutostart(1)
>               else:
>                   # disable autostart for others
> @@ -1735,6 +1741,47 @@ class LogicalPoolDef(StoragePoolDef):
>           return xml
>
>
> +class ScsiPoolDef(StoragePoolDef):
> +    poolType = 'scsi'
> +
> +    def prepare(self, conn=None):

> +        # Adapters type are only available in libvirt >= 1.0.5
> +        if is_libvirt_version_lesser('1.0.5'):

As I commented in previous patch we should avoid comparing versions to 
decide between implementations
Please, add a feature test for that.

> +            self.poolArgs['source']['adapter_type'] = 'scsi_host'

You are always overriding the 'adapter_type'.
You should set values only if user doesn't pass it

> +            tmp_name = self.poolArgs['source']['adapter_name']
> +            self.poolArgs['source']['adapter_name'] = tmp_name.replace('scsi_','')
> +            msg = "Libvirt version <= 1.0.5. Setting SCSI host name as '%s'; "\
> +                  "setting SCSI adapter type as 'scsi_host'; "\
> +                  "ignoring wwnn and wwpn." %tmp_name
> +            kimchi_log.info(msg)
> +

And what about the code for the 'else' statement?

> +    @property
> +    def xml(self):
> +        # Required parameters
> +        # name:
> +        # source[adapter_type]:
> +        # source[adapter_name]:
> +        # source[wwnn]:
> +        # source[wwpn]:
> +        # path:
> +

> +        xml = """
> +        <pool type='scsi'>
> +          <name>{name}</name>
> +          <source>
> +            <adapter type='{source[adapter_type]}'\
> +                     name='{source[adapter_name]}'\
> +                     wwnn='{source[wwnn]}'\
> +                     wwpn='{source[wwpn]}'/>
> +          </source>
> +          <target>
> +            <path>{path}</path>
> +          </target>
> +        </pool>
> +        """.format(**self.poolArgs)

Does this xml work for all libvirt versions in supported distros?
I thought the xml will be different according to libvirt support.

> +        return xml
> +
> +
>   class IscsiPoolDef(StoragePoolDef):
>       poolType = 'iscsi'
>




More information about the Kimchi-devel mailing list