[PATCH] [TEST] Fixing 04_hs_to_EAPF.py tc
# HG changeset patch # User Deepti B. Kalakeri <deeptik@linux.vnet.ibm.com> # Date 1218478580 25200 # Node ID 3eca51b419cc44654c94c21f32c4361632a43fac # Parent 0b5dea4e96cd0e128c0da6019cc545964deeba49 [TEST] Fixing 04_hs_to_EAPF.py tc. 1) Fixed the 04_hs_to_EAPF.py tc. 2) Set test_disk appropriately. 3) Improved pool_init_list() fn to include only pool info belonging to which the VS belongs. 4) Cleaned up unnecessary declarations and import stmt. 5) Replaced create_diskpool_file() function to create a DiskPool, with create_diskpool_conf() which calls either creating of diskconf file or using the diskpool depending on the version of the libvirt on the machine. Tested with Xen, XenFV and KVM on latest sources. This tc might not work on LXC with these changes. Signed-off-by: Deepti B. Kalakeri <deeptik@linux.vnet.ibm.com> diff -r 0b5dea4e96cd -r 3eca51b419cc suites/libvirt-cim/cimtest/HostSystem/04_hs_to_EAPF.py --- a/suites/libvirt-cim/cimtest/HostSystem/04_hs_to_EAPF.py Mon Aug 11 00:42:50 2008 -0700 +++ b/suites/libvirt-cim/cimtest/HostSystem/04_hs_to_EAPF.py Mon Aug 11 11:16:20 2008 -0700 @@ -59,8 +59,8 @@ from CimTest.ReturnCodes import PASS, FA from CimTest.ReturnCodes import PASS, FAIL, SKIP from XenKvmLib.test_doms import destroy_and_undefine_all from XenKvmLib.logicaldevices import verify_device_values -from XenKvmLib.common_util import cleanup_restore, test_dpath, \ -create_diskpool_file, create_netpool_conf, destroy_netpool +from XenKvmLib.common_util import cleanup_restore, \ +create_diskpool_conf, destroy_netpool sup_types = ['Xen', 'KVM', 'XenFV', 'LXC'] @@ -68,27 +68,34 @@ test_mac = "00:11:22:33:44:aa" test_mac = "00:11:22:33:44:aa" test_mem = 128 test_vcpus = 1 -test_disk = "xvdb" -diskid = "%s/%s" % ("DiskPool", test_dpath) -memid = "%s/%s" % ("MemoryPool", 0) -procid = "%s/%s" % ("ProcessorPool", 0) def print_err(err, detail, cn): logger.error(err % cn) logger.error("Exception: %s", detail) -def pool_init_list(pool_assoc): +def pool_init_list(virt, pool_assoc, net_name, dpool_name): """ Creating the lists that will be used for comparisons. """ in_pllist = {} + npool = get_typed_class(virt, 'NetworkPool') + dpool = get_typed_class(virt, 'DiskPool') + ppool = get_typed_class(virt, 'ProcessorPool') + mpool = get_typed_class(virt, 'MemoryPool') + n_instid = '%s/%s' % ('NetworkPool', net_name) + for i in range(len(pool_assoc)): classname_keyvalue = pool_assoc[i].classname instid = pool_assoc[i]['InstanceID'] - in_pllist[classname_keyvalue] = instid + if classname_keyvalue == npool and instid == n_instid: + in_pllist[classname_keyvalue] = instid + elif classname_keyvalue == dpool and instid == dpool_name: + in_pllist[classname_keyvalue] = instid + elif classname_keyvalue == ppool or classname_keyvalue == mpool: + in_pllist[classname_keyvalue] = instid return in_pllist -def eapf_list(server, virt="Xen"): +def eapf_list(server, virt, test_disk): disk_inst = get_typed_class(virt, "LogicalDisk") proc_inst = get_typed_class(virt, "Processor") net_inst = get_typed_class(virt, "NetworkPort") @@ -136,7 +143,7 @@ def get_inst_for_dom(assoc_val): return dom_list -def get_assocname_info(server, cn, an, qcn, hostname, virt="Xen"): +def get_assocname_info(server, cn, an, qcn, hostname, virt): status = PASS assoc_info = [] try: @@ -155,9 +162,6 @@ def get_assocname_info(server, cn, an, q print_err(CIM_ERROR_ASSOCIATORNAMES, detail, cn) status = FAIL - if status != PASS: - cleanup_restore(server, virt='Xen') - return status, assoc_info def check_len(an, assoc_list_info, qcn, exp_len): @@ -167,14 +171,14 @@ def check_len(an, assoc_list_info, qcn, return FAIL return PASS -def verify_eafp_values(server, in_pllist, virt="Xen"): +def verify_eafp_values(server, in_pllist, virt, test_disk): # Looping through the in_pllist to get association for various pools. status = PASS an = get_typed_class(virt, "ElementAllocatedFromPool") exp_len = 1 - qcn = "Logical Devices" - eafp_values = eapf_list(server, virt) + eafp_values = eapf_list(server, virt, test_disk) for cn, instid in sorted(in_pllist.items()): + qcn = cn try: assoc_info = Associators(server, an, cn, virt, InstanceID = instid) inst_list = get_inst_for_dom(assoc_info) @@ -185,14 +189,12 @@ def verify_eafp_values(server, in_pllist CCName = assoc_eafp_info['CreationClassName'] status = verify_device_values(assoc_eafp_info, eafp_values, virt) - if status != PASS: return status - except Exception, detail: logger.error(CIM_ERROR_ASSOCIATORS, an) logger.error("Exception: %s", detail) - cleanup_restore(server, virt='Xen') + cleanup_restore(server, virt) status = FAIL return status @@ -200,38 +202,37 @@ def main(): def main(): options= main.options server = options.ip - if options.virt == "XenFV": - virt = "Xen" - else: - virt=options.virt + virt=options.virt # Get the host info status, host_name, classname = get_host_info(server, virt) if status != PASS: return status + destroy_and_undefine_all(server) + if virt == 'Xen': + test_disk = 'xvdb' + else: + test_disk = 'hdb' + virt_type = get_class(virt) if virt == 'LXC': - vsxml = virt_type(test_dom) + vsxml = virt_type(test_dom, ntype="network") else: vsxml = virt_type(test_dom, vcpus = test_vcpus, mac = test_mac, - disk = test_disk) + disk = test_disk, ntype="network") ret = vsxml.define(server) if not ret: logger.error("Failed to define the dom: '%s'", test_dom) return FAIL - # Verify DiskPool on machine - status = create_diskpool_file() + + # Get the network pool info which is used by the VS. + net_name = vsxml.xml_get_net_network() + + status, dpool_name = create_diskpool_conf(server, virt) if status != PASS: logger.error("Failed to create diskpool") - vsxml.undefine(server) - return FAIL - # Create NetPool on machine - status, net_name = create_netpool_conf(server, virt, use_existing=False) - if status != PASS: - logger.error('Unable to create network pool') vsxml.undefine(server) - cleanup_restore(server, virt=virt) return FAIL # Get the hostedResourcePool info first @@ -244,6 +245,7 @@ def main(): cleanup_restore(server, virt=virt) destroy_netpool(server, virt, net_name) return status + # One pool for each Device type, hence len should be 4 exp_len = 4 status = check_len(an, pool, qcn, exp_len) @@ -253,13 +255,14 @@ def main(): destroy_netpool(server, virt, net_name) return FAIL - in_pllist = pool_init_list(pool) + in_pllist = pool_init_list(virt, pool, net_name, dpool_name) if virt == 'LXC': for cn, instid in in_pllist.items(): if cn == 'LXC_MemoryPool': in_pllist = {cn: instid} break - status = verify_eafp_values(server, in_pllist, virt) + + status = verify_eafp_values(server, in_pllist, virt, test_disk) vsxml.undefine(server) cleanup_restore(server, virt=virt) destroy_netpool(server, virt, net_name)
-def pool_init_list(pool_assoc): +def pool_init_list(virt, pool_assoc, net_name, dpool_name): """ Creating the lists that will be used for comparisons. """ in_pllist = {} + npool = get_typed_class(virt, 'NetworkPool') + dpool = get_typed_class(virt, 'DiskPool') + ppool = get_typed_class(virt, 'ProcessorPool') + mpool = get_typed_class(virt, 'MemoryPool') + n_instid = '%s/%s' % ('NetworkPool', net_name) + for i in range(len(pool_assoc)): classname_keyvalue = pool_assoc[i].classname instid = pool_assoc[i]['InstanceID'] - in_pllist[classname_keyvalue] = instid + if classname_keyvalue == npool and instid == n_instid: + in_pllist[classname_keyvalue] = instid + elif classname_keyvalue == dpool and instid == dpool_name: + in_pllist[classname_keyvalue] = instid + elif classname_keyvalue == ppool or classname_keyvalue == mpool: + in_pllist[classname_keyvalue] = instid return in_pllist
This loop is a little hard to read. I think this loop can be removed. Instead, just build a list with the values you're expecting. in_pllist = { npool, '%s/%s' % ('NetworkPool', net_name), ... } I know this was already existing code, but if pool_assoc doesn't contain the instances you're expecting, then in_pllist won't contain the proper values to query against ElementAllocatedFromPool. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
-def pool_init_list(pool_assoc): +def pool_init_list(virt, pool_assoc, net_name, dpool_name): """ Creating the lists that will be used for comparisons. """ in_pllist = {} + npool = get_typed_class(virt, 'NetworkPool') + dpool = get_typed_class(virt, 'DiskPool') + ppool = get_typed_class(virt, 'ProcessorPool') + mpool = get_typed_class(virt, 'MemoryPool') + n_instid = '%s/%s' % ('NetworkPool', net_name) + for i in range(len(pool_assoc)): classname_keyvalue = pool_assoc[i].classname instid = pool_assoc[i]['InstanceID'] - in_pllist[classname_keyvalue] = instid + if classname_keyvalue == npool and instid == n_instid: + in_pllist[classname_keyvalue] = instid + elif classname_keyvalue == dpool and instid == dpool_name: + in_pllist[classname_keyvalue] = instid + elif classname_keyvalue == ppool or classname_keyvalue == mpool: + in_pllist[classname_keyvalue] = instid return in_pllist
This loop is a little hard to read. I think this loop can be removed. Instead, just build a list with the values you're expecting.
in_pllist = { npool, '%s/%s' % ('NetworkPool', net_name), ... }
I know this was already existing code, but if pool_assoc doesn't contain the instances you're expecting, then in_pllist won't contain the proper values to query against ElementAllocatedFromPool. I agree that the loop is little hard to read, but considering the fact
Kaitlin Rupert wrote: that this is a Cross class test case where we would like to be able to make use of the output from the previous provider as input for the next step as much as possible, do you think that manually generating a list of our own will serve the purpose ? If yes, then the Crosss Class tc will no longer require query from the *HostedResourcePool, *correct?? Also, the above *if condition*s checks along with the following check after the *pool_init_list()* is called in the *main loop *make sure that we have *4* *required resourcepool *as inputs for *EAFP *query. in_pllist = pool_init_list(virt, pool, net_name, dpool_name) # One pool for each Device type, hence len should be 4 exp_len = 4 status = check_len(an, in_pllist, qcn, exp_len) if status != PASS: vsxml.undefine(server) cleanup_restore(server, virt=virt) destroy_netpool(server, virt, net_name) return FAIL Apart from this, the above *pool_init_list() *function can be made more readable as follows: def pool_init_list(virt, pool_assoc, net_name, dp_InstID): """ Creating the lists that will be used for comparisons. """ in_pllist = {} dp_CName = get_typed_class(virt, 'DiskPool') pp_CName = get_typed_class(virt, 'ProcessorPool') mp_CName = get_typed_class(virt, 'MemoryPool') np_CName = get_typed_class(virt, 'NetworkPool') np_InstID = '%s/%s' % ('NetworkPool', net_name) for p_inst in pool_assoc: CName = p_inst.classname InstID = p_inst['InstanceID'] if CName == np_CName and InstID == np_InstID: in_pllist[CName] = InstID elif CName == dp_CName and InstID == dp_InstID: in_pllist[CName] = InstID elif CName == pp_CName or CName == mp_CName: in_pllist[CName] = InstID return in_pllist Inputs are welcome as always :). Thanks and Regards, Deepti.
I know this was already existing code, but if pool_assoc doesn't contain the instances you're expecting, then in_pllist won't contain the proper values to query against ElementAllocatedFromPool.
I agree that the loop is little hard to read, but considering the fact that this is a Cross class test case where we would like to be able to as input for the next step as much as possible, do you think that manually generating a list of our own will serve the purpose ? If yes, then the Crosss Class tc will no longer require query from the *HostedResourcePool, *correct??
This is a really good point. I won't considering this a cross class test when I first read the patch.
def pool_init_list(virt, pool_assoc, net_name, dp_InstID): """ Creating the lists that will be used for comparisons. """ in_pllist = {} dp_CName = get_typed_class(virt, 'DiskPool') pp_CName = get_typed_class(virt, 'ProcessorPool') mp_CName = get_typed_class(virt, 'MemoryPool') np_CName = get_typed_class(virt, 'NetworkPool') np_InstID = '%s/%s' % ('NetworkPool', net_name)
for p_inst in pool_assoc: CName = p_inst.classname InstID = p_inst['InstanceID'] if CName == np_CName and InstID == np_InstID: in_pllist[CName] = InstID elif CName == dp_CName and InstID == dp_InstID: in_pllist[CName] = InstID elif CName == pp_CName or CName == mp_CName: in_pllist[CName] = InstID return in_pllist
What confuses me here are the comparisons: The outcome of each if statement is the same. You're essentially building a list that is: in_pllist = { Cname : InstID, ... } Why only verify the network pool and disk pool InstanceIDs but not the memory pool or processor pool InstanceIDs? If you want to verify in_pllist only has InstanceIDs you're expecting, you should check for proc and mem as well. Why not do something like: exp_pllist = { dp_CName : dp_InstID, ... } for p_inst in pool_assoc: CName = p_inst.classname InstID = p_inst['InstanceID'] if exp_pllist[Cname] == InstID: in_pllist[Cname] = InstID This way, you can build a list of expected values. in_pllist should match exp_pllist. You could then call check_len() at this point, or you could call it after pool_init_list() returns. Although, this seems silly. You could do something like: for p_inst in pool_assoc: CName = p_inst.classname InstID = p_inst['InstanceID'] if exp_pllist[Cname] != InstID: return FAIL, None return PASS, exp_pllist I agree that you want to use the output from the previous provider. However, if you've already confirmed that A == B, then returning A is no different than returning B. But either way is fine in this case. I just think the multiple if statements in the loop were confusing - it took me awhile to understand what that loop was trying to accomplish. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
Kaitlin Rupert wrote:
I know this was already existing code, but if pool_assoc doesn't contain the instances you're expecting, then in_pllist won't contain the proper values to query against ElementAllocatedFromPool.
I agree that the loop is little hard to read, but considering the fact that this is a Cross class test case where we would like to be able to as input for the next step as much as possible, do you think that manually generating a list of our own will serve the purpose ? If yes, then the Crosss Class tc will no longer require query from the *HostedResourcePool, *correct??
This is a really good point. I won't considering this a cross class test when I first read the patch.
def pool_init_list(virt, pool_assoc, net_name, dp_InstID): """ Creating the lists that will be used for comparisons. """ in_pllist = {} dp_CName = get_typed_class(virt, 'DiskPool') pp_CName = get_typed_class(virt, 'ProcessorPool') mp_CName = get_typed_class(virt, 'MemoryPool') np_CName = get_typed_class(virt, 'NetworkPool') np_InstID = '%s/%s' % ('NetworkPool', net_name)
for p_inst in pool_assoc: CName = p_inst.classname InstID = p_inst['InstanceID'] if CName == np_CName and InstID == np_InstID: in_pllist[CName] = InstID elif CName == dp_CName and InstID == dp_InstID: in_pllist[CName] = InstID elif CName == pp_CName or CName == mp_CName: in_pllist[CName] = InstID return in_pllist
What confuses me here are the comparisons: The outcome of each if statement is the same. You're essentially building a list that is:
in_pllist = { Cname : InstID, ... }
Why only verify the network pool and disk pool InstanceIDs but not the memory pool or processor pool InstanceIDs? If you want to verify in_pllist only has InstanceIDs you're expecting, you should check for proc and mem as well.
Since the processorpool and memorypool InstanceID do not change I did not consider checking them as well.
Why not do something like:
exp_pllist = { dp_CName : dp_InstID, ... }
for p_inst in pool_assoc: CName = p_inst.classname InstID = p_inst['InstanceID'] if exp_pllist[Cname] == InstID: in_pllist[Cname] = InstID
This way, you can build a list of expected values. in_pllist should match exp_pllist. You could then call check_len() at this point, or you could call it after pool_init_list() returns.
Although, this seems silly. You could do something like:
for p_inst in pool_assoc: CName = p_inst.classname InstID = p_inst['InstanceID'] if exp_pllist[Cname] != InstID: return FAIL, None
return PASS, exp_pllist
I agree that you want to use the output from the previous provider. However, if you've already confirmed that A == B, then returning A is no different than returning B.
But either way is fine in this case. I just think the multiple if statements in the loop were confusing - it took me awhile to understand what that loop was trying to accomplish.
We should be returning the new list B that will be created since the HostedResourcePool might not contain all the Pool details that exp_list is expecting to have. These are good approach :) thanks. I sent a new patch with the changes. Thanks and Regards, Deepti.
participants (3)
-
Deepti B Kalakeri -
Deepti B. Kalakeri
-
Kaitlin Rupert