[Kimchi-devel] [PATCH 2/5 - v4] Enable create guests with multiple disks from different pools
Aline Manera
alinefm at linux.vnet.ibm.com
Mon Nov 30 11:08:29 UTC 2015
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.
> "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
> 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?
> 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):
More information about the Kimchi-devel
mailing list