[PATCH] [TEST] Enabling 02_reverse.py with XenFV, KVM, LXC and modified logicaldevices.py to work for all virt types

# HG changeset patch # User Deepti B. Kalakeri <deeptik@linux.vnet.ibm.com> # Date 1216384179 25200 # Node ID f8f6995ce08915d738d82600b15559c81fc40095 # Parent 3703b7be5a107c67e901546978e974546b3d5562 [TEST] Enabling 02_reverse.py with XenFV, KVM, LXC and modified logicaldevices.py to work for all virt types. Changes: ------- 1) Removed unnecessary import statements. 2) Updated verify_proc_values, verify_mem_values, verify_net_values, verify_disk_values function of logicaldevices to work with all virt types. 3) Used verify_proc_values, verify_mem_values, verify_net_values, verify_disk_values fn. 4) Removed conf_file(), clean_up_restore(), get_or_bail(), print_error(), init_list(), get_spec_fields_list(), assoc_values() fns. 5) Removed global status variable. 6) Added verify_eafp_values(), init_pllist(), create_diskpool_conf() fn. 7) Included create_diskpool_conf(), cleanup_restore() fn. Tested on KVM on rpm, KVM current sources, LXC, XenFV, Xen. Signed-off-by: Deepti B. Kalakeri <deeptik@linux.vnet.ibm.com> diff -r 3703b7be5a10 -r f8f6995ce089 suites/libvirt-cim/cimtest/ElementAllocatedFromPool/02_reverse.py --- a/suites/libvirt-cim/cimtest/ElementAllocatedFromPool/02_reverse.py Wed Jul 16 07:23:32 2008 -0700 +++ b/suites/libvirt-cim/cimtest/ElementAllocatedFromPool/02_reverse.py Fri Jul 18 05:29:39 2008 -0700 @@ -20,9 +20,8 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA # - # This tc is used to verify the classname, InstanceID and certian prop are -# are appropriately set for the domains when verified using the +# appropriately set for the domains when verified using the # Xen_ElementAllocatedFromPool asscoiation. # # Example command for LogicalDisk w.r.t to Xen_ElementAllocatedFromPool \ @@ -47,111 +46,80 @@ import sys import os -from distutils.file_util import move_file import pywbem -from VirtLib import utils -from VirtLib import live -from XenKvmLib import assoc -from XenKvmLib import enumclass -from CimTest import Globals -from CimTest.Globals import do_main -from CimTest.ReturnCodes import PASS, FAIL, SKIP -from XenKvmLib.test_xml import testxml_bridge -from XenKvmLib.test_doms import test_domain_function, destroy_and_undefine_all -from VirtLib.live import network_by_bridge +from XenKvmLib.assoc import Associators +from XenKvmLib.vxml import get_class +from CimTest.Globals import do_main, logger, CIM_ERROR_ASSOCIATORS +from CimTest.ReturnCodes import PASS, FAIL +from XenKvmLib.test_doms import destroy_and_undefine_all +from XenKvmLib.classes import get_typed_class +from XenKvmLib.common_util import create_diskpool_conf, cleanup_restore +from XenKvmLib.logicaldevices import verify_proc_values, verify_mem_values, \ +verify_net_values, verify_disk_values -sup_types = ['Xen'] +sup_types = ['Xen' , 'KVM', 'XenFV', 'LXC'] -status = PASS -test_dom = "hd_domain" +test_dom = "eafp_domain" test_mac = "00:11:22:33:44:aa" test_mem = 128 test_vcpus = 4 -test_disk = "xvdb" -test_dpath = "foo" -disk_file = '/tmp/diskpool.conf' -back_disk_file = disk_file + "." + "02_reverse" -diskid = "%s/%s" % ("DiskPool", test_dpath) -memid = "%s/%s" % ("MemoryPool", 0) -procid = "%s/%s" % ("ProcessorPool", 0) -def conf_file(): - """ - Creating diskpool.conf file. - """ - try: - f = open(disk_file, 'w') - f.write('%s %s' % (test_dpath, '/')) - f.close() - except Exception,detail: - Globals.logger.error("Exception: %s", detail) - status = SKIP - sys.exit(status) +def init_pllist(virt, vsxml, diskid): + keys = { + 'MemoryPool' : 'MemoryPool/0', + } + if virt != 'LXC': + virt_network = vsxml.xml_get_net_network() + keys['DiskPool'] = diskid + keys['ProcessorPool'] = 'ProcessorPool/0' + keys['NetworkPool'] = 'NetworkPool/%s' %virt_network -def clean_up_restore(ip): - """ - Restoring back the original diskpool.conf - file. - """ - try: - if os.path.exists(back_disk_file): - os.remove(disk_file) - move_file(back_disk_file, disk_file) - except Exception, detail: - Globals.logger.error("Exception: %s", detail) - status = SKIP - ret = test_domain_function(test_dom, ip, \ - cmd = "destroy") - sys.exit(status) - + pllist = { } + for cn, k in keys.iteritems(): + cn = get_typed_class(virt, cn) + pllist[cn] = k -def get_or_bail(ip, id, pool_class): - """ - Getinstance for the CLass and return instance on success, otherwise - exit after cleanup_restore and destroying the guest. - """ - key_list = { 'InstanceID' : id } - try: - instance = enumclass.getInstance(ip, pool_class, key_list) - except Exception, detail: - Globals.logger.error(Globals.CIM_ERROR_GETINSTANCE, '%s', pool_class) - Globals.logger.error("Exception: %s", detail) - clean_up_restore(ip) - status = FAIL - ret = test_domain_function(test_dom, ip, \ - cmd = "destroy") - sys.exit(status) - return instance + return pllist -def print_error(field, ret_val, req_val): - Globals.logger.error("%s Mismatch", field) - Globals.logger.error("Returned %s instead of %s", ret_val, req_val) +def eafp_list(virt, test_disk): + mcn = get_typed_class(virt, "Memory") + mem = { + 'SystemName' : test_dom, + 'CreationClassName' : mcn, + 'DeviceID' : "%s/%s" % (test_dom, "mem"), + 'NumberOfBlocks' : test_mem * 1024 + } -def init_list(ip, disk, mem, net, proc): - """ - Creating the lists that will be used for comparisons. - """ + eaf_values = { mcn : mem } - pllist = { - "Xen_DiskPool" : disk.InstanceID, \ - "Xen_MemoryPool" : mem.InstanceID, \ - "Xen_NetworkPool" : net.InstanceID, \ - "Xen_ProcessorPool": proc.InstanceID - } - cllist = [ - "Xen_LogicalDisk", \ - "Xen_Memory", \ - "Xen_NetworkPort", \ - "Xen_Processor" - ] - prop_list = ["%s/%s" % (test_dom, test_disk), test_disk, \ - "%s/%s" % (test_dom, "mem"), test_mem, \ - "%s/%s" % (test_dom, test_mac), test_mac - ] - proc_prop = [] - for i in range(test_vcpus): - proc_prop.append("%s/%s" % (test_dom, i)) - return pllist, cllist, prop_list, proc_prop + if virt != 'LXC': + dcn = get_typed_class(virt, "LogicalDisk") + pcn = get_typed_class(virt, "Processor") + ncn = get_typed_class(virt, "NetworkPort") + + disk = { + 'SystemName' : test_dom, + 'CreationClassName' : dcn, + 'DeviceID' : "%s/%s" % (test_dom, test_disk), + 'Name' : test_disk + } + proc = { + 'SystemName' : test_dom, + 'CreationClassName' : pcn, + 'DeviceID' : None + } + net = { + 'SystemName' : test_dom, + 'CreationClassName' : ncn, + 'DeviceID' : "%s/%s" % (test_dom, test_mac), + 'NetworkAddresses' : test_mac + } + + eaf_values[pcn] = proc + eaf_values[dcn] = disk + eaf_values[ncn] = net + + return eaf_values def get_inst_for_dom(assoc_val): list = [] @@ -162,196 +130,87 @@ def get_inst_for_dom(assoc_val): return list -def get_spec_fields_list(inst_list, field_name): - global status - specific_fields = { } - if (len(inst_list)) != 1: - Globals.logger.error("Got %s record for Memory/Network/LogicalDisk instead of \ -1", len(inst_list)) - status = FAIL - return -# verifying the Name field for LogicalDisk - try: - if inst_list[0]['CreationClassName'] != 'Xen_Memory': - field_value = inst_list[0][field_name] - if field_name == 'NetworkAddresses': -# For network we NetworkAddresses is a list of addresses, since we -# are assigning only one address we are taking field_value[0] - field_value = field_value[0] - else: - field_value = ((int(inst_list[0]['NumberOfBlocks'])*4096)/1024) - specific_fields = { - "field_name" : field_name,\ - "field_value" : field_value - } - except Exception, detail: - Globals.logger.error("Exception in get_spec_fields_list(): %s", detail) - status = FAIL - return specific_fields +def verify_eafp_values(server, virt, in_pllist, test_disk): + # Looping through the in_pllist to get association for various pools. + eafp_values = eafp_list(virt, test_disk) + an = get_typed_class(virt, "ElementAllocatedFromPool") + for cn, instid in sorted(in_pllist.iteritems()): + try: + assoc_info = Associators(server, an, cn, virt, InstanceID = instid) + assoc_inst_list = get_inst_for_dom(assoc_info) + if len(assoc_inst_list) < 1 : + logger.error("'%s' with '%s' did not return any records for" + " domain: '%s'", an, cn, test_dom) + return FAIL -def assoc_values(assoc_list, field , list, index, specific_fields_list=""): - """ - Verifying the records retruned by the associations. - """ - global status - if field == "CreationClassName": - for i in range(len(assoc_list)): - if assoc_list[i][field] != list[index]: - print_error(field, assoc_list[i][field], list[index]) + assoc_eafp_info = assoc_inst_list[0] + CCName = assoc_eafp_info['CreationClassName'] + if CCName == get_typed_class(virt, 'Processor'): + if len(assoc_inst_list) != test_vcpus: + logger.error("'%s' should have returned '%i' Processor" + " details, got '%i'", an, test_vcpus, + len(assoc_inst_list)) + return FAIL + for i in range(test_vcpus): + eafp_values[CCName]['DeviceID'] = "%s/%s" % (test_dom,i) + status = verify_proc_values(assoc_inst_list[i], + eafp_values, virt) + elif CCName == get_typed_class(virt, 'NetworkPort'): + status = verify_net_values(assoc_eafp_info, eafp_values, virt) + elif CCName == get_typed_class(virt, 'LogicalDisk'): + status = verify_disk_values(assoc_eafp_info, eafp_values, virt) + elif CCName == get_typed_class(virt, 'Memory'): + status = verify_mem_values(assoc_eafp_info, eafp_values, virt) + else: status = FAIL if status != PASS: break - elif field == "DeviceID": - if assoc_list[0]['CreationClassName'] == 'Xen_Processor': -# Verifying the list of DeviceId returned by the association -# against the list created intially . - for i in range(len(list)): - if assoc_list[i]['DeviceID'] != list[i]: - print_error(field, assoc_list[i]['DeviceID'], list[i]) - status = FAIL - else: -# Except for Xen_Processor, we get only once record for a domain for -# other classes. - if assoc_list[0]['DeviceID'] != list[index]: - print_error(field, assoc_list[0]['DeviceID'] , list[index]) - status = FAIL - else: - # other specific fields verification - if assoc_list[0]['CreationClassName'] != 'Xen_Processor': - spec_field_name = specific_fields_list['field_name'] - spec_field_value = specific_fields_list['field_value'] - if spec_field_value != list[index]: - print_error(field, spec_field_value, list[index]) - status = FAIL - + except Exception, detail: + logger.error(CIM_ERROR_ASSOCIATORS, an) + logger.error("Exception: %s", detail) + status = FAIL + return status @do_main(sup_types) def main(): options = main.options - global status loop = 0 server = options.ip - destroy_and_undefine_all(options.ip) - test_xml, bridge = testxml_bridge(test_dom, mem = test_mem, vcpus = test_vcpus, \ - mac = test_mac, disk = test_disk, server = options.ip) - if bridge == None: - Globals.logger.error("Unable to find virtual bridge") - return SKIP + virt = options.virt + if virt == 'Xen': + test_disk = 'xvdb' + else: + test_disk = 'hda' - if test_xml == None: - Globals.logger.error("Guest xml was not created properly") + # Getting the VS list and deleting the test_dom if it already exists. + destroy_and_undefine_all(server) + virt_type = get_class(virt) + if virt == 'LXC': + vsxml = virt_type(test_dom, vcpus = test_vcpus) + else: + vsxml = virt_type(test_dom, mem = test_mem, vcpus = test_vcpus, + mac = test_mac, disk = test_disk) + + # Verify DiskPool on machine + status, diskid = create_diskpool_conf(server, virt) + if status != PASS: + return status + + ret = vsxml.create(server) + if not ret: + logger.error("Failed to Create the dom: '%s'", test_dom) return FAIL - virt_network = network_by_bridge(bridge, server) - if virt_network == None: - Globals.logger.error("No virtual network found for bridge %s", bridge) - return SKIP + # Get pool list against which the EAFP should be queried + pllist = init_pllist(virt, vsxml, diskid) - ret = test_domain_function(test_xml, server, cmd = "create") - if not ret: - Globals.logger.error("Failed to Create the dom: %s", test_dom) - return FAIL + + status = verify_eafp_values(server, virt, pllist, test_disk) + vsxml.destroy(server) + cleanup_restore(server, virt) + return status - # Taking care of already existing diskconf file - # Creating diskpool.conf if it does not exist - # Otherwise backing up the prev file and create new one. - os.system("rm -f %s" % back_disk_file ) - if not (os.path.exists(disk_file)): - conf_file() - else: - move_file(disk_file, back_disk_file) - conf_file() - try : - disk = get_or_bail(server, id=diskid, \ - pool_class=enumclass.Xen_DiskPool) - mem = get_or_bail(server, id = memid, \ - pool_class=enumclass.Xen_MemoryPool) - netid = "%s/%s" % ("NetworkPool", virt_network) - net = get_or_bail(server, id = netid, \ - pool_class=enumclass.Xen_NetworkPool) - proc = get_or_bail(server, id = procid, \ - pool_class=enumclass.Xen_ProcessorPool) - - except Exception, detail: - Globals.logger.error("Exception: %s", detail) - clean_up_restore(server) - status = FAIL - ret = test_domain_function(test_dom, server, \ - cmd = "destroy") - return status - - pllist, cllist, prop_list, proc_prop = init_list(server, disk, mem, net, proc) - -# Looping through the pllist to get association for various pools. - for cn, instid in sorted(pllist.items()): - try: - assoc_info = assoc.Associators(server, \ - "Xen_ElementAllocatedFromPool", \ - cn, \ - InstanceID = instid) -# Verifying the Creation Class name for all the records returned for each -# pool class queried - inst_list = get_inst_for_dom(assoc_info) - if (len(inst_list)) == 0: - Globals.logger.error("Association did not return any records for \ -the specified domain: %s", test_dom) - status = FAIL - break - - assoc_values(assoc_list=inst_list, field="CreationClassName", \ - list=cllist, \ - index=loop) -# verifying the DeviceID - if inst_list[0]['CreationClassName'] == 'Xen_Processor': -# The DeviceID for the processor varies from 0 to (vcpu - 1 ) - list_index = 0 - assoc_values(assoc_list=inst_list, field="DeviceID", \ - list=proc_prop, \ - index=list_index) - else: -# For LogicalDisk, Memory and NetworkPort - if inst_list[0]['CreationClassName'] == 'Xen_LogicalDisk': - list_index = 0 - elif inst_list[0]['CreationClassName'] == 'Xen_Memory': - list_index = 2 - else: - list_index = 4 # NetworkPort - assoc_values(assoc_list=inst_list, field="DeviceID", \ - list=prop_list, \ - index=list_index) - if inst_list[0]['CreationClassName'] == 'Xen_LogicalDisk': -# verifying the Name field for LogicalDisk - specific_fields = get_spec_fields_list(inst_list,field_name="Name") - list_index = 1 - elif inst_list[0]['CreationClassName'] == 'Xen_Memory': -# verifying the NumberOfBlocks allocated for Memory - specific_fields = get_spec_fields_list(inst_list,field_name="NumberOfBlocks") - list_index = 3 - else: -# verifying the NetworkAddresses for the NetworkPort - specific_fields = get_spec_fields_list(inst_list,field_name="NetworkAddresses") - list_index = 5 # NetworkPort - assoc_values(assoc_list=inst_list, field="Other", \ - list=prop_list, \ - index=list_index, \ - specific_fields_list=specific_fields) - if status != PASS: - break - else: -# The loop variable is used to index the cllist to verify the creationclassname - loop = loop + 1 - except Exception, detail: - Globals.logger.error(Globals.CIM_ERROR_ASSOCIATORS, \ - 'Xen_ElementAllocatedFromPool') - Globals.logger.error("Exception: %s", detail) - clean_up_restore(server) - status = FAIL - - ret = test_domain_function(test_dom, server, \ - cmd = "destroy") - clean_up_restore(server) - return status if __name__ == "__main__": sys.exit(main()) diff -r 3703b7be5a10 -r f8f6995ce089 suites/libvirt-cim/lib/XenKvmLib/logicaldevices.py --- a/suites/libvirt-cim/lib/XenKvmLib/logicaldevices.py Wed Jul 16 07:23:32 2008 -0700 +++ b/suites/libvirt-cim/lib/XenKvmLib/logicaldevices.py Fri Jul 18 05:29:39 2008 -0700 @@ -23,6 +23,7 @@ import os import os from CimTest.Globals import logger from CimTest.ReturnCodes import PASS, FAIL, SKIP +from XenKvmLib.vxml import get_typed_class # The list_values that is passed should be of the type .. # disk = { @@ -53,8 +54,9 @@ def spec_err(fieldvalue, field_list, fie logger.error("%s Mismatch", fieldname) logger.error("Returned %s instead of %s", fieldvalue, field_list[fieldname]) -def verify_proc_values(assoc_info, list_values): - proc_values = list_values['Xen_Processor'] +def verify_proc_values(assoc_info, list_values, virt='Xen'): + cn = get_typed_class(virt, 'Processor') + proc_values = list_values[cn] if assoc_info['CreationClassName'] != proc_values['CreationClassName']: field_err(assoc_info, proc_values, fieldname = 'CreationClassName') return FAIL @@ -67,8 +69,9 @@ def verify_proc_values(assoc_info, list_ return FAIL return PASS -def verify_mem_values(assoc_info, list_values): - mem_values = list_values['Xen_Memory'] +def verify_mem_values(assoc_info, list_values, virt='Xen'): + cn = get_typed_class(virt, 'Memory') + mem_values = list_values[cn] if assoc_info['CreationClassName'] != mem_values['CreationClassName']: field_err(assoc_info, mem_values, fieldname = 'CreationClassName') return FAIL @@ -85,8 +88,9 @@ def verify_mem_values(assoc_info, list_v return FAIL return PASS -def verify_net_values(assoc_info, list_values): - net_values = list_values['Xen_NetworkPort'] +def verify_net_values(assoc_info, list_values, virt='Xen'): + cn = get_typed_class(virt, 'NetworkPort') + net_values = list_values[cn] if assoc_info['CreationClassName'] != net_values['CreationClassName']: field_err(assoc_info, net_values, fieldname = 'CreationClassName') return FAIL @@ -97,16 +101,17 @@ def verify_net_values(assoc_info, list_v if sysname != net_values['SystemName']: spec_err(sysname, net_values, fieldname = 'SystemName') return FAIL -# We are assinging only one mac address and hence we expect only one -# address in the list + # We are assinging only one mac address and hence we expect only one + # address in the list netadd = assoc_info['NetworkAddresses'][0] if netadd != net_values['NetworkAddresses']: spec_err(netadd, net_values, fieldname = 'NetworkAddresses') return FAIL return PASS -def verify_disk_values(assoc_info, list_values): - disk_values = list_values['Xen_LogicalDisk'] +def verify_disk_values(assoc_info, list_values, virt='Xen'): + cn = get_typed_class(virt, 'LogicalDisk') + disk_values = list_values[cn] if assoc_info['CreationClassName'] != disk_values['CreationClassName']: field_err(assoc_info, disk_values, fieldname = 'CreationClassName') return FAIL

+def verify_eafp_values(server, virt, in_pllist, test_disk): + # Looping through the in_pllist to get association for various pools. + eafp_values = eafp_list(virt, test_disk) + an = get_typed_class(virt, "ElementAllocatedFromPool") + for cn, instid in sorted(in_pllist.iteritems()): + try: + assoc_info = Associators(server, an, cn, virt, InstanceID = instid) + assoc_inst_list = get_inst_for_dom(assoc_info) + if len(assoc_inst_list) < 1 : + logger.error("'%s' with '%s' did not return any records for" + " domain: '%s'", an, cn, test_dom)
This should be; logger.error("'%s' should have returned '%i' Processor" " details, got '%i'" % (an, test_vcpus, len(assoc_inst_list))) If you use commas instead, the formatting of the string is off.
+ return FAIL
-def assoc_values(assoc_list, field , list, index, specific_fields_list=""): - """ - Verifying the records retruned by the associations. - """ - global status - if field == "CreationClassName": - for i in range(len(assoc_list)): - if assoc_list[i][field] != list[index]: - print_error(field, assoc_list[i][field], list[index]) + assoc_eafp_info = assoc_inst_list[0] + CCName = assoc_eafp_info['CreationClassName'] + if CCName == get_typed_class(virt, 'Processor'): + if len(assoc_inst_list) != test_vcpus: + logger.error("'%s' should have returned '%i' Processor" + " details, got '%i'", an, test_vcpus, + len(assoc_inst_list))
Same here - format the log message using % (an, test_vcpus, len(assoc_inst_list))
+ + # Verify DiskPool on machine + status, diskid = create_diskpool_conf(server, virt) + if status != PASS: + return status + + ret = vsxml.create(server) + if not ret: + logger.error("Failed to Create the dom: '%s'", test_dom)
Will need to call cleanup_restore() in case of error.
return FAIL
sys.exit(main()) diff -r 3703b7be5a10 -r f8f6995ce089 suites/libvirt-cim/lib/XenKvmLib/logicaldevices.py --- a/suites/libvirt-cim/lib/XenKvmLib/logicaldevices.py Wed Jul 16 07:23:32 2008 -0700 +++ b/suites/libvirt-cim/lib/XenKvmLib/logicaldevices.py Fri Jul 18 05:29:39 2008 -0700 @@ -23,6 +23,7 @@ import os
-def verify_proc_values(assoc_info, list_values):
Instead of having a verify_<>_values function for each device type, could these be condensed into one function? All of them check the CreationClassName and the DeviceID, which isn't needed. The function could take the classname, which would enable you to determine with values need to be checked. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com

