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

[Aline Manera]

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 at 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 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