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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to