Kaitlin Rupert wrote:
+def verify_eafp_values(server, virt, in_pllist, test_disk): + # Looping through the in_pllist to get association for various pools. + eafp_values = eafp_list(virt, test_disk) + an = get_typed_class(virt, "ElementAllocatedFromPool") + for cn, instid in sorted(in_pllist.iteritems()): + try: + assoc_info = Associators(server, an, cn, virt, InstanceID = instid) + assoc_inst_list = get_inst_for_dom(assoc_info) + if len(assoc_inst_list) < 1 : + logger.error("'%s' with '%s' did not return any records for" + " domain: '%s'", an, cn, test_dom)
This should be;
logger.error("'%s' should have returned '%i' Processor" " details, got '%i'" % (an, test_vcpus, len(assoc_inst_list)))
If you use commas instead, the formatting of the string is off.
The above log stmt is just not for Processor. I will try to make it better. I did not find any formatting difference by using Commas and I got the same o/p for with/without commas. Did I misunderstand something ?
+ return FAIL
-def assoc_values(assoc_list, field , list, index, specific_fields_list=""): - """ - Verifying the records retruned by the associations. - """ - global status - if field == "CreationClassName": - for i in range(len(assoc_list)): - if assoc_list[i][field] != list[index]: - print_error(field, assoc_list[i][field], list[index]) + assoc_eafp_info = assoc_inst_list[0] + CCName = assoc_eafp_info['CreationClassName'] + if CCName == get_typed_class(virt, 'Processor'): + if len(assoc_inst_list) != test_vcpus: + logger.error("'%s' should have returned '%i' Processor" + " details, got '%i'", an, test_vcpus, + len(assoc_inst_list))
Same here - format the log message using % (an, test_vcpus, len(assoc_inst_list))
+ + # Verify DiskPool on machine + status, diskid = create_diskpool_conf(server, virt) + if status != PASS: + return status + + ret = vsxml.create(server) + if not ret: + logger.error("Failed to Create the dom: '%s'", test_dom)
Will need to call cleanup_restore() in case of error.
Yes, I missed this one. I will include this as well.
return FAIL
sys.exit(main()) diff -r 3703b7be5a10 -r f8f6995ce089 suites/libvirt-cim/lib/XenKvmLib/logicaldevices.py --- a/suites/libvirt-cim/lib/XenKvmLib/logicaldevices.py Wed Jul 16 07:23:32 2008 -0700 +++ b/suites/libvirt-cim/lib/XenKvmLib/logicaldevices.py Fri Jul 18 05:29:39 2008 -0700 @@ -23,6 +23,7 @@ import os
-def verify_proc_values(assoc_info, list_values):
Instead of having a verify_<>_values function for each device type, could these be condensed into one function? All of them check the CreationClassName and the DeviceID, which isn't needed. The function could take the classname, which would enable you to determine with values need to be checked.
I think DeviceID is a very important field that needs to be checked. Did you mean I should skip checking CreationClassName and DeviceID and check only Device specific details ? Thanks and Regards, Deepti.

