[Kimchi-devel] [PATCH V2 1/7] (WIP) Storagepool SCSI/FC: SCSI/Fibre Channel backend implementation
Rodrigo Trujillo
rodrigo.trujillo at linux.vnet.ibm.com
Tue Jan 28 20:28:10 UTC 2014
On 01/25/2014 08:26 PM, Aline Manera wrote:
> 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
I am going to simplify this.
>
>> ### 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?
My bad... mistake while rebasing
>
>> "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)
This will not be necessary anymore
>
>> + "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_outpifferentut,
>> 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.
I don't agree, as this does not seem to depend on libvirt compilation.
But will implement as FEATURE TEST
>
>> + 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
Will remove this parameter and make tests when creating the VM
>
>> + 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?
By KIMCHI design, the 'prepare' function is always called, so I just
have to do something according to version of libvirt.
All the code I made should work without modifications if libvirt is >=
1.0.5 ... so does not need ELSE.
>> + @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.
The storagepoll xml works, the Disk xml should be different . ww[P/N]n
will be ignored in older versions, if passed.
>
>> + return xml
>> +
>> +
>> class IscsiPoolDef(StoragePoolDef):
>> poolType = 'iscsi'
>>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
More information about the Kimchi-devel
mailing list