Re: [Libvirt-cim] [PATCH 1 of 2] [TEST] Created testcases for the forward HostedAccessPoint association (HostSystem -> KVMRedirectionSAP

Accidentally hit send before finishing my review.. > +def get_kvmrsap_inst(virt, ip, guest_name): > + kvmrsap_inst = None + > + try: > + kvmrsap_cn = get_typed_class(virt, 'KVMRedirectionSAP') > + enum_list = EnumInstances(ip, kvmrsap_cn) > + > + for kvmrsap in enum_list: > + if kvmrsap.SystemName == guest_name: > + if kvmrsap_inst is not None: > + print "More than one KVMRedirectionSAP found \ > + the same guest" Instead of using print, elogger.error() should be used. > + return kvmrsap_inst, FAIL Since you also return the inst and FAIL in the except block, you could remove the line above and change the print to a "raise Exception()" call. Either one is valid, though I think raising an exception is cleaner. > + kvmrsap_inst = kvmrsap > + > + except Exception, details: > + logger.error(details) > + return kvmrsap_inst, FAIL > + > + return kvmrsap_inst, PASS > + > +def verify_kvmrsap(enum_list, kvmrsap_inst): + status = FAIL > + > + for item in enum_list: You should get get only one KVMRedirectionSAP. Verify enum_list only has one element. If it has more than one, return an error. Remove the for loop - since there's only one element in the list, no need to use a loop here.

> + if item.classname != kvmrsap_inst.Classname: > + print "Wrong returned class name (%s)", item.classname This needs to be a logger.error() call instead of a print. > + return status > + > + if compare_all_prop(item, kvmrsap_inst) == PASS: > + if status == PASS: > + print "More than one instance found" Replace the print with logger.error()