Re: [Kimchi-devel] [PATCH 2/5 - v4] Enable create guests with multiple disks from different pools

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(a)linux.vnet.ibm.com&gt; > --- > 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): _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel