Re: [Kimchi-devel] [PATCH 3/5] AsyncTask: Improve continuous status feedback
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(a)linux.vnet.ibm.com
Telephone: 86-10-82454397