1:17 p.m.
# HG changeset patch
# User Deepti B. Kalakeri <deeptik(a)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(a)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)
4:10 p.m.
-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(a)linux.vnet.ibm.com
2:50 a.m.
Kaitlin Rupert wrote:
> -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
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.
5:06 p.m.
>
> 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
7:38 a.m.
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.
5975
days inactive
5977
days old
4 comments
3 participants
participants (3)
-
Deepti B Kalakeri -
Deepti B. Kalakeri
-
Kaitlin Rupert