[Libvirt-cim] [PATCH] [TEST]#2 Fixed the RASD/07_parent_disk_pool.py

[Kaitlin Rupert]

> +def init_list():
> +    pval = "/dev/null"
> +    ptype1 = { 'ResourceType' : 17,
> +               'PoolID'       : "DiskPool/0",
> +               'Type' : 1L, 'DevicePaths': None, 
> +               'Host' : None, 'SourceDirectory': None, 
> +               'Path' : pval }
> +
> +    ptype2 = ptype1.copy()
> +    ptype2['Type'] = 2L
> +    ptype2['DevicePaths'] = [u'/dev/sda100']
> +
> +    ptype3 = ptype1.copy()
> +    ptype3['Type'] = 3L
> +    ptype3['Host']  = u'host_sys.domain.com'
> +    ptype3['SourceDirectory'] = u'/var/lib/images'
> +    
> +    ptype4 = ptype1.copy()
> +    ptype4['Type'] = 4L
> +    ptype4['DevicePaths'] = [u'/dev/VolGroup00/LogVol100']
> +    
> +    ptype5 = ptype1.copy()
> +    ptype5['Type'] = 5L
> +    ptype5['DevicePaths'] = [u'iscsi-target']
> +    ptype3['Host']  = u'host_sys.domain.com'
> +
> +    ptype6 = ptype1.copy()
> +    ptype6['Type'] = 6L
> +
> +    ptype7 = ptype1.copy()
> +    ptype7['Type'] = 7L
> +    ptype7['Path'] = '/dev/disk/by-id'

Can you include some comments here?  It's not clear which pool type has 
which values set.  Instead of using comments, you could define the pool 
type values in pool.py.  Then you could do something like:

ptype6['Type'] = LOGICAL_POOL  - this will make it easier to tell which 
type is which.

> +
> +    exp_t_dp_h_sdir_path = [ ptype1, ptype2, ptype3, ptype4, 
> +                             ptype5, ptype6, ptype7 ]

I would use more descriptive names... instead of ptype1, use something 
like dir, fs, etc - that'll also make it more clear which dictionary 
belongs to a given pool.

> +    return exp_t_dp_h_sdir_path
> 
>  def get_rec(diskpool_rasd, inst_id='Default'):
>      recs = []
> @@ -70,16 +108,9 @@
>      if status != PASS:
>          return status
>      inst_list = [ 'Default', 'Minimum', 'Maximum', 'Increment' ]
> -    n_rec_val = { 'ResourceType' : 17,
> -                  'PoolID'       : "DiskPool/0",
> -                  'Path'         : "/dev/null",
> -                }
> -    exp_type_path_host_dir = [('1', 'None', 'None', 'None'),
> -                              ('2', '/dev/sda100', 'None', 'None'), 
> -                              ('3', 'None', 'host_sys.domain.com', 
> -                               '/var/lib/images')]
> -                    
> -                  
> +
> +    exp_t_dp_h_sdir_path = init_list()
> +
>      for inst_type in inst_list:
>          logger.info("Verifying '%s' records", inst_type)
> 
> @@ -89,23 +120,30 @@
>                  raise Exception("Got %s recs instead of %s" %(len(n_rec), 
>                                   DISKPOOL_REC_LEN))
> 
> -            res_type_path_host_dir = []
>              for rec in n_rec:
> -                l = (str(rec['Type']), str(rec['DevicePath']), 
> -                         str(rec['Host']), str(rec['SourceDirectory']))
> -                res_type_path_host_dir.append(l)
> +                found = False
> +                for item in exp_t_dp_h_sdir_path:
> +                    if rec['Type'] == item['Type']:
> +                        found = True
> +                        for key, val in item.iteritems():
> +                             exp_val = val
> +                             res_val = rec[key]
> +                             if type(val).__name__ == 'list':
> +                                 cmp_exp = (len(Set(res_val) - \
> +                                            Set(exp_val)) != 0)
> +                             elif type(val).__name__ != 'NoneType':
> +                                 cmp_exp = (exp_val != res_val)
> +                             elif type(val).__name__ == 'NoneType':
> +                                 continue

This is a lot of indention.  Can some of this be moved out to a helper 
function?

-- 
Kaitlin Rupert
IBM Linux Technology Center
kaitlin at linux.vnet.ibm.com