Deepti B Kalakeri wrote:
Kaitlin Rupert wrote:
+def verify_eafp_values(server, virt, in_pllist, test_disk): + # Looping through the in_pllist to get association for various pools. + eafp_values = eafp_list(virt, test_disk) + an = get_typed_class(virt, "ElementAllocatedFromPool") + for cn, instid in sorted(in_pllist.iteritems()): + try: + assoc_info = Associators(server, an, cn, virt, InstanceID = instid) + assoc_inst_list = get_inst_for_dom(assoc_info) + if len(assoc_inst_list) < 1 : + logger.error("'%s' with '%s' did not return any records for" + " domain: '%s'", an, cn, test_dom)
This should be;
logger.error("'%s' should have returned '%i' Processor" " details, got '%i'" % (an, test_vcpus, len(assoc_inst_list)))
If you use commas instead, the formatting of the string is off.
The above log stmt is just not for Processor.
Oops, you are right. Sorry, I had copied this from a different part of the code and pasted it here accidentally. It should be: logger.error("'%s' with '%s' did not return any records for" " domain: '%s'" % (an, cn, test_dom)) I will try to make it better.
I did not find any formatting difference by using Commas and I got the same o/p for with/without commas. Did I misunderstand something ?
You're right, it is printing correctly. In the past, I'd seen the string not be formatted correctly with the arguments passed this way. But maybe that was an old issue. It appears to be working fine now. You can ignore this comment. =)
-def verify_proc_values(assoc_info, list_values):
Instead of having a verify_<>_values function for each device type, could these be condensed into one function? All of them check the CreationClassName and the DeviceID, which isn't needed. The function could take the classname, which would enable you to determine with values need to be checked.
I think DeviceID is a very important field that needs to be checked. Did you mean I should skip checking CreationClassName and DeviceID and check only Device specific details ?
I agree - I think checking the DeviceID is important. What I meant is that it's redundant to have each function checking the same value. I think these could be consolidated into something like: def verify_device_values(assoc_info, list_values, cn, virt='Xen'): values = list_values[cn] if assoc_info['CreationClassName'] != values['CreationClassName']: field_err(assoc_info, values, fieldname = 'CreationClassName') return FAIL if assoc_info['DeviceID'] != values['DeviceID']: field_err(assoc_info, values, fieldname = 'DeviceID') return FAIL if cn == get_typed_class(virt, 'Processor'): sysname = assoc_info['SystemName'] if sysname != values['SystemName']: spec_err(sysname, values, fieldname = 'SystemName') return FAIL # Handle other device types here return PASS If you think it's cleaner, you could have something like: if cn == get_typed_class(virt, 'Processor'): return verify_proc_values() Either approach is fine. Creating one function (or a wrapper function) will clean up the elif portion of verify_eafp_values() some. Thoughts? -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
participants (3)
-
Deepti B Kalakeri
-
Deepti B. Kalakeri
-
Kaitlin Rupert