[Libvirt-cim] [PATCH] [TEST] #3 Add tc to verify VSMS.RemoveResourceSettings() with correct resource

[Kaitlin Rupert]

> +import sys
> +from XenKvmLib.vsms import get_vsms_class
> +from XenKvmLib.vxml import get_class
> +from XenKvmLib.classes import get_typed_class
> +from XenKvmLib.assoc import AssociatorNames
> +from CimTest.Globals import logger
> +from XenKvmLib.const import do_main, get_provider_version
> +from CimTest.ReturnCodes import FAIL, PASS

Need to include SKIP here.

> +
> +sup_types = ['Xen', 'KVM', 'XenFV']
> +default_dom = 'domain'
> +rem_res_err_rev_start = 779
> +rem_res_err_rev_end = 828
> +ntype = 'network'

This isn't used in the test.

> +                
> +        service = get_vsms_class(options.virt)(options.ip)
> +        for dev in devs:
> +            if dev['CreationClassName'] in dev_not_rem:
> +                continue
> +            else:

No need for an else here.  If the if condition is met, the rest of the 
for loop is skipped over.  This is why I suggested changing the if 
statement...

> +                ccn = dev['CreationClassName']
> +                sccn = dev['SystemCreationClassName']
> +                rasd = AssociatorNames(options.ip, sds_classname, ccn,
> +                                       DeviceID = dev['DeviceID'],
> +                                       CreationClassName = ccn,
> +                                       SystemName = dev['SystemName'],
> +                                       SystemCreationClassName = sccn)
> +                if len(rasd) != 1:
> +                    raise Exception("%i RASD insts for %s", len(rasd), dev.DeviceID)

Instead of dev.DeviceID, this should be dev['DeviceID'].  Also, your 
formatting is off here - this needs to be:

raise Exception("%i RASD insts for %s" % (len(rasd), dev.DeviceID))

Also, this line is also longer than 80 characters..

> +                # Invoke RemoveResourceSettings() to remove resource
> +                ret = service.RemoveResourceSettings(ResourceSettings=[rasd[0]])
> +                if ret[0] != 0:
> +                    raise Exception("Remove %s error, please check", rasd[0])

This error is a little odd, also the formatting of Exception() is incorrect.

raise Exception("RemoveResourceSettings() returned %d removing %s" % 
(ret[0], rasd[0]))

> +    except Exception, details:       
> +        logger.error(details)
> +        cxml.undefine(options.ip)
> +        return FAIL
> +
> +    cxml.dumpxml(options.ip) 
> +    device = cxml.get_value_xpath('/domain/@devices')
> +    curr_cim_rev, changeset = get_provider_version(options.virt, options.ip)
> +
> +    if device == None:
> +        return PASS

You don't undefine() the guest in this case..

> +    elif device != None and curr_cim_rev >= rem_res_err_rev_start and\
> +        curr_cim_rev < rem_res_err_rev_end:
> +        return SKIP

Same issue here..

> +    else:
> +        logger.error('The devices are not removed successfully')
> +        cxml.undefine(options.ip)
> +        return FAIL
> +

Instead of calling undefine() so many times in a row, this could be 
formatted as:

if device == None:
     status = PASS
elif device != None and curr_cim_rev >= rem_res_err_rev_start and\
      curr_cim_rev < rem_res_err_rev_end:
     status = SKIP

You don't undefine the guest in this case.

else:
     logger.error('The devices are not removed successfully')
     status = FAIL


> +    cxml.undefine(options.ip)
> +    return PASS

This should then be:

return status

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