slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r517983999



##########
File path: api/src/main/java/com/cloud/storage/Snapshot.java
##########
@@ -48,7 +48,7 @@ public boolean equals(String snapshotType) {
     }
 
     public enum State {
-        Allocated, Creating, CreatedOnPrimary, BackingUp, BackedUp, Copying, 
Destroying, Destroyed,
+        Allocated, AllocatedKVM, Creating, CreatingForVM, CreatedOnPrimary, 
CreatedOnPrimaryForVM, BackingUp, BackingUpForVM, BackedUp, BackedUpForVM, 
Copying, Destroying, Destroyed,

Review comment:
       Hello @sureshanaparti  , thanks for the review! The design of the 
current implementation was to change as less as possible in CloudStack, that's 
why we are reusing a lot of the functionality. The additional snapshot's states 
are added to distinguish with the existing functionality of VM snapshots. In 
this PR we are lying to CloudStack that a VM snapshot is created, actually we 
create a separate snapshot for each disk of the VM (and they are consistent, 
because of the freeze of VM). And the additional states are needed to forbid 
the user at the end to delete one of the volume's snapshots (by mistake or on 
purpose). 
   I'd be glad if you can give me an advice with another option




----------------------------------------------------------------
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.

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


Reply via email to