[GitHub] [cloudstack] DaanHoogland commented on pull request #6868: Set root volume as destroyed when destroying a VM

[GitBox <gi...@apache.org>]

DaanHoogland commented on PR #6868:
URL: https://github.com/apache/cloudstack/pull/6868#issuecomment-1304235170

   > > > I think you are missing a (not necessarily new) use-case where the root disk is expunged and the VM restored. both with and without this setting this will go wrong. You just made it a bit more explicit with your code.
   > > 
   > > 
   > > Right. Because vm can be recovered, the root disk should not be removed. If root disk is set to Destroyed, it will be cleaned after a period, then the vm cannot be recovered.
   > > If you want to exclude destroyed vm in resource count, please look at the methods for resource calculation. @JoaoJandre
   > 
   > The VM is already being excluded in the resource count, but its root volume is not. This is what this PR proposes to change.
   
   That is not the point @JoaoJandre, and you are right that you are not introducing the fact that the restore will fail. What your PR does is trying to restore the VM with the volume when it has any other state than Destroyed, which includes Expunging and Expunged. These are two cases that should be excluded from that logic, I think. Would you agree?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org