[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