slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r696611744
##########
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, AllocatedVM, Creating, CreatingForVM, CreatedOnPrimary,
CreatedOnPrimaryForVM, BackingUp, BackingUpForVM, BackedUp, BackedUpForVM,
Copying, Destroying, Destroyed,
Review comment:
Hi @sureshanaparti, I was thinking a lot about your suggestion (and I
did it also before 2 years when I've started implementing this PR).
Unfortunately, this isn't possible. With this feature, we are creating
something like a group snapshot for primary storages that do not have such an
implementation. That's why we have to invoke for each volume the respective
storage plugin method `takeSnapshot` which requires a DB record in `snapshots`
and `snapshot_store_ref` tables.
I think that there are two options with those states, which are:
- to hide them when listing snapshots.
- or all storage plugins have to be modified for this functionality without
the need for a DB record
we discussed offline that the NFS snapshots are backup to secondary storage,
which was required for me to check that the job of `drive-backup` command is
finished and to thaw the VM as soon as possible. I have one suggestion here:
- to remove the support in this functionality for NFS volumes. In this case,
there is no need to backup the snapshots on secondary (for the plugins which
support backup)
- and to use this implementation when it gets in - [PR
5297](https://github.com/apache/cloudstack/pull/5297).
--
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]
