[Libvirt-cim] [PATCH] [TEST] #5 Add new test to verify enum of DiskRASD to have EmulatedType=0 for Disk and EmulatedType=1 for CDROM

[Deepti B Kalakeri]

Kaitlin Rupert wrote:
>> +import sys
>> +import pywbem
>
> pywbem is not used
>
>> +from VirtLib import utils
>
> utils is not used
>
>> +from XenKvmLib import enumclass
>
> Instead of importing all of the enumclass module, instead you can 
> import just the EnumInstances() function:
>
> from XenKvmLib.enumclass import EnumInstances
>
>> +from XenKvmLib.classes import get_typed_class
>> +from XenKvmLib.test_doms import undefine_test_domain
>
> undefine_test_domain() isn't used, this can be removed
>
>> +from XenKvmLib.common_util import parse_instance_id
>> +from XenKvmLib.const import do_main
>> +from XenKvmLib import vxml
>
> Since you only use get_class(), it's better to do:
>
> from XenKvmLib.vxml import get_class
>
>> +from CimTest.ReturnCodes import PASS, FAIL
>> +from CimTest.Globals import logger, CIM_ERROR_ENUMERATE
>
> CIM_ERROR_ENUMERATE is not used
>
>> +from XenKvmLib.const import get_provider_version
>> +
>> +SUPPORTED_TYPES = ['KVM']
>
> This should also support Xen and XenFV.
>
>> +default_dom = 'test_domain'
>> +libvirt_em_type_changeset = 737
>> +
>> + at do_main(SUPPORTED_TYPES)
>> +def main():
>> + status = FAIL
>> + options = main.options
>> + curr_cim_rev, changeset = get_provider_version(options.virt, 
>> options.ip)
>> + if curr_cim_rev < libvirt_em_type_changeset:
>> + return SKIP
>> +
>> + emu_types = [0, 1]
>> + for exp_emu_type in emu_types:
>> + cxml = vxml.get_class(options.virt)(default_dom, 
>> emu_type=exp_emu_type)
>> + ret = cxml.cim_define(options.ip)
>> + if not ret:
>> + logger.error("Failed to call DefineSystem()")
>> + return FAIL
>> + + drasd= 
>> get_typed_class(options.virt,'DiskResourceAllocationSettingData')
>> + try:
>> + drasd_list = enumclass.EnumInstances(options.ip, drasd, + 
>> ret_cim_inst=True)
>> + if len(drasd_list) < 1:
>> + raise Exception("%s returned %i instances, excepted at least 1.",
>
> Typo - excepted should be expected. Also, this line is 81 characters - 
> removing the ".' should fix that =)
>
>> + drasd, len(drasd_list))
>> +
>> + found_rasd = None
>> + for rasd in drasd_list:
>> + guest, dev, status = parse_instance_id(rasd['InstanceID'])
>> + if status != PASS:
>> + raise Exception("Unable to parse InstanceID: %s" \
>> + % rasd['InstanceID'])
>> + if guest == default_dom:
>> + if rasd['EmulatedType'] == exp_emu_type:
>> + found_rasd = rasd
>> + status = PASS
>> + break
>> + else:
>> + raise Exception("EmulatedType Mismatch: got %d, \
>> + expected %d", rasd['EmulatedType'], exp_emu_type)
>
> The indention is off here. Also, you'll need to use % not commas. This 
> should be:
>
> raise Exception("EmulatedType Mismatch: got %d, \
> expected %d" % (rasd['EmulatedType'],
> exp_emu_type))
We need not have to use % , comma works. Also, we can avoid slash. The 
above can be rewritten as follows:
raise Exception("EmulatedType Mismatch: got %d, "
"expected %d", (rasd['EmulatedType'],
exp_emu_type))
>
>
Thanks and Regards,
Deepti.