[PATCH] [TEST]#3 Add new test to verify enum of DiskRASD to have EmulatedType=0 for Disk and EmulatedType=1 for CDROM

# HG changeset patch # User Guolian Yun <yunguol@cn.ibm.com> # Date 1230011917 28800 # Node ID 5ca127a33c44467f28b564062b11cb93be29f178 # Parent a1eb0390e4bb3bb1a468411a33e5c666fe998bee [TEST]#3 Add new test to verify enum of DiskRASD to have EmulatedType=0 for Disk and EmulatedType=1 for CDROM Updates from 2 to 3: 1) Return error if the defined guest is not found 2) Test both EmulatedType = 1 and EmulatedType = 0 in this tc Signed-off-by: Guolian Yun <yunguol@cn.ibm.com> diff -r a1eb0390e4bb -r 5ca127a33c44 suites/libvirt-cim/cimtest/RASD/05_disk_rasd_emu_type.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/suites/libvirt-cim/cimtest/RASD/05_disk_rasd_emu_type.py Mon Dec 22 21:58:37 2008 -0800 @@ -0,0 +1,90 @@ +#!/usr/bin/python +# +# Copyright 2008 IBM Corp. +# +# Authors: +# Guolian Yun <yunguol@cn.ibm.com> +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +# + +import sys +import pywbem +from VirtLib import utils +from XenKvmLib import enumclass +from XenKvmLib.classes import get_typed_class +from XenKvmLib.test_doms import undefine_test_domain +from XenKvmLib.common_util import parse_instance_id +from XenKvmLib.const import do_main +from XenKvmLib import vxml +from CimTest.ReturnCodes import PASS, FAIL +from CimTest.Globals import logger, CIM_ERROR_ENUMERATE +from XenKvmLib.const import get_provider_version + +SUPPORTED_TYPES = ['KVM'] +default_dom = 'test_domain' +libvirt_em_type_changeset = 737 + +@do_main(SUPPORTED_TYPES) +def main(): + status = FAIL + options = main.options + curr_cim_rev, changeset = get_provider_version(options.virt, options.ip) + if curr_cim_rev < libvirt_em_type_changeset: + return SKIP + + emu_types = [0, 1] + for exp_emu_type in emu_types: + cxml = vxml.get_class(options.virt)(default_dom, emu_type=exp_emu_type) + ret = cxml.cim_define(options.ip) + if not ret: + logger.error("Failed to call DefineSystem()") + return FAIL + + drasd= get_typed_class(options.virt, 'DiskResourceAllocationSettingData') + try: + drasd_list = enumclass.EnumInstances(options.ip, drasd, ret_cim_inst=True) + if len(drasd_list) < 1: + logger.error("%s returned %i instances, excepted at least 1.", + drasd, len(drasd_list)) + return FAIL + except Exception, detail: + logger.error(CIM_ERROR_ENUMERATE, drasd) + logger.error("Exception: %s", detail) + return FAIL + + for rasd in drasd_list: + guest, dev, status = parse_instance_id(rasd['InstanceID']) + if status != PASS: + logger.error("Unable to parse InstanceID: %s" % rasd['InstanceID']) + return FAIL + if guest == default_dom: + if rasd['EmulatedType'] == exp_emu_type: + status = PASS + break + else: + logger.error("EmulatedType Mismatch: got %d, expected %d",\ + rasd['EmulatedType'], exp_emu_type) + return FAIL + elif rasd == drasd_list[len(drasd_list)-1]: + logger.error("The defined guest can not be found") + return FAIL + + cxml.undefine(options.ip) + + return status + +if __name__ == "__main__": + sys.exit(main()) diff -r a1eb0390e4bb -r 5ca127a33c44 suites/libvirt-cim/lib/XenKvmLib/vsms.py --- a/suites/libvirt-cim/lib/XenKvmLib/vsms.py Mon Dec 22 02:56:58 2008 -0800 +++ b/suites/libvirt-cim/lib/XenKvmLib/vsms.py Mon Dec 22 21:58:37 2008 -0800 @@ -126,8 +126,10 @@ # classes to define RASD parameters class CIM_DiskResourceAllocationSettingData(CIMClassMOF): - def __init__(self, dev, source, name): + def __init__(self, dev, source, name, emu_type=None): self.ResourceType = RASD_TYPE_DISK + if emu_type != None: + self.EmulatedType = emu_type if dev != None: self.VirtualDevice = dev self.InstanceID = '%s/%s' % (name, dev) @@ -141,10 +143,12 @@ pass class LXC_DiskResourceAllocationSettingData(CIMClassMOF): - def __init__(self, mountpoint, source, name): + def __init__(self, mountpoint, source, name, emu_type=None): self.MountPoint = mountpoint self.Address = source self.InstanceID = '%s/%s' % (name, mountpoint) + if emu_type != None: + self.EmulatedType = emu_type @eval_cls('DiskResourceAllocationSettingData') def get_dasd_class(virt): @@ -239,6 +243,7 @@ proc_vcpu=1, mem_mb=512, malloc_units="MegaBytes", + emu_type=None, virt='Xen'): vssd = get_vssd_mof(virt, dom_name) @@ -252,7 +257,7 @@ elif virt == 'LXC': disk_dev = const.LXC_default_mp disk_source = const.LXC_default_source - d = class_dasd(disk_dev, disk_source, dom_name) + d = class_dasd(disk_dev, disk_source, dom_name, emu_type) class_masd = get_masd_class(virt) m = class_masd( diff -r a1eb0390e4bb -r 5ca127a33c44 suites/libvirt-cim/lib/XenKvmLib/vxml.py --- a/suites/libvirt-cim/lib/XenKvmLib/vxml.py Mon Dec 22 02:56:58 2008 -0800 +++ b/suites/libvirt-cim/lib/XenKvmLib/vxml.py Mon Dec 22 21:58:37 2008 -0800 @@ -466,11 +466,13 @@ class VirtCIM: def __init__(self, virt, dom_name, disk_dev, disk_source, - net_type, net_name, net_mac, vcpus, mem, mem_allocunits): + net_type, net_name, net_mac, vcpus, mem, + mem_allocunits, emu_type=None): self.virt = virt self.domain_name = dom_name self.vssd = vsms.get_vssd_mof(virt, dom_name) - self.dasd = vsms.get_dasd_class(virt)(disk_dev, disk_source, dom_name) + self.dasd = vsms.get_dasd_class(virt)(disk_dev, disk_source, + dom_name, emu_type) self.nasd = vsms.get_nasd_class(virt)(type=net_type, mac=net_mac, name=dom_name, @@ -695,13 +697,15 @@ disk_file_path=const.KVM_disk_path, disk=const.KVM_default_disk_dev, ntype=const.default_net_type, - net_name=const.default_network_name): + net_name=const.default_network_name, + emu_type=None): if not os.path.exists(disk_file_path): logger.error('Error: Disk image does not exist') sys.exit(1) VirtXML.__init__(self, 'kvm', test_dom, set_uuid(), mem, vcpus) VirtCIM.__init__(self, 'KVM', test_dom, disk, disk_file_path, - ntype, net_name, mac, vcpus, mem, mem_allocunits) + ntype, net_name, mac, vcpus, mem, mem_allocunits, + emu_type) self._os() self._devices(const.KVM_default_emulator, ntype, disk_file_path, disk, mac, net_name)

yunguol@cn.ibm.com wrote:
# HG changeset patch # User Guolian Yun <yunguol@cn.ibm.com> # Date 1230011917 28800 # Node ID 5ca127a33c44467f28b564062b11cb93be29f178 # Parent a1eb0390e4bb3bb1a468411a33e5c666fe998bee [TEST]#3 Add new test to verify enum of DiskRASD to have EmulatedType=0 for Disk and EmulatedType=1 for CDROM
Updates from 2 to 3: 1) Return error if the defined guest is not found 2) Test both EmulatedType = 1 and EmulatedType = 0 in this tc
Signed-off-by: Guolian Yun <yunguol@cn.ibm.com>
diff -r a1eb0390e4bb -r 5ca127a33c44 suites/libvirt-cim/cimtest/RASD/05_disk_rasd_emu_type.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/suites/libvirt-cim/cimtest/RASD/05_disk_rasd_emu_type.py Mon Dec 22 21:58:37 2008 -0800 @@ -0,0 +1,90 @@ +#!/usr/bin/python +# +# Copyright 2008 IBM Corp. +# +# Authors: +# Guolian Yun <yunguol@cn.ibm.com> +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +# + +import sys +import pywbem +from VirtLib import utils +from XenKvmLib import enumclass +from XenKvmLib.classes import get_typed_class +from XenKvmLib.test_doms import undefine_test_domain +from XenKvmLib.common_util import parse_instance_id +from XenKvmLib.const import do_main +from XenKvmLib import vxml +from CimTest.ReturnCodes import PASS, FAIL +from CimTest.Globals import logger, CIM_ERROR_ENUMERATE +from XenKvmLib.const import get_provider_version + +SUPPORTED_TYPES = ['KVM'] +default_dom = 'test_domain' +libvirt_em_type_changeset = 737 + +@do_main(SUPPORTED_TYPES) +def main(): + status = FAIL + options = main.options + curr_cim_rev, changeset = get_provider_version(options.virt, options.ip) + if curr_cim_rev < libvirt_em_type_changeset: + return SKIP + + emu_types = [0, 1] + for exp_emu_type in emu_types: + cxml = vxml.get_class(options.virt)(default_dom, emu_type=exp_emu_type) + ret = cxml.cim_define(options.ip) + if not ret: + logger.error("Failed to call DefineSystem()") + return FAIL + + drasd= get_typed_class(options.virt, 'DiskResourceAllocationSettingData') + try: + drasd_list = enumclass.EnumInstances(options.ip, drasd, ret_cim_inst=True) + if len(drasd_list) < 1:
you can check for len(drasd_list) == 1 instead as we should get only one record.
+ logger.error("%s returned %i instances, excepted at least 1.", + drasd, len(drasd_list)) + return FAIL
The return statement leaves the guest defined on the machine. Call cxml.undefine() before returning.
+ except Exception, detail: + logger.error(CIM_ERROR_ENUMERATE, drasd) + logger.error("Exception: %s", detail) + return FAIL + + for rasd in drasd_list:
Do we really require the for loop, since we are sure that we will get only one DiskRASD, why not compare the rasd[0]['EmulatedType'] == exp_emu_type ??
+ guest, dev, status = parse_instance_id(rasd['InstanceID']) + if status != PASS: + logger.error("Unable to parse InstanceID: %s" % rasd['InstanceID']) + return FAIL + if guest == default_dom: + if rasd['EmulatedType'] == exp_emu_type: + status = PASS + break + else: + logger.error("EmulatedType Mismatch: got %d, expected %d",\ + rasd['EmulatedType'], exp_emu_type) + return FAIL + elif rasd == drasd_list[len(drasd_list)-1]: + logger.error("The defined guest can not be found") + return FAIL
I did not get how the above check rasd == drasd_list[len(drasd_list)-1] is useful in the tc ? Once the verification of the guest for a particular emu_type is successful we need to call cxml.undefine() before verifying for the next emu_type.
+ + cxml.undefine(options.ip) + + return status + +if __name__ == "__main__": + sys.exit(main()) diff -r a1eb0390e4bb -r 5ca127a33c44 suites/libvirt-cim/lib/XenKvmLib/vsms.py --- a/suites/libvirt-cim/lib/XenKvmLib/vsms.py Mon Dec 22 02:56:58 2008 -0800 +++ b/suites/libvirt-cim/lib/XenKvmLib/vsms.py Mon Dec 22 21:58:37 2008 -0800 @@ -126,8 +126,10 @@
# classes to define RASD parameters class CIM_DiskResourceAllocationSettingData(CIMClassMOF): - def __init__(self, dev, source, name): + def __init__(self, dev, source, name, emu_type=None): self.ResourceType = RASD_TYPE_DISK + if emu_type != None: + self.EmulatedType = emu_type if dev != None: self.VirtualDevice = dev self.InstanceID = '%s/%s' % (name, dev) @@ -141,10 +143,12 @@ pass
class LXC_DiskResourceAllocationSettingData(CIMClassMOF): - def __init__(self, mountpoint, source, name): + def __init__(self, mountpoint, source, name, emu_type=None): self.MountPoint = mountpoint self.Address = source self.InstanceID = '%s/%s' % (name, mountpoint) + if emu_type != None: + self.EmulatedType = emu_type
The support here for LXC was not required as of now as the tc only supports KVM.
@eval_cls('DiskResourceAllocationSettingData') def get_dasd_class(virt): @@ -239,6 +243,7 @@ proc_vcpu=1, mem_mb=512, malloc_units="MegaBytes", + emu_type=None, virt='Xen'): vssd = get_vssd_mof(virt, dom_name)
@@ -252,7 +257,7 @@ elif virt == 'LXC': disk_dev = const.LXC_default_mp disk_source = const.LXC_default_source - d = class_dasd(disk_dev, disk_source, dom_name) + d = class_dasd(disk_dev, disk_source, dom_name, emu_type)
class_masd = get_masd_class(virt) m = class_masd( diff -r a1eb0390e4bb -r 5ca127a33c44 suites/libvirt-cim/lib/XenKvmLib/vxml.py --- a/suites/libvirt-cim/lib/XenKvmLib/vxml.py Mon Dec 22 02:56:58 2008 -0800 +++ b/suites/libvirt-cim/lib/XenKvmLib/vxml.py Mon Dec 22 21:58:37 2008 -0800 @@ -466,11 +466,13 @@
class VirtCIM: def __init__(self, virt, dom_name, disk_dev, disk_source, - net_type, net_name, net_mac, vcpus, mem, mem_allocunits): + net_type, net_name, net_mac, vcpus, mem, + mem_allocunits, emu_type=None): self.virt = virt self.domain_name = dom_name self.vssd = vsms.get_vssd_mof(virt, dom_name) - self.dasd = vsms.get_dasd_class(virt)(disk_dev, disk_source, dom_name) + self.dasd = vsms.get_dasd_class(virt)(disk_dev, disk_source, + dom_name, emu_type) self.nasd = vsms.get_nasd_class(virt)(type=net_type, mac=net_mac, name=dom_name, @@ -695,13 +697,15 @@ disk_file_path=const.KVM_disk_path, disk=const.KVM_default_disk_dev, ntype=const.default_net_type, - net_name=const.default_network_name): + net_name=const.default_network_name, + emu_type=None): if not os.path.exists(disk_file_path): logger.error('Error: Disk image does not exist') sys.exit(1) VirtXML.__init__(self, 'kvm', test_dom, set_uuid(), mem, vcpus) VirtCIM.__init__(self, 'KVM', test_dom, disk, disk_file_path, - ntype, net_name, mac, vcpus, mem, mem_allocunits) + ntype, net_name, mac, vcpus, mem, mem_allocunits, + emu_type) self._os() self._devices(const.KVM_default_emulator, ntype, disk_file_path, disk, mac, net_name)
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim

+ try: + drasd_list = enumclass.EnumInstances(options.ip, drasd, ret_cim_inst=True) + if len(drasd_list) < 1:
you can check for len(drasd_list) == 1 instead as we should get only one record.
I disagree. If there are other guests defined on the system, EnumInstances() of DiskRASD may return more than one instance.
+ logger.error("%s returned %i instances, excepted at least 1.", + drasd, len(drasd_list)) + return FAIL
The return statement leaves the guest defined on the machine. Call cxml.undefine() before returning.
+ except Exception, detail: + logger.error(CIM_ERROR_ENUMERATE, drasd) + logger.error("Exception: %s", detail) + return FAIL + + for rasd in drasd_list:
Do we really require the for loop, since we are sure that we will get only one DiskRASD, why not compare the rasd[0]['EmulatedType'] == exp_emu_type ??
Yes, the loop here is required. If multiple DiskRASDs are returned, we need to filter the list to find the DiskRASD that corresponds with our guest.
+ guest, dev, status = parse_instance_id(rasd['InstanceID']) + if status != PASS: + logger.error("Unable to parse InstanceID: %s" % rasd['InstanceID']) + return FAIL + if guest == default_dom: + if rasd['EmulatedType'] == exp_emu_type: + status = PASS + break + else: + logger.error("EmulatedType Mismatch: got %d, expected %d",\ + rasd['EmulatedType'], exp_emu_type) + return FAIL + elif rasd == drasd_list[len(drasd_list)-1]: + logger.error("The defined guest can not be found") + return FAIL
I did not get how the above check rasd == drasd_list[len(drasd_list)-1] is useful in the tc ? Once the verification of the guest for a particular emu_type is successful we need to call cxml.undefine() before verifying for the next emu_type.
This is bit of code is attempting to make sure that a DiskRASD for the guest was actually returned. I think this can be done in an easier way.. See my review for info on that. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com

+ try: + drasd_list = enumclass.EnumInstances(options.ip, drasd, ret_cim_inst=True) + if len(drasd_list) < 1: you can check for len(drasd_list) == 1 instead as we should get only one record.
I disagree. If there are other guests defined on the system, EnumInstances() of DiskRASD may return more than one instance. Sorry, my mistake when I was reviewing this I had GetInstance in mind. Correct, we will get more than one DiskRASD when the machine has more
Kaitlin Rupert wrote: than one guest defined.
+ logger.error("%s returned %i instances, excepted at least 1.", + drasd, len(drasd_list)) + return FAIL The return statement leaves the guest defined on the machine. Call cxml.undefine() before returning. + except Exception, detail: + logger.error(CIM_ERROR_ENUMERATE, drasd) + logger.error("Exception: %s", detail) + return FAIL + + for rasd in drasd_list: Do we really require the for loop, since we are sure that we will get only one DiskRASD, why not compare the rasd[0]['EmulatedType'] == exp_emu_type ??
Yes, the loop here is required. If multiple DiskRASDs are returned, we need to filter the list to find the DiskRASD that corresponds with our guest.
Yes the loop is required for handling multiple DiskRASD's .
+ guest, dev, status = parse_instance_id(rasd['InstanceID']) + if status != PASS: + logger.error("Unable to parse InstanceID: %s" % rasd['InstanceID']) + return FAIL + if guest == default_dom: + if rasd['EmulatedType'] == exp_emu_type: + status = PASS + break + else: + logger.error("EmulatedType Mismatch: got %d, expected %d",\ + rasd['EmulatedType'], exp_emu_type) + return FAIL + elif rasd == drasd_list[len(drasd_list)-1]: + logger.error("The defined guest can not be found") + return FAIL I did not get how the above check rasd == drasd_list[len(drasd_list)-1] is useful in the tc ? Once the verification of the guest for a particular emu_type is successful we need to call cxml.undefine() before verifying for the next emu_type.
This is bit of code is attempting to make sure that a DiskRASD for the guest was actually returned. I think this can be done in an easier way.. See my review for info on that.

+ +SUPPORTED_TYPES = ['KVM']
This should work with other guest types as well.
+default_dom = 'test_domain' +libvirt_em_type_changeset = 737 + +@do_main(SUPPORTED_TYPES) +def main(): + status = FAIL + options = main.options + curr_cim_rev, changeset = get_provider_version(options.virt, options.ip) + if curr_cim_rev < libvirt_em_type_changeset: + return SKIP + + emu_types = [0, 1] + for exp_emu_type in emu_types: + cxml = vxml.get_class(options.virt)(default_dom, emu_type=exp_emu_type) + ret = cxml.cim_define(options.ip) + if not ret: + logger.error("Failed to call DefineSystem()") + return FAIL + + drasd= get_typed_class(options.virt, 'DiskResourceAllocationSettingData') + try: + drasd_list = enumclass.EnumInstances(options.ip, drasd, ret_cim_inst=True) + if len(drasd_list) < 1: + logger.error("%s returned %i instances, excepted at least 1.", + drasd, len(drasd_list)) Need to undefine the guest here.
+ return FAIL + except Exception, detail: + logger.error(CIM_ERROR_ENUMERATE, drasd) + logger.error("Exception: %s", detail)
Need to undefine the guest here.
+ return FAIL + + for rasd in drasd_list: + guest, dev, status = parse_instance_id(rasd['InstanceID']) + if status != PASS: + logger.error("Unable to parse InstanceID: %s" % rasd['InstanceID'])
Need to undefine the guest here.
+ return FAIL + if guest == default_dom: + if rasd['EmulatedType'] == exp_emu_type: + status = PASS + break
You don't need to break here if you use the raise exception solution (see below).
+ else: + logger.error("EmulatedType Mismatch: got %d, expected %d",\ + rasd['EmulatedType'], exp_emu_type) + return FAIL
You don't want to have this else statement here. If there are multiple guests defined, then you will return an error if the first DiskRASD in the list doesn't belong to our guest.
+ elif rasd == drasd_list[len(drasd_list)-1]: + logger.error("The defined guest can not be found") + return FAIL
This is an awkward check. Instead, you should define a variable like found_rasd before the for loop. Once you find a match, set found_rasd equal to the rasd that matches our guest. At the end of the loop, if found_rasd is None, then you know a match wasn't found and an error should be returned.
+ + cxml.undefine(options.ip) + + return status + +if __name__ == "__main__": + sys.exit(main())
Since there are so many places where you need to undefine the guest, you can do the following: for exp_emu_type in emu_types: cxml = vxml.get_class(options.virt)(default_dom, emu_type=exp_emu_type) ret = cxml.cim_define(options.ip) if not ret: logger.error("Failed to call DefineSystem()") return FAIL try: drasd= get_typed_class(options.virt, 'DiskResourceAllocationSettingData') drasd_list = enumclass.EnumInstances(options.ip, drasd, ret_cim_inst=True) if len(drasd_list) < 1: raise Exception("%s returned %i instances, excepted at least 1.", drasd, len(drasd_list)) for rasd in drasd_list: guest, dev, status = parse_instance_id(rasd['InstanceID']) if status != PASS: raise Exception("Unable to parse InstanceID: %s" % rasd['InstanceID']) if guest == default_dom: if rasd['EmulatedType'] == exp_emu_type: status = PASS except Exception, detail: logger.error(CIM_ERROR_ENUMERATE, drasd) logger.error("Exception: %s", detail) status = FAIL cxml.undefine(options.ip) if status != PASS: return status -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
participants (3)
-
Deepti B Kalakeri
-
Kaitlin Rupert
-
yunguol@cn.ibm.com