[Kimchi-devel] [PATCH 2/5 - v4] Enable create guests with multiple disks from different pools
Rodrigo Trujillo
rodrigo.trujillo at linux.vnet.ibm.com
Mon Nov 30 13:42:10 UTC 2015
On 11/30/2015 09:08 AM, Aline Manera wrote:
>
>
> On 30/11/2015 01:29, Rodrigo Trujillo wrote:
>> This patch changes the Kimchi backend in order to support creation of
>> guests from Templates that have multiple disks from different storage
>> pools.
>> Before, it was only possible to create a guest with one, two or more
>> disks from only one storagepool. In order to add a disk from another
>> pool, user would have to edit the guest and add it.
>> Now users can do this in creation time, using pre-configured Template.
>>
>> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
>> ---
>> src/wok/plugins/kimchi/i18n.py | 1 +
>> src/wok/plugins/kimchi/model/templates.py | 21 +++++++++++++++------
>> src/wok/plugins/kimchi/model/vms.py | 18 ++++++------------
>> src/wok/plugins/kimchi/vmtemplate.py | 30
>> ++++++++++++++++--------------
>> 4 files changed, 38 insertions(+), 32 deletions(-)
>>
>> diff --git a/src/wok/plugins/kimchi/i18n.py
>> b/src/wok/plugins/kimchi/i18n.py
>> index de7962a..b18048c 100644
>> --- a/src/wok/plugins/kimchi/i18n.py
>> +++ b/src/wok/plugins/kimchi/i18n.py
>> @@ -128,6 +128,7 @@ messages = {
>> "KCHVM0069E": _("Password field must be a string."),
>> "KCHVM0070E": _("Error creating local host ssh rsa key of user
>> 'root'."),
>> "KCHVM0071E": _("Memory value %(mem)s must be aligned to
>> %(alignment)sMiB."),
>
>> + "KCHVM0072E": _("Template given has multiple disks assigned to
>> different types of storage pools, conflicting with storage pool
>> provided."),
>
> As each volume will have its own pool information we should remove the
> former storage pool information. Otherwise, it will be an useless
> information.
I agree, and disagree. I is still used, I decided to keep it in order
to have compatibility with older templates version.
Otherwise, I would have to find a way to update information in objectstore.
So, storagepool is used in the code for 2 reason now:
- It holds the DEFAULT pool from templates.conf. If user does
not pass pool when create a template, then , it is used
- And, for compatibility
>
>> "KCHVMHDEV0001E": _("VM %(vmid)s does not contain directly
>> assigned host device %(dev_name)s."),
>> "KCHVMHDEV0002E": _("The host device %(dev_name)s is not
>> allowed to directly assign to VM."),
>> diff --git a/src/wok/plugins/kimchi/model/templates.py
>> b/src/wok/plugins/kimchi/model/templates.py
>> index bac6e47..eae2714 100644
>> --- a/src/wok/plugins/kimchi/model/templates.py
>> +++ b/src/wok/plugins/kimchi/model/templates.py
>> @@ -147,6 +147,16 @@ class TemplateModel(object):
>> params = session.get('template', name)
>> if overrides:
>> params.update(overrides)
>
>> + if 'storagepool' in params:
>> + libvVMT = LibvirtVMTemplate(params, conn=conn)
>> + poolType =
>> libvVMT._get_storage_type(params['storagepool'])
>> + for i, disk in enumerate(params['disks']):
>> + if disk['pool']['type'] != poolType:
>> + raise InvalidOperation('KCHVM0072E')
>> + else:
>> + params['disks'][i]['pool']['name'] = \
>> + params['storagepool']
>> +
>
> The pool is defined for each volume. We should remove the former
> storage pool information
>
User is not forced to give each disk 'pool", so, kimchi uses the
"default" storagepool configured.
This information must be recorded somewhere.
>> return LibvirtVMTemplate(params, False, conn)
>>
>> def lookup(self, name):
>> @@ -274,8 +284,8 @@ class LibvirtVMTemplate(VMTemplate):
>> raise InvalidParameter("KCHTMPL0007E", {'network':
>> name,
>> 'template': self.name})
>>
>> - def _get_storage_path(self):
>> - pool = self._storage_validate()
>> + def _get_storage_path(self, pool_uri=None):
>> + pool = self._storage_validate(pool_uri)
>> xml = pool.XMLDesc(0)
>> return xpath_get_text(xml, "/pool/target/path")[0]
>>
>> @@ -285,7 +295,7 @@ class LibvirtVMTemplate(VMTemplate):
>> return xpath_get_text(xml, "/pool/@type")[0]
>>
>> def _get_volume_path(self, pool, vol):
>> - pool = self._storage_validate()
>> + pool = self._storage_validate(pool)
>> try:
>> return pool.storageVolLookupByName(vol).path()
>> except:
>> @@ -293,12 +303,11 @@ class LibvirtVMTemplate(VMTemplate):
>> 'pool': pool})
>>
>> def fork_vm_storage(self, vm_uuid):
>> - # Provision storage:
>> - # TODO: Rebase on the storage API once upstream
>> - pool = self._storage_validate()
>> + # Provision storages:
>> vol_list = self.to_volume_list(vm_uuid)
>> try:
>> for v in vol_list:
>> + pool = self._storage_validate(v['pool'])
>> # outgoing text to libvirt, encode('utf-8')
>> pool.createXML(v['xml'].encode('utf-8'), 0)
>> except libvirt.libvirtError as e:
>> diff --git a/src/wok/plugins/kimchi/model/vms.py
>> b/src/wok/plugins/kimchi/model/vms.py
>> index 366f1b4..ebe716e 100644
>> --- a/src/wok/plugins/kimchi/model/vms.py
>> +++ b/src/wok/plugins/kimchi/model/vms.py
>> @@ -151,28 +151,22 @@ class VMsModel(object):
>> wok_log.error('Error trying to update database with
>> guest '
>> 'icon information due error: %s',
>> e.message)
>>
>> - # If storagepool is SCSI, volumes will be LUNs and must be
>> passed by
>> - # the user from UI or manually.
>> - cb('Provisioning storage for new VM')
>> - vol_list = []
>> - if t._get_storage_type() not in ["iscsi", "scsi"]:
>> - vol_list = t.fork_vm_storage(vm_uuid)
>
>> + cb('Provisioning storages for new VM')
>> + vol_list = t.fork_vm_storage(vm_uuid)
>
>
>
>> graphics = params.get('graphics', {})
>> stream_protocols = self.caps.libvirt_stream_protocols
>> xml = t.to_vm_xml(name, vm_uuid,
>> libvirt_stream_protocols=stream_protocols,
>> - graphics=graphics,
>> - volumes=vol_list)
>> + graphics=graphics)
>>
>> cb('Defining new VM')
>> try:
>> conn.defineXML(xml.encode('utf-8'))
>> except libvirt.libvirtError as e:
>> - if t._get_storage_type() not in READONLY_POOL_TYPE:
>> - for v in vol_list:
>> - vol = conn.storageVolLookupByPath(v['path'])
>> - vol.delete(0)
>
>> + for v in vol_list:
>> + vol = conn.storageVolLookupByPath(v['path'])
>> + vol.delete(0)
>> raise OperationFailed("KCHVM0007E", {'name': name,
>> 'err':
>> e.get_error_message()})
>
> Why are not you considering the read-only pools?
Notice above that the old code says:
- if t._get_storage_type() not in ["iscsi", "scsi"]:
- vol_list = t.fork_vm_storage(vm_uuid)
fork_vm_storage call "to_volume_list", where I moved this condition (see
below):
+ # Create only .img. If storagepool is (i)SCSI, volumes will
be LUNs
+ if d['pool']['type'] in ["iscsi", "scsi"]:
+ continue
So vol_list will is always a list of physical ".img"s.
Maybe the confusion is in the "volume" name associated. "Volumes"
related to SCSI and iSCSI
(read-only) pools does not need the path to be discovered here.
>
>> diff --git a/src/wok/plugins/kimchi/vmtemplate.py
>> b/src/wok/plugins/kimchi/vmtemplate.py
>> index c7635b0..69cc9b5 100644
>> --- a/src/wok/plugins/kimchi/vmtemplate.py
>> +++ b/src/wok/plugins/kimchi/vmtemplate.py
>> @@ -184,11 +184,6 @@ class VMTemplate(object):
>> return xml
>>
>> def _get_disks_xml(self, vm_uuid):
>> - # Current implementation just allows to create disk in one
>> single
>> - # storage pool, so we cannot mix the types (scsi volumes vs
>> img file)
>> - storage_type = self._get_storage_type()
>> - storage_path = self._get_storage_path()
>> -
>> base_disk_params = {'type': 'disk', 'disk': 'file',
>> 'bus': self.info['disk_bus']}
>> logical_disk_params = {'format': 'raw'}
>> @@ -199,37 +194,44 @@ class VMTemplate(object):
>> 'format': 'raw', 'bus': 'scsi'}
>>
>> disks_xml = ''
>> - pool_name = pool_name_from_uri(self.info['storagepool'])
>> for index, disk in enumerate(self.info['disks']):
>> params = dict(base_disk_params)
>> params['format'] = disk['format']
>> - params.update(locals().get('%s_disk_params' %
>> storage_type, {}))
>> + params.update(locals().get('%s_disk_params' %
>> + disk['pool']['type'], {}))
>> params['index'] = index
>>
>> volume = disk.get('volume')
>> if volume is not None:
>> - params['path'] = self._get_volume_path(pool_name,
>> volume)
>> + params['path'] =
>> self._get_volume_path(disk['pool']['name'],
>> + volume)
>> else:
>> - volume = "%s-%s.img" % (vm_uuid, params['index'])
>> - params['path'] = os.path.join(storage_path, volume)
>> + img = "%s-%s.img" % (vm_uuid, params['index'])
>> + storage_path =
>> self._get_storage_path(disk['pool']['name'])
>> + params['path'] = os.path.join(storage_path, img)
>>
>> disks_xml += get_disk_xml(params)[1]
>>
>> return unicode(disks_xml, 'utf-8')
>>
>> def to_volume_list(self, vm_uuid):
>> - storage_path = self._get_storage_path()
>> ret = []
>> for i, d in enumerate(self.info['disks']):
>> + # Create only .img. If storagepool is (i)SCSI, volumes
>> will be LUNs
>> + if d['pool']['type'] in ["iscsi", "scsi"]:
>> + continue
>> +
>> index = d.get('index', i)
>> volume = "%s-%s.img" % (vm_uuid, index)
>>
>> + storage_path = self._get_storage_path(d['pool']['name'])
>> info = {'name': volume,
>> 'capacity': d['size'],
>> 'format': d['format'],
>> - 'path': '%s/%s' % (storage_path, volume)}
>> + 'path': '%s/%s' % (storage_path, volume),
>> + 'pool': d['pool']['name']}
>>
>> - if 'logical' == self._get_storage_type() or \
>> + if 'logical' == d['pool']['type'] or \
>> info['format'] not in ['qcow2', 'raw']:
>> info['allocation'] = info['capacity']
>> else:
>> @@ -414,7 +416,7 @@ class VMTemplate(object):
>> def fork_vm_storage(self, vm_uuid):
>> pass
>>
>> - def _get_storage_path(self):
>> + def _get_storage_path(self, pool_uri=None):
>> return ''
>>
>> def _get_storage_type(self, pool=None):
>
> _______________________________________________
> 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