[Libvirt-cim] [PATCH] [TEST] Update RAFP.01 for LXC support

[Guo Lian Yun]

libvirt-cim-bounces at redhat.com wrote on 2008-06-25 22:37:55:

> yunguol at cn.ibm.com wrote:
> > # HG changeset patch
> > # User Guolian Yun <yunguol at cn.ibm.com>
> > # Date 1214371183 -28800
> > # Node ID c8c27374e304a66cd71d1f0b5d0fc462e230a898
> > # Parent  727d97c09d77d73f3542ba49a9dd19ba119a67eb
> > [TEST] Update RAFP.01 for LXC support
> > 
> 
> Can you keep the revision number in the subject next time around? 
> Having the revision number makes it easier to keep track of which patch 
> is the current version.
> 
> Also, please include a log of what has changed for each iteration of the 

> patch.
> 
  Sure. I will keep the revision number and log the changes for each 
patch.
  Thanks a lot for your great comments during the iteration of RAFP.01 
patch.
  Hopefully, the latest #7 revision works well.

 
> Thanks!
> 
> > +def verify_rasd(server, assoc_cn, cn, virt, list, rasd):
> > +    try:
> > +        data = assoc.AssociatorNames(server,
> > +                                     assoc_cn,
> > +                                     cn,
> > +                                     virt,
> > +                                     InstanceID=list)
> > +    except Exception:
> > +        logger.error(Globals.CIM_ERROR_ASSOCIATORNAMES % cn)
> > +        return FAIL
> > +
> > +    if len(data) < 1:
> > +        logger.error("Return NULL, expect at least one instance")
> > +        return FAIL
> > + 
> > +    for i in range(0, len(data)):
> > +        if data[i].classname == 
"LXC_MemResourceAllocationSettingData" \
> > +                   and data[i]['InstanceID'] == rasd["MemoryPool"]:
> > +            logger.info("MemoryPool InstanceID match")
> > +            return PASS
> > +        elif i == len(data) and data[i]['InstanceID'] != 
> rasd["MemoryPool"]:
> > +            logger.error("InstanceID Mismatch, expect %s not %s" % \
> > +                         (rasd['MemoryPool'], data[i]['InstanceID']))
> > +            return FAIL
> > +        if data[i].classname == 
"LXC_ProcResourceAllocationSettingData" \
> > +                   and data[i]['InstanceID'] == 
rasd["ProcessorPool"]:
> > +            logger.info("ProcessorPool InstanceID match")
> > +            return PASS
> > +        elif i == len(data) and data[i]['InstanceID'] != 
> rasd["ProcessorPool"]:
> > +            logger.error("InstanceID Mismatch, expect %s not %s" % \
> > +                         (rasd['ProcessorPool'], 
data[i]['InstanceID']))
> > +            return FAIL
> > +        if data[i].classname == 
"LXC_DiskResourceAllocationSettingData" \
> > +                   and data[i]['InstanceID'] == rasd["DiskPool"]:
> > +            logger.info("DiskPool InstanceID match")
> > +            return PASS
> > +        elif i == len(data) and data[i]['InstanceID'] != 
rasd["DiskPool"]:
> > +            logger.error("InstanceID Mismatch, expect %s" % \
> > +                         (rasd['DiskPool'], data[i]['InstanceID']))
> > +            return FAIL
> > +        if data[i].classname == 
"LXC_NetResourceAllocationSettingData" \
> > +                   and data[i]['InstanceID'] == rasd["NetworkPool"]:
> > +            logger.info("NetworkPool InstanceID match")
> > +            return PASS
> > +        elif i == len(data) and data[i]['InstanceID'] != 
> rasd["NetworkPool"]:
> > +            logger.error("InstanceID Mismatch, expect %s" % \
> > +                         (rasd['NetworkPool'], 
data[i]['InstanceID']))
> > +            return FAIL
> > + 
> 
> This is a lot of duplicated code and is difficult to read.  Also, code 
> like this can be difficult to maintain.  This can be collapsed into 
> something much simpler:
> 
>      for item in data:
>              if item['InstanceID'] == rasd[cn]:
>                  logger.info("%s InstanceID match - expected %s, got %s" 

> % (cn,
>                              rasd[cn], item['InstanceID']))
>                  return PASS
> 
>      logger.error("RASD instance with InstanceID %s not found" % 
rasd[cn])
> 
>      return FAIL
> 
> 
> > 
> > +    for k, v in pool.iteritems():
> > +        status, inst = get_instance(options.ip, k, v, options.virt) 
> > +        if status != PASS:
> > +            cleanup_restore(options.ip, options.virt)
> > +            vsxml.undefine(options.ip)
> > +            return status
> 
> Instead of duplicating the cleanup code here, you can use break instead. 

>   That will drop you out of the for loop, and the rest of the code will 
> execute normally.
> 
> > + 
> > +        status = verify_rasd(options.ip, 
"ResourceAllocationFromPool", 
> > +                             k, options.virt, inst.InstanceID,
> > +                             rasd)
> > +        if status != PASS:
> > +            cleanup_restore(options.ip, options.virt)
> > +            vsxml.undefine(options.ip)
> > +            return status
> 
> Same here - use break instead.
> 
> > +
> > +
> > +    cleanup_restore(options.ip, options.virt)
> > +    vsxml.undefine(options.ip)
> >      return status 
> > 
> 
> -- 
> Kaitlin Rupert
> IBM Linux Technology Center
> kaitlin at linux.vnet.ibm.com
> 
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvirt-cim/attachments/20080626/3a88008c/attachment.htm>