On 27/04/2015 17:51, Rodrigo Trujillo wrote:
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>
>> ---
>> 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.
And trust user will read the comment and do the right thing...?
I don't think it is a good idea.
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.
I don't care about future changes!
I am concerned only about the patch I am reviewing.
>> + 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