[Libvirt-cim] [PATCH] [TEST] Adding new tc to verify fs storage pool creation

[Kaitlin Rupert]

> +import os
> +import sys
> +from optparse import OptionParser
> +from commands  import getstatusoutput
> +from distutils.text_file import TextFile
> +from pywbem import WBEMConnection, cim_types
> +sys.path.append('../../../lib')
> +from CimTest import Globals
> +from CimTest.Globals import logger, log_param
> +sys.path.append('../lib')
> +from XenKvmLib.classes import inst_to_mof, get_typed_class
> +from XenKvmLib.pool import get_pool_rasds
> +sys.path.append('../')
> +from main import pre_check

I like the use of this check, but importing from main.py is a little 
odd.  main.py isn't intended to be a module... can you move pre-check to 
  XenKvmLib?

You don't have to do this change in the this patch - you can follow up 
with a different patch once this test is in the tree.

> +
> +PASS = 0
> +FAIL = 1

Why not import the PASS / FAIL values from lib/CimTest/ReturnCodes.py?

> +
> +def get_pool_settings(dp_rasds, pooltype, part_dev, mount_pt, pool_name):
> +    pool_settings = None
> +    for dpool_rasd in dp_rasds:
> +        if dpool_rasd['Type'] == pooltype and \
> +            dpool_rasd['InstanceID'] == 'Default':
> +            dpool_rasd['DevicePaths'] =[part_dev]

Need a space between =[part_dev] so that is looks like:

dpool_rasd['DevicePaths'] = [part_dev]

> +            dpool_rasd['Path'] = mount_pt
> +            dp_pid = "%s/%s" %("DiskPool", pool_name)

Need a space here so it looks like:

  dp_pid = "%s/%s" % ("DiskPool", pool_name)

> +            dpool_rasd['PoolID'] = dpool_rasd['InstanceID'] = dp_pid
> +            break
> +    if not pool_name in dpool_rasd['InstanceID']:
> +        return pool_settings
> +
> +    pool_settings = inst_to_mof(dpool_rasd)
> +    return pool_settings
> +
> +


> +def cleanup(virt, pool_name, sysname, mount_pt, del_dir):
> +    virsh = "virsh -c %s" % get_uri(virt)
> +    cmd = "%s pool-destroy  %s && %s pool-undefine %s" \
> +           % (virsh, pool_name, virsh, pool_name)

Why not use the providers to clean up the pool?

> +    ret, out = getstatusoutput(cmd)
> +    if ret != PASS:
> +        logger.error("WARNING: pool '%s' was not cleaned on '%s'", 
> +                    pool_name, sysname)
> +        logger.error("WARNING: Please remove it manually")
> +
> +    if del_dir == True:
> +        cmd ="rm -rf %s"  % mount_pt
> +        status, out = getstatusoutput(cmd)
> +        if status != PASS:
> +            logger.error("WARNING: '%s' was not removed", mount_pt)
> +            logger.error("WARNING: Please remove %s manually", mount_pt)
> +


> +        # Verify if the desired pool was successfully created ..
> +        status, out = verify_pool(virt, pool_name)
> +        if status != PASS:
> +          raise Exception("Failed to create pool: %s " % pool_name)
> +
> +    except Exception, details:
> +        logger.error("In main(), exception '%s'", details)
> +        if del_dir == True:
> +            cmd ="rm -rf %s"  % mount_pt
> +            status, out = getstatusoutput(cmd)

If the pool is created properly bu the verify step fails, then you will 
attempt to remove a directory that has a disk mounted on it.

Also, if the pool is created properly, you don't clean it up here. 
Instead of attempting to remove the directory here, why not just set 
res[0] = FAIL and then fall through so that cleanup() is called?

> +        return FAIL
> +
> +    # Clean up the pool and the mount dir that was created ...
> +    cleanup(virt, pool_name, sysname, mount_pt, del_dir)
> +
> +    if res[0] == PASS:
> +        logger.info("Pool %s was successfully verified for pool type %s", 
> +                    pool_name , options.pool_type)
> +
> +        # Place holder to give a hint to the user the tc passed 
> +        # otherwise the user will have to look into the cimtest.log in the 
> +        # current dir.
> +        print "Pool '", pool_name,"' was successfully verified for pool type "\
> +              "'", options.pool_type , "'"
> +    else:
> +        logger.error("Test Failed to verify '%s' pool creation ....", 
> +                     options.pool_type)
> +    return res[0]
> +if __name__=="__main__":
> +    sys.exit(main())
> +
> 
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim


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