on 2014/10/28 20:39, Crístian Viana wrote:

On 28-10-2014 04:48, Zhou Zheng Sheng wrote:

...

- inst.storagevolumes_create(pool, params) + task_id = inst.storagevolumes_create(pool, params)['id'] rollback.prependDefer(inst.storagevolume_delete, pool, vol) + self._wait_task(inst, task_id)

I think you should add the delete call to the rollback session only after the task has finished (i.e. after waiting for it). What if the task raises an exception and the storage volume isn't created? The rollback session will try to delete it but it won't exist.

Hi, thank your Crístian for the comments. I noticed this problem as well. My thought is it's harmless to delete a non-existent storage volume. If the exception is raised from the background thread and failed to create volume, the fore ground thread will not notice it and register the cleanup action. It will raise an exception from "storagevolume_delete()" in this case, so the unittest framework would consider the test fail and report to us. We can then investigate why the storage volume does not get created and fix a possible bug. If the exception is raised from "_wait_task()", it does not mean the storage volume is not successfully created, but just means that there is a problem in "_wait_task()", in this case if we don't cleanup the storage volume, it'll leave it be. A good feature of RollbackContext is that it keeps the earliest exception raised. If we use the operation order in this patch, and suppose the exception is raised from "_wait_task()", then firstly, this is the earliest exception, so it is taken down by RollbackContext and re-raise after the rollback ends. Secondly, as I mentioned "storagevolume_delete()" does a harmless cleanup. -- Zhou Zheng Sheng / 周征晟 E-mail: zhshzhou@linux.vnet.ibm.com Telephone: 86-10-82454397