libvirt-cim-bounces@redhat.com wrote on 2008-12-25 05:44:15:

> > +import sys
> > +import pywbem
>
> pywbem is not used
>
> > +from VirtLib import utils
>
> utils is not used
>
> > +from XenKvmLib import enumclass
>
> Instead of importing all of the enumclass module, instead you can import
> just the EnumInstances() function:
>
> from XenKvmLib.enumclass import EnumInstances
>
> > +from XenKvmLib.classes import get_typed_class
> > +from XenKvmLib.test_doms import undefine_test_domain
>
> undefine_test_domain() isn't used, this can be removed
>
> > +from XenKvmLib.common_util import parse_instance_id
> > +from XenKvmLib.const import do_main
> > +from XenKvmLib import vxml
>
> Since you only use get_class(), it's better to do:
>
> from XenKvmLib.vxml import get_class
>
> > +from CimTest.ReturnCodes import PASS, FAIL
> > +from CimTest.Globals import logger, CIM_ERROR_ENUMERATE
>
> CIM_ERROR_ENUMERATE is not used
>
> > +from XenKvmLib.const import get_provider_version
> > +
> > +SUPPORTED_TYPES = ['KVM']
>
> This should also support Xen and XenFV.
>
  I cann't connect to the machine for Xen test, so #6 patch only supports KVM.
  Once the machine brings up, I will work on follow up patch.

  Thanks!
 
> > +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:
> > +                raise Exception("%s returned %i instances,
> excepted at least 1.",
>
> Typo - excepted should be expected.  Also, this line is 81 characters -
> removing the ".' should fix that =)
>
> > +                                drasd, len(drasd_list))
> > +
> > +            found_rasd = None
> > +            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:
> > +                        found_rasd = rasd
> > +                        status = PASS
> > +                        break
> > +                    else:
> > +                        raise Exception("EmulatedType Mismatch: got %d, \
> > +                              expected %d", rasd['EmulatedType'],
> exp_emu_type)
>
> The indention is off here.  Also, you'll need to use % not commas.  This
> should be:
>
> raise Exception("EmulatedType Mismatch: got %d, \
>                   expected %d" % (rasd['EmulatedType'],
>                   exp_emu_type))
>
> > +
> > +            if found_rasd == None:
>
> This should be:  if found_rasd is None
>
> > +                raise Exception("The defined dom is not found")
>
> This error message is a bit vague.. it should be something like
> "DiskRASD for define guest was not found"
>
> > +        except Exception, detail:
> > +            logger.error("Exception: %s", detail)
> > +            cxml.undefine(options.ip)
>
> No need to have the undefine() call here.  Instead, change the return
> FAIL to status = FAIL and remove the undefine() call.
>
> > +            return FAIL
> > +
> > +        cxml.undefine(options.ip)
>
> Put the undefine() outside of the for loop.  The for loop should be in
> the try block.  So your code should look like:
>
> try:
>      for ...
>
>      ....
>
> except Exception, detail:
>      logger.error("Exception: %s", detail)
>      status = FAIL
>
> cxml.undefine(options.ip)
>
> return status
>
> >  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 patch doesn't update the schema for LXC - it's Xen and KVM only.
>
> > diff -r b7d80fdeb2a3 -r 7b57ee2aab2c suites/libvirt-
> cim/lib/XenKvmLib/vxml.py
> > --- a/suites/libvirt-cim/lib/XenKvmLib/vxml.py   Mon Dec 22 23:03:
> 01 2008 -0800
> > +++ b/suites/libvirt-cim/lib/XenKvmLib/vxml.py   Tue Dec 23 22:53:
> 59 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):
>
> Don't have emu_type=None - have it be a mandatory value like all the
> others. The init for KVM sets it as None if nothing is passed in.
>
> --
> Kaitlin Rupert
> IBM Linux Technology Center
> kaitlin@linux.vnet.ibm.com
>
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim@redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim