[Libvirt-cim] [PATCH] [TEST] Fix verify_rasds() and verify_devices() in SD 02_reverse.py

[Deepti B Kalakeri]

In the main fn:
can you remove the cxml.undefine() from the following :

status, cxml = setup_env(options.ip, virt)
if status != PASS:
cxml.undefine(options.ip)
return status


Kaitlin Rupert wrote:
> # HG changeset patch
> # User Kaitlin Rupert <karupert at us.ibm.com>
> # Date 1232391542 28800
> # Node ID 1f401b57daf5ef20dda27f391a4eaff3d15f72d8
> # Parent  1313971a3554942a538a2c322f29a2321d0f4e08
> [TEST] Fix verify_rasds() and verify_devices() in SD 02_reverse.py
>
> Since VSSDC should return RASDs that all correspond to the guest, there's
> no need to parse the InstanceID to see if the guest name matches the guest
> we've created.
>
> Also, fix the logic of the verify functions to be a little cleaner. Compare
> all the properties of the instances, not just the InstanceIDs/DeviceIDs.
>
> Remove parse_instance_id() from verify_devices(); guest and dev_id were not
> being used in the function.
>
> Signed-off-by: Kaitlin Rupert <karupert at us.ibm.com>
>
> diff -r 1313971a3554 -r 1f401b57daf5 suites/libvirt-cim/cimtest/SettingsDefine/02_reverse.py
> --- a/suites/libvirt-cim/cimtest/SettingsDefine/02_reverse.py	Mon Jan 19 10:51:08 2009 -0800
> +++ b/suites/libvirt-cim/cimtest/SettingsDefine/02_reverse.py	Mon Jan 19 10:59:02 2009 -0800
> @@ -55,7 +55,7 @@
>  from CimTest.Globals import logger
>  from CimTest.ReturnCodes import PASS, FAIL
>  from XenKvmLib.const import do_main
> -from XenKvmLib.assoc import AssociatorNames
> +from XenKvmLib.assoc import AssociatorNames, compare_all_prop
>  from XenKvmLib.vxml import get_class
>  from XenKvmLib.classes import get_typed_class
>  from XenKvmLib.rasd import enum_rasds
> @@ -135,7 +135,7 @@
>
>      return dev_insts, PASS
>
> -def verify_rasd(virt, enum_list, rasds, guest_name):
> +def verify_rasd(virt, enum_list, rasds):
>      proc_rasd_cn = get_typed_class(virt, "ProcResourceAllocationSettingData")
>
>      for rasd in enum_list:
> @@ -143,48 +143,40 @@
>              enum_list.remove(rasd) 
>              continue
>
> -        guest, dev, status = parse_instance_id(rasd['InstanceID'])
> +        exp_rasd = rasds[rasd.classname]
> +
> +        if rasd['InstanceID'] != exp_rasd.InstanceID:
> +            logger.error("Got %s instead of %s", rasd['InstanceID'],
> +                         exp_rasd.InstanceID)
> +            return FAIL
> +
> +        status = compare_all_prop(rasd, exp_rasd)
>          if status != PASS:
> -            logger.error("Unable to parse InstanceID: %s", rasd['InstanceID'])
>              return status
>
> -        if guest != guest_name:
> -            continue
> -
> -        exp_rasd = rasds[rasd.classname]
> -
> -        if rasd['InstanceID'] == exp_rasd.InstanceID:
> -            status = PASS
> -        else:
> -            logger.info("Got %s instead of %s" % (rasd['InstanceID'],
> -                        exp_rasd.InstanceID))
> -            status = FAIL
> -
> -    if status != PASS:
> -        logger.error("RASD with id %s not returned", exp_rasd.InstanceID)
> +    if len(enum_list) != len(rasds):
> +        logger.error("Got %d RASDs, expected %d", len(enum_list), len(rasds))
>          return FAIL
>   
Can you move this check before the for loop.
>      return PASS
>
>  def verify_devices(enum_list, devs):
> -    for dev in enum_list:
> -        guest, dev_id, status = parse_instance_id(dev['DeviceID'])
> -        if status != PASS:
> -            logger.error("Unable to parse InstanceID: %s", dev['DeviceID'])
> -            return status
> +    dev = enum_list[0]
> +    dev_cn = dev.classname
>
> -        exp_dev = devs[dev.classname]
> +    if len(enum_list) != 1:
> +        logger.error("Got %d %s devices, expected 1", len(enum_list), dev_cn)
> +        return FAIL
>
> -        if dev['DeviceID'] == exp_dev.DeviceID:
> -            status = PASS
> -        else:
> -            logger.info("Got %s instead of %s" % (dev['DeviceID'],
> -                        exp_dev.DeviceID))
> -            status = FAIL
> +    exp_dev = devs[dev_cn]
>
> +    if dev['DeviceID'] != exp_dev.DeviceID:
> +        logger.error("Got %s instead of %s", dev['DeviceID'], exp_dev.DeviceID)
> +        return FAIL
> +
> +    status = compare_all_prop(dev, exp_dev)
>      if status != PASS:
> -        logger.error("Device with id %s not returned", exp_dev.DeviceID)
> -        return FAIL
> +        return status
>
>      return PASS
>   
You can print the error message when status != PASS, and then return 
status instead of two return statements.
> @@ -217,13 +209,10 @@
>          assoc = AssociatorNames(options.ip, vssdc_cn, vssd_cn,
>                                  InstanceID = instIdval)
>
> -        status = verify_rasd(virt, assoc, rasds, test_dom)
> +        status = verify_rasd(virt, assoc, rasds)
>          if status != PASS:
>              raise Exception("Failed to verify RASDs")
>
> -        if len(assoc) != len(rasds):
> -            raise Exception("Got %d RASDs, exp %d" % (len(assoc), len(rasds)))
> -
>          sds_cn = get_typed_class(virt, 'SettingsDefineState')
>
>          devs, status = init_device_list(virt, options.ip, test_dom)
>
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim
>   

-- 
Thanks and Regards,
Deepti B. Kalakeri
IBM Linux Technology Center
deeptik at linux.vnet.ibm.com