[Libvirt-cim] [PATCH] [TEST] Added scsi pool support

[Kaitlin Rupert]

> +        # Check if the mount point specified already exist, if not then create it..

This comment is longer than 80 characters.

> +        if not os.path.exists(mount_pt):
> +            os.mkdir(mount_pt)
> 
> -    # Check if the mount point specified already exist, if not then create it..

Same here.  The verification of the fs pool params is rather long. If 
you want to save on indention, you could use a helper function here.

> -    if not os.path.exists(mount_pt):
> -        os.mkdir(mount_pt)
> +            # set del_dir to True so that we remove it before exiting from the tc.
> +            del_dir = True 
> +        else:
> +            # Check if the mount point specified is a dir
> +            if not os.path.isdir(mount_pt):
> +                logger.error("The mount point [%s] should be a dir", mount_pt)
> +                return FAIL, del_dir
> 
> -        # set del_dir to True so that we remove it before exiting from the tc.
> -        del_dir = True 
> -    else:
> -        # Check if the mount point specified is a dir
> -        if not os.path.isdir(mount_pt):
> -            logger.error("The mount point [%s] should be a dir", mount_pt)
> -            return FAIL, del_dir
> -
> -        files = os.listdir(mount_pt)
> -        if len(files) != 0:
> -            logger.info("The mount point [%s] given is not empty", mount_pt)
> +            files = os.listdir(mount_pt)
> +            if len(files) != 0:
> +                logger.info("The mount point [%s] given is not empty", mount_pt)
> 
>      return PASS, del_dir
> 
> @@ -195,7 +216,8 @@
>          vuri = 'lxc:///system'
>      return vuri
> 
> -def get_pool_settings(dp_rasds, pooltype, part_dev, mount_pt, pool_name):
> +def get_pool_settings(dp_rasds, pooltype, part_dev, mount_pt, 
> +                      pool_name, adap_name):
>      pool_settings = None
>      for dpool_rasd in dp_rasds:
>          if dpool_rasd['Type'] == pooltype and \
> @@ -207,6 +229,9 @@
>                  dpool_rasd['DevicePaths'] = [part_dev]
>              elif pooltype == pool_types['DISK_POOL_LOGICAL']:
>                  dpool_rasd['Path'] = part_dev
> +            if pooltype == pool_types['DISK_POOL_SCSI']:
> +                dpool_rasd['AdapterName'] = adap_name
> +                dpool_rasd['Path'] = "/dev/disk/by-id"
>              break

I've got a selfish request here..

Can you add some spacing in the for loop part of this function?  For 
some reason, it's hard for me to read.  Just add a blank line before 
some of the if / elif statements.




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