Re: [Kimchi-devel] [PATCH] Fix disk format lock and checking during template creation

On 04/27/2015 05:00 PM, Aline Manera wrote: > > > On 27/04/2015 16:23, Rodrigo Trujillo wrote: >> Users are able to pass the disk format (qcow, raw, etc) in disk >> Template >> information. However, Kimchi was ignoring this information and always >> creating QCOW2 disk images (if the storagepool is not 'LOGICAL') >> when it >> creates a VM based on a given Template. >> This patch fixes this problem and also moves the checking and disk >> format auto assignment to Template creation phase, in VMTemplate class >> init. >> >> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com&gt; >> --- >> src/kimchi/i18n.py | 1 + >> src/kimchi/vmtemplate.py | 15 +++++++++++---- >> 2 files changed, 12 insertions(+), 4 deletions(-) >> >> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py >> index 18e84bc..0651032 100644 >> --- a/src/kimchi/i18n.py >> +++ b/src/kimchi/i18n.py >> @@ -154,6 +154,7 @@ messages = { >> "KCHTMPL0025E": _("When specifying CPU topology, VCPUs must be >> a product of sockets, cores, and threads."), >> "KCHTMPL0026E": _("When specifying CPU topology, each element >> must be an integer greater than zero."), >> "KCHTMPL0027E": _("Invalid disk image format. Valid formats: >> bochs, cloop, cow, dmg, qcow, qcow2, qed, raw, vmdk, vpc."), >> + "KCHTMPL0028E": _("Invalid disk image format. Logical, SCSI and >> iSCSI storage pools must use disk image format 'raw'."), >> >> "KCHPOOL0001E": _("Storage pool %(name)s already exists"), >> "KCHPOOL0002E": _("Storage pool %(name)s does not exist"), >> diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py >> index ef97d83..84e877b 100644 >> --- a/src/kimchi/vmtemplate.py >> +++ b/src/kimchi/vmtemplate.py >> @@ -75,8 +75,16 @@ class VMTemplate(object): >> args['graphics'] = graphics >> self.info.update(args) >> >> - # Assign right disk format to logical and [i]scsi storagepools >> + # Check disk image format passed and pool type >> if self._get_storage_type() in ['logical', 'iscsi', 'scsi']: >> + paramdisks = args.get('disks') >> + if paramdisks is not None: > > You should use 'self.info' instead of 'args'. > As we plan to support change the Template defaults from a file the > default information needs to be validated as well and it will be in > self.info. > > For default disk format values, the second 'for', below, is changing all disk formats to 'RAW'.

What about write in the configuration file a comment saying that for [i]scsi and logical, the default format is going to be always 'raw' ? Better that let user change and raise errors.

Once this is finished I am going to code the support to multiple disks , so this checkings are going to change anyway in a near future.

>> + for disk in paramdisks: >> + fmt = disk.get('format') >> + if fmt is not None and fmt != 'raw': >> + raise InvalidParameter('KCHTMPL0028E') >> + # Finally add disk image format when not passed or >> + # using default >> for i, disk in enumerate(self.info['disks']): >> self.info['disks'][i]['format'] = 'raw' >> >> @@ -198,7 +206,6 @@ class VMTemplate(object): >> >> def to_volume_list(self, vm_uuid): >> storage_path = self._get_storage_path() >> - fmt = 'raw' if self._get_storage_type() in ['logical'] else >> 'qcow2' >> ret = [] >> for i, d in enumerate(self.info['disks']): >> index = d.get('index', i) >> @@ -206,11 +213,11 @@ class VMTemplate(object): >> >> info = {'name': volume, >> 'capacity': d['size'], >> - 'format': fmt, >> + 'format': d['format'], >> 'path': '%s/%s' % (storage_path, volume)} >> >> if 'logical' == self._get_storage_type() or \ >> - fmt not in ['qcow2', 'raw']: >> + info['format'] not in ['qcow2', 'raw']: >> info['allocation'] = info['capacity'] >> else: >> info['allocation'] = 0 > > _______________________________________________ > Kimchi-devel mailing list > Kimchi-devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel > _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel