Re: [Libvirt-cim] [PATCH 1 of 5] [TEST] Add destroy_diskpool()

>> +def create_diskpool(server, virt='KVM', dpool=default_pool_name, >> + useExisting=False): >> status = PASS >> dpoolname = None >> try: >> - dpool_list = diskpool_list(server, virt='KVM') >> - if len(dpool_list) > 0: >> - dpoolname=dpool_list[0] >> - else: >> - diskxml = PoolXML(server, virt=virt) >> + if useExisting == True: >> + dpool_list = diskpool_list(server, virt='KVM') >> + if len(dpool_list) > 0: >> + dpoolname=dpool_list[0] >> + >> + if dpoolname == None: >> + cmd = "virsh -c %s pool-list --all | grep %s" % \ >> + (utils.virt2uri(virt), dpool) >> + ret, out = utils.run_remote(server, cmd) >> + if out != "": >> + logger.error("Disk pool with name '%s' already >> exists", dpool) >> + return FAIL, "Unknown" >> > If the diskpool cinmtest-diskpool already exist on the machine then > the tc execution wont proceed unless we delete manually and then > re-run the tc. > I think we should not pass FAIL as status value, instead supply PASS > as the status value. > The same comment applies for network pool also. I'm not sure I understand why we should return PASS if the diskpool already exists? The purpose of this function is to create a diskpool with a specific XML. If a diskpool with the same name already exists on the system, we cannot guarantee that it was created with the same XML. That is, the pool might have the same name, but it might be entirely different from the pool we want to create.

If the caller wants to use an existing pool, the caller can use the useExisting param to do so. Is there a scenario you were thinking of where returning PASS if the pool already exists would be useful?