<br><tt><font size=2>libvirt-cim-bounces@redhat.com wrote on 2008-06-25
22:37:55:<br>
<br>
> yunguol@cn.ibm.com wrote:<br>
> > # HG changeset patch<br>
> > # User Guolian Yun <yunguol@cn.ibm.com><br>
> > # Date 1214371183 -28800<br>
> > # Node ID c8c27374e304a66cd71d1f0b5d0fc462e230a898<br>
> > # Parent  727d97c09d77d73f3542ba49a9dd19ba119a67eb<br>
> > [TEST] Update RAFP.01 for LXC support<br>
> > <br>
> <br>
> Can you keep the revision number in the subject next time around?
<br>
> Having the revision number makes it easier to keep track of which
patch <br>
> is the current version.<br>
> <br>
> Also, please include a log of what has changed for each iteration
of the <br>
> patch.<br>
> </font></tt>
<br><tt><font size=2>  Sure. I will keep the revision number and log
the changes for each patch.</font></tt>
<br><tt><font size=2>  Thanks a lot for your great comments during
the iteration of RAFP.01 patch.</font></tt>
<br><tt><font size=2>  Hopefully, the latest #7 revision works well.</font></tt>
<br>
<br><tt><font size=2>  <br>
> Thanks!<br>
> <br>
> > +def verify_rasd(server, assoc_cn, cn, virt, list, rasd):<br>
> > +    try:<br>
> > +        data = assoc.AssociatorNames(server,<br>
> > +                  
                  assoc_cn,<br>
> > +                  
                  cn,<br>
> > +                  
                  virt,<br>
> > +                  
                  InstanceID=list)<br>
> > +    except Exception:<br>
> > +        logger.error(Globals.CIM_ERROR_ASSOCIATORNAMES
% cn)<br>
> > +        return FAIL<br>
> > +<br>
> > +    if len(data) < 1:<br>
> > +        logger.error("Return NULL,
expect at least one instance")<br>
> > +        return FAIL<br>
> > +    <br>
> > +    for i in range(0, len(data)):<br>
> > +        if data[i].classname == "LXC_MemResourceAllocationSettingData"
\<br>
> > +                  
and data[i]['InstanceID'] == rasd["MemoryPool"]:<br>
> > +            logger.info("MemoryPool
InstanceID match")<br>
> > +            return PASS<br>
> > +        elif i == len(data) and data[i]['InstanceID']
!= <br>
> rasd["MemoryPool"]:<br>
> > +            logger.error("InstanceID
Mismatch, expect %s not %s" % \<br>
> > +                  
      (rasd['MemoryPool'], data[i]['InstanceID']))<br>
> > +            return FAIL<br>
> > +        if data[i].classname == "LXC_ProcResourceAllocationSettingData"
\<br>
> > +                  
and data[i]['InstanceID'] == rasd["ProcessorPool"]:<br>
> > +            logger.info("ProcessorPool
InstanceID match")<br>
> > +            return PASS<br>
> > +        elif i == len(data) and data[i]['InstanceID']
!= <br>
> rasd["ProcessorPool"]:<br>
> > +            logger.error("InstanceID
Mismatch, expect %s not %s" % \<br>
> > +                  
      (rasd['ProcessorPool'], data[i]['InstanceID']))<br>
> > +            return FAIL<br>
> > +        if data[i].classname == "LXC_DiskResourceAllocationSettingData"
\<br>
> > +                  
and data[i]['InstanceID'] == rasd["DiskPool"]:<br>
> > +            logger.info("DiskPool
InstanceID match")<br>
> > +            return PASS<br>
> > +        elif i == len(data) and data[i]['InstanceID']
!= rasd["DiskPool"]:<br>
> > +            logger.error("InstanceID
Mismatch, expect %s" % \<br>
> > +                  
      (rasd['DiskPool'], data[i]['InstanceID']))<br>
> > +            return FAIL<br>
> > +        if data[i].classname == "LXC_NetResourceAllocationSettingData"
\<br>
> > +                  
and data[i]['InstanceID'] == rasd["NetworkPool"]:<br>
> > +            logger.info("NetworkPool
InstanceID match")<br>
> > +            return PASS<br>
> > +        elif i == len(data) and data[i]['InstanceID']
!= <br>
> rasd["NetworkPool"]:<br>
> > +            logger.error("InstanceID
Mismatch, expect %s" % \<br>
> > +                  
      (rasd['NetworkPool'], data[i]['InstanceID']))<br>
> > +            return FAIL<br>
> > +               <br>
> <br>
> This is a lot of duplicated code and is difficult to read.  Also,
code <br>
> like this can be difficult to maintain.  This can be collapsed
into <br>
> something much simpler:<br>
> <br>
>      for item in data:<br>
>              if item['InstanceID']
== rasd[cn]:<br>
>                  logger.info("%s
InstanceID match - expected %s, got %s" <br>
> % (cn,<br>
>                    
         rasd[cn], item['InstanceID']))<br>
>                  return
PASS<br>
> <br>
>      logger.error("RASD instance with InstanceID
%s not found" % rasd[cn])<br>
> <br>
>      return FAIL<br>
> <br>
> <br>
> > <br>
> > +    for k, v in pool.iteritems():<br>
> > +        status, inst = get_instance(options.ip,
k, v, options.virt) <br>
> > +        if status != PASS:<br>
> > +            cleanup_restore(options.ip,
options.virt)<br>
> > +            vsxml.undefine(options.ip)<br>
> > +            return status<br>
> <br>
> Instead of duplicating the cleanup code here, you can use break instead.
<br>
>   That will drop you out of the for loop, and the rest of the
code will <br>
> execute normally.<br>
> <br>
> > + <br>
> > +        status = verify_rasd(options.ip,
"ResourceAllocationFromPool", <br>
> > +                  
          k, options.virt, inst.InstanceID,<br>
> > +                  
          rasd)<br>
> > +        if status != PASS:<br>
> > +            cleanup_restore(options.ip,
options.virt)<br>
> > +            vsxml.undefine(options.ip)<br>
> > +            return status<br>
> <br>
> Same here - use break instead.<br>
> <br>
> > +<br>
> > +<br>
> > +    cleanup_restore(options.ip, options.virt)<br>
> > +    vsxml.undefine(options.ip)<br>
> >      return status <br>
> >          <br>
> <br>
> -- <br>
> Kaitlin Rupert<br>
> IBM Linux Technology Center<br>
> kaitlin@linux.vnet.ibm.com<br>
> <br>
> _______________________________________________<br>
> Libvirt-cim mailing list<br>
> Libvirt-cim@redhat.com<br>
> https://www.redhat.com/mailman/listinfo/libvirt-cim<br>
</font></tt>