[Kimchi-devel] vmtemplate logic while validating disk parameters

[Aline Manera]

Hi,

On 02/15/2017 05:34 PM, Ramon Medeiros wrote:
> Hi,
>
>
> just looking at kimchi/vmtemplate.py, i saw this confusing logic:
>
>
> 101         basic_disk = ['index', 'format', 'pool', 'size']
> 102         basic_path_disk = ['index', 'format', 'path', 'size']
> 103         ro_disk = ['index', 'format', 'pool', 'volume']
> 104         base_disk = ['index', 'base', 'pool', 'size', 'format']
> 105         base_path_disk = ['index', 'base', 'path', 'size', 'format']
>
>
>
> 148                 if ((keys != sorted(basic_disk)) and
> 149                         (keys != sorted(ro_disk)) and
> 150                         (keys != sorted(base_disk))):
> 151                     # Addition check required only on s390x
> 152                     if not is_s390x() or (keys != 
> sorted(basic_path_disk)):
> 153                         raise MissingParameter('KCHTMPL0028E')
>
> The code is trying to validate if all fields are present, based on 
> combinations of parameters. I want to add more parameters, that are 
> non-optional. 

What do you mean about non-optional parameter? Will that new parameter 
be required for any type of disk? Shouldn't Kimchi set the default value 
when no one is passed?

> Is it good just add it or it's worth to rethink this logic?
>

Why do we need to rethink this logic? What is wrong there?

> What about removing this, since some validations already take place at 
> API.json?

The disk validation can not be done on API.json only. The API.json only 
validates the paramete type but not a combination of the required fields 
given a type of disk.
So this logic still makes sense.