>
> 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(a)linux.vnet.ibm.com