CA-199734: best-effort DP destroy and other cleanups for mirror/copy task failures #2678
Conversation
... aggregates two similar branching logics, no semantics change. Signed-off-by: Zheng Li <dev@zheng.li>
ocaml/xapi/xapi_vm_migrate.ml
Outdated
| post_mirror ?mirror_id mirror_record)) | ||
| else | ||
| let mirror_record = get_mirror_record vconf.location (XenAPI.VDI.get_by_uuid remote.rpc remote.session vdi_uuid) in | ||
| post_mirror mirror_record |
There was a problem hiding this comment.
Do you need to call post_mirror here, or just call the continuation with the mirror record?
There was a problem hiding this comment.
Yes, with the current code, the effect is the same.
If we want to put more common logic in post_mirror in the future, making a uniform call might make better sense. For now, indeed, the semantic is no difference.
Let me know if you'd prefer calling the continuation directly here, in which case I'll make a quick update.
There was a problem hiding this comment.
I think it'll be clearer directly calling the continuation at this point, as it's not obvious without carefully looking that the post_mirror call does nothing if mirror=false. If we want to add common logic later, we can reconsider then. Please make the update, thanks!
…task failure ... previously we didn't do this which caused issue like CA-199734. A bit of code refactoring is done as part of this. Big chunk of code is re-orgnized into small parts. Also use with_<resource> style functions to facilitate the separation and composition of error handling etc. Signed-off-by: Zheng Li <dev@zheng.li>
The code change was suggested by Christian Lindig <christian.lindig@citrix.com> for better naming and style. Signed-off-by: Zheng Li <dev@zheng.li>
|
I think this is better than #2664. |
2 participants
Alternative version of #2664.