You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2022/07/04 10:42:44 UTC
[GitHub] [cloudstack] harikrishna-patnala opened a new pull request, #6527: [KVM] Fix for Revert volume snapshot
harikrishna-patnala opened a new pull request, #6527:
URL: https://github.com/apache/cloudstack/pull/6527
### Description
This PR fixes the issue https://github.com/apache/cloudstack/issues/6209 where the snapshot revert operation fails after certain volume operations like Migrate VM with volume / migrate volume / reinstall VM.
The root cause of the issue after these volume operations, the primary storage entry is getting deleted for that volume. We have fixed it here to get the primary datastore entry wrt volume and continue the operation.
<!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
<!--- ********************************************************************************* -->
<!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ********************************************************************************* -->
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [ ] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
1. Create a VM and stop the VM on a KVM environment
2. Create snapshot of the volume
3. migrate VM with volume or volume to a different storage.
4. Create another snapshot of the same volume
5. Performed revert operations on these two snapshots and they are successful
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174564037
@harikrishna-patnala a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174911869
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3710
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174620744
@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174871756
@harikrishna-patnala a Jenkins job has been kicked to build packages. It will be bundled with
SystemVM template(s). I'll keep you posted as I make progress.
--
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
[GitHub] [cloudstack] harikrishna-patnala commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174924618
@blueorangutan test
--
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
[GitHub] [cloudstack] acs-robot commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174853862
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1175106517
<b>Trillian test result (tid-4427)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36996 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6527-t4427-kvm-centos7.zip
Smoke tests completed. 98 look OK, 0 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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
[GitHub] [cloudstack] rohityadavcloud merged pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
rohityadavcloud merged PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174533189
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3706
--
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
[GitHub] [cloudstack] harikrishna-patnala commented on a diff in pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on code in PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#discussion_r914452217
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java:
##########
@@ -178,20 +178,24 @@ protected void revertVolumeToSnapshot(SnapshotObjectTO snapshotOnPrimaryStorage,
* and the snapshot path from the primary storage.
*/
protected Pair<String, SnapshotObjectTO> getSnapshot(SnapshotObjectTO snapshotOnPrimaryStorage, SnapshotObjectTO snapshotOnSecondaryStorage,
- KVMStoragePool kvmStoragePoolPrimary, KVMStoragePool kvmStoragePoolSecondary){
- String snapshotPath = snapshotOnPrimaryStorage.getPath();
-
- if (Files.exists(Paths.get(snapshotPath))) {
- return new Pair<>(snapshotPath, snapshotOnPrimaryStorage);
+ KVMStoragePool kvmStoragePoolPrimary, KVMStoragePool kvmStoragePoolSecondary) {
+ String snapshotPath = null;
Review Comment:
No need to set or use it. If the snapshot on primary is not there then we will get it from secondary storage.
##########
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java:
##########
@@ -379,11 +382,21 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback<CreateCm
@Override
public void revertSnapshot(SnapshotInfo snapshot, SnapshotInfo snapshotOnPrimaryStore, AsyncCompletionCallback<CommandResult> callback) {
- RevertSnapshotCommand cmd = new RevertSnapshotCommand((SnapshotObjectTO)snapshot.getTO(), (SnapshotObjectTO)snapshotOnPrimaryStore.getTO());
+ SnapshotObjectTO dataOnPrimaryStorage = null;
+ if (snapshotOnPrimaryStore != null) {
Review Comment:
It can be null. We are handling the same case where snapshotOnPrimary is null.
Following is the complete flow
1. Look for snapshotOnPrimary entry exists
2. If exists, use that entry to find the appropriate driver to execute revertSnapshotCommand
3. If not found, use volume information to find the appropriate driver to execute revertSnapshotCommand
4. While executing the revertSnapshotCommand, if snapshotOnPrimary exists, use it to revert the volume
5. If snapshotOnPrimary does not exists, use the snapshot from secondary storage to revert the volume
##########
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java:
##########
@@ -428,11 +431,15 @@ public boolean deleteSnapshot(SnapshotInfo snapInfo) {
@Override
public boolean revertSnapshot(SnapshotInfo snapshot) {
+ PrimaryDataStore store = null;
SnapshotInfo snapshotOnPrimaryStore = _snapshotFactory.getSnapshot(snapshot.getId(), DataStoreRole.Primary);
if (snapshotOnPrimaryStore == null) {
- throw new CloudRuntimeException("Cannot find an entry for snapshot " + snapshot.getId() + " on primary storage pools");
+ s_logger.warn("Cannot find an entry for snapshot " + snapshot.getId() + " on primary storage pools, searching with volume's primary storage pool");
+ VolumeInfo volumeInfo = volFactory.getVolume(snapshot.getVolumeId(), DataStoreRole.Primary);
+ store = (PrimaryDataStore)volumeInfo.getDataStore();
Review Comment:
We are not looking for snapshotOnPrimaryStore again. We are just trying to get the appropriate datastore driver to run the command. We are using volume's info here if snapshot on primary os not found. So need to check again.
--
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
[GitHub] [cloudstack] harikrishna-patnala commented on a diff in pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on code in PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#discussion_r914455097
##########
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java:
##########
@@ -379,11 +382,21 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback<CreateCm
@Override
public void revertSnapshot(SnapshotInfo snapshot, SnapshotInfo snapshotOnPrimaryStore, AsyncCompletionCallback<CommandResult> callback) {
- RevertSnapshotCommand cmd = new RevertSnapshotCommand((SnapshotObjectTO)snapshot.getTO(), (SnapshotObjectTO)snapshotOnPrimaryStore.getTO());
+ SnapshotObjectTO dataOnPrimaryStorage = null;
+ if (snapshotOnPrimaryStore != null) {
Review Comment:
It can be null. We are handling the same case where snapshotOnPrimary is null.
Following is the complete flow
1. Look for snapshotOnPrimary entry if exists
2. If exists, use that entry to find the appropriate driver to execute revertSnapshotCommand
3. If not found, use volume information to find the appropriate driver to execute revertSnapshotCommand
4. While executing the revertSnapshotCommand, if snapshotOnPrimary exists, use it to revert the volume
5. If snapshotOnPrimary does not exists, use the snapshot from secondary storage to revert the volume
--
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
[GitHub] [cloudstack] harikrishna-patnala commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1175832110
> LGTM but left some comments. @harikrishna-patnala can you advise how this was tested, is there an existing smoketest or was it manually tested?
For revert volume to snapshot I did not see any smoke tests, I've tested it manually and added the steps that I've tested in the description.
1. Create a VM and stop the VM on a KVM environment
2. Create snapshot of the volume
3. migrate VM with volume or volume to a different storage.
4. Create another snapshot of the same volume
5. Performed revert operations on these two snapshots and they are successful
To write tests to address the revert operation and the issue here, we have to write the exact same flow as above.
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174624147
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6527 (SL-JID-1903)
--
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
[GitHub] [cloudstack] acs-robot commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174620255
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
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
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#discussion_r913547522
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java:
##########
@@ -178,19 +178,21 @@ protected void revertVolumeToSnapshot(SnapshotObjectTO snapshotOnPrimaryStorage,
* and the snapshot path from the primary storage.
*/
protected Pair<String, SnapshotObjectTO> getSnapshot(SnapshotObjectTO snapshotOnPrimaryStorage, SnapshotObjectTO snapshotOnSecondaryStorage,
- KVMStoragePool kvmStoragePoolPrimary, KVMStoragePool kvmStoragePoolSecondary){
- String snapshotPath = snapshotOnPrimaryStorage.getPath();
-
- if (Files.exists(Paths.get(snapshotPath))) {
- return new Pair<>(snapshotPath, snapshotOnPrimaryStorage);
+ KVMStoragePool kvmStoragePoolPrimary, KVMStoragePool kvmStoragePoolSecondary) {
+ String snapshotPath = null;
+ if (snapshotOnPrimaryStorage != null) {
+ snapshotPath = snapshotOnPrimaryStorage.getPath();
+ if (Files.exists(Paths.get(snapshotPath))) {
+ return new Pair<>(snapshotPath, snapshotOnPrimaryStorage);
+ }
}
if (kvmStoragePoolSecondary == null) {
throw new CloudRuntimeException(String.format("Snapshot [%s] does not exists on secondary storage, unable to revert volume [%s] to it.",
snapshotOnSecondaryStorage, snapshotOnSecondaryStorage.getVolume()));
}
- s_logger.trace(String.format("Snapshot [%s] does not exists on primary storage [%s], searching snapshot [%s] on secondary storage [%s].", snapshotOnPrimaryStorage,
+ s_logger.trace(String.format("Snapshot does not exists on primary storage [%s], searching snapshot [%s] on secondary storage [%s].",
kvmStoragePoolPrimary, snapshotOnSecondaryStorage, kvmStoragePoolSecondary));
Review Comment:
```suggestion
if (s_logger.isTraceEnabled()) {
s_logger.trace(String.format("Snapshot does not exists on primary storage [%s], searching snapshot [%s] on secondary storage [%s].",
kvmStoragePoolPrimary, snapshotOnSecondaryStorage, kvmStoragePoolSecondary));
}
```
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174860885
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6527 (SL-JID-1905)
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174925100
@harikrishna-patnala a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
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
[GitHub] [cloudstack] rohityadavcloud commented on a diff in pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on code in PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#discussion_r914391007
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java:
##########
@@ -178,20 +178,24 @@ protected void revertVolumeToSnapshot(SnapshotObjectTO snapshotOnPrimaryStorage,
* and the snapshot path from the primary storage.
*/
protected Pair<String, SnapshotObjectTO> getSnapshot(SnapshotObjectTO snapshotOnPrimaryStorage, SnapshotObjectTO snapshotOnSecondaryStorage,
- KVMStoragePool kvmStoragePoolPrimary, KVMStoragePool kvmStoragePoolSecondary){
- String snapshotPath = snapshotOnPrimaryStorage.getPath();
-
- if (Files.exists(Paths.get(snapshotPath))) {
- return new Pair<>(snapshotPath, snapshotOnPrimaryStorage);
+ KVMStoragePool kvmStoragePoolPrimary, KVMStoragePool kvmStoragePoolSecondary) {
+ String snapshotPath = null;
Review Comment:
If snapshotOnPrimaryStorage is null, then this is never set?
--
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
[GitHub] [cloudstack] rohityadavcloud commented on a diff in pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on code in PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#discussion_r914391302
##########
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java:
##########
@@ -379,11 +382,21 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback<CreateCm
@Override
public void revertSnapshot(SnapshotInfo snapshot, SnapshotInfo snapshotOnPrimaryStore, AsyncCompletionCallback<CommandResult> callback) {
- RevertSnapshotCommand cmd = new RevertSnapshotCommand((SnapshotObjectTO)snapshot.getTO(), (SnapshotObjectTO)snapshotOnPrimaryStore.getTO());
+ SnapshotObjectTO dataOnPrimaryStorage = null;
+ if (snapshotOnPrimaryStore != null) {
Review Comment:
same as above if snapshotOnPrimaryStore is null then dataOnPrimaryStorage is never set? (or is it that this will never be null?)
--
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
[GitHub] [cloudstack] harikrishna-patnala commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174563628
@blueorangutan test
--
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
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174651219
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6527)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6527&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6527&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6527&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_duplicated_lines_density&view=list)
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1173668688
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6527 (SL-JID-1895)
--
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
[GitHub] [cloudstack] harikrishna-patnala commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174513283
@blueorangutan package
--
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
[GitHub] [cloudstack] harikrishna-patnala commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174870754
@blueorangutan package
--
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
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174903803
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6527)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6527&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6527&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6527&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_duplicated_lines_density&view=list)
--
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
[GitHub] [cloudstack] acs-robot commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1173664909
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
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
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1173711879
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6527)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6527&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6527&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6527&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_duplicated_lines_density&view=list)
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174854923
@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1173670378
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6527 (SL-JID-1896)
--
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
[GitHub] [cloudstack] rohityadavcloud commented on a diff in pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on code in PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#discussion_r914390739
##########
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java:
##########
@@ -428,11 +431,15 @@ public boolean deleteSnapshot(SnapshotInfo snapInfo) {
@Override
public boolean revertSnapshot(SnapshotInfo snapshot) {
+ PrimaryDataStore store = null;
SnapshotInfo snapshotOnPrimaryStore = _snapshotFactory.getSnapshot(snapshot.getId(), DataStoreRole.Primary);
if (snapshotOnPrimaryStore == null) {
- throw new CloudRuntimeException("Cannot find an entry for snapshot " + snapshot.getId() + " on primary storage pools");
+ s_logger.warn("Cannot find an entry for snapshot " + snapshot.getId() + " on primary storage pools, searching with volume's primary storage pool");
+ VolumeInfo volumeInfo = volFactory.getVolume(snapshot.getVolumeId(), DataStoreRole.Primary);
+ store = (PrimaryDataStore)volumeInfo.getDataStore();
Review Comment:
@harikrishna-patnala should we check for subsequent cases where it's not found again or this is null?
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1174513597
@harikrishna-patnala a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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
[GitHub] [cloudstack] acs-robot commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1173663397
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1173665471
@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1173663651
@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.
--
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
[GitHub] [cloudstack] harikrishna-patnala commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1173664547
@blueorangutan package
--
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
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1173721554
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6527)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6527&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6527&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6527&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6527&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6527&metric=new_duplicated_lines_density&view=list)
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1175523087
<b>Trillian test result (tid-4429)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36972 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6527-t4429-kvm-centos7.zip
Smoke tests completed. 98 look OK, 0 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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
[GitHub] [cloudstack] harikrishna-patnala commented on pull request #6527: [KVM] Fix for Revert volume snapshot
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on PR #6527:
URL: https://github.com/apache/cloudstack/pull/6527#issuecomment-1176057242
@nvazquez @Pearl1594 will you be able to help me test this PR please.
--
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