[Kimchi-devel] [PATCH] Issue #1052 : incorrect pool initialisation on s390x

[Aline Manera]

On 10/12/2016 11:30 AM, harshalp at linux.vnet.ibm.com wrote:
> From: Harshal Patil <harshalp at linux.vnet.ibm.com>
>
> This patch fixes issue with initialisation of storage pools
> on s390x when they are explicitly mentioned in
> template_s390x.conf file
>
> Signed-off-by: Harshal Patil <harshalp at linux.vnet.ibm.com>
> ---
>   model/storagepools.py |  5 ++++-
>   osinfo.py             | 25 +++++++++++++++++--------
>   2 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/model/storagepools.py b/model/storagepools.py
> index 5942b31..e918f10 100644
> --- a/model/storagepools.py
> +++ b/model/storagepools.py
> @@ -74,7 +74,10 @@ class StoragePoolsModel(object):
>           pools = {}
>
>           if is_s390x():
> -            return
> +            # Don't create default pool if it's not
> +            # explicitly specified in template_s390x.conf
> +            if 'pool' not in tmpl_defaults['disks'][0]:
> +                return
>
>           default_pool = tmpl_defaults['disks'][0]['pool']['name']
>           default_pool = default_pool.split('/')[-1]
> diff --git a/osinfo.py b/osinfo.py
> index 1527896..bf649f1 100644
> --- a/osinfo.py
> +++ b/osinfo.py
> @@ -210,16 +210,25 @@ def _get_tmpl_defaults():
>           data['index'] = int(disk.split('.')[1])
>           # Right now 'Path' is only supported on s390x
>           if storage_section[disk].get('path') and is_on_s390x:
> -            data['path'] = storage_section[disk].pop('path')
> -            if 'size' not in storage_section[disk]:
> -                data['size'] = tmpl_defaults['storage']['disk.0']['size']
> -            else:
> -                data['size'] = storage_section[disk].pop('size')


> +            if storage_section[disk].get('pool'):
> +                # If 'pool' is explicitly specified in
> +                # template_s390x.conf, drop 'path'

You should raise an error instead of silently ignore the user input.
The 'path' and 'pool' are specified for s390x, raise an error informing 
user they are exclusively option.

> +                storage_section[disk].pop('path', None)
> +                data['pool'] = {"name": '/plugins/kimchi/storagepools/' +
> +                                        storage_section[disk].pop('pool')}
>
> -            if 'format' not in storage_section[disk]:
> -                data['format'] = tmpl_defaults['storage']['disk.0']['format']
>               else:
> -                data['format'] = storage_section[disk].pop('format')
> +                data['path'] = storage_section[disk].pop('path')
> +                if 'size' not in storage_section[disk]:
> +                    data['size'] = tmpl_defaults['storage']['disk.0']['size']
> +                else:
> +                    data['size'] = storage_section[disk].pop('size')
> +
> +                if 'format' not in storage_section[disk]:
> +                    data['format'] = tmpl_defaults['storage']['disk.0']['format']
> +                else:
> +                    data['format'] = storage_section[disk].pop('format')
> +
>           else:
>               data['pool'] = {"name": '/plugins/kimchi/storagepools/' +
>                                       storage_section[disk].pop('pool')}