You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by anshul1886 <gi...@git.apache.org> on 2016/12/26 08:48:30 UTC
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
GitHub user anshul1886 opened a pull request:
https://github.com/apache/cloudstack/pull/1867
CLOUDSTACK-9706: Added snapshots cleanup in start and storage GC thre\u2026
\u2026ad if they are failed to cleanup during DeleteSnapshot command
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/anshul1886/cloudstack-1 CLOUDSTACK-9706
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1867.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1867
----
commit a6bf82d529e5df5eec6ed071a2b8e066d78eb45e
Author: Anshul Gangwar <an...@citrix.com>
Date: 2016-05-09T07:15:31Z
CLOUDSTACK-9706: Added snapshots cleanup in start and storage GC thread if they are failed to cleanup during DeleteSnapshot command
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by anshul1886 <gi...@git.apache.org>.
GitHub user anshul1886 reopened a pull request:
https://github.com/apache/cloudstack/pull/1867
CLOUDSTACK-9706: Added snapshots cleanup in start and storage GC thre\u2026
\u2026ad if they are failed to cleanup during DeleteSnapshot command
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/anshul1886/cloudstack-1 CLOUDSTACK-9706
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1867.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1867
----
commit 1c04b30143c84cd4c5fabf9467d2509e78b49f06
Author: Anshul Gangwar <an...@citrix.com>
Date: 2016-05-09T07:15:31Z
CLOUDSTACK-9706: Added snapshots cleanup in start and storage GC thread if they are failed to cleanup during DeleteSnapshot command
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1867#discussion_r106133214
--- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java ---
@@ -191,7 +191,8 @@ public void processEvent(ObjectInDataStoreStateMachine.Event event) {
s_logger.debug("Failed to update state:" + e.toString());
throw new CloudRuntimeException("Failed to update state: " + e.toString());
} finally {
- if (event == ObjectInDataStoreStateMachine.Event.OperationFailed) {
+ DataObjectInStore obj = objectInStoreMgr.findObject(this, this.getDataStore());
+ if (event == ObjectInDataStoreStateMachine.Event.OperationFailed && !obj.getState().equals(ObjectInDataStoreStateMachine.State.Destroying)) {
--- End diff --
Is there a possibility for 'obj' to be null? Can you put some comment as to why there is a need to check 'obj' state not in 'Destroying'?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1867#discussion_r106385035
--- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java ---
@@ -191,7 +191,8 @@ public void processEvent(ObjectInDataStoreStateMachine.Event event) {
s_logger.debug("Failed to update state:" + e.toString());
throw new CloudRuntimeException("Failed to update state: " + e.toString());
} finally {
- if (event == ObjectInDataStoreStateMachine.Event.OperationFailed) {
+ DataObjectInStore obj = objectInStoreMgr.findObject(this, this.getDataStore());
+ if (event == ObjectInDataStoreStateMachine.Event.OperationFailed && !obj.getState().equals(ObjectInDataStoreStateMachine.State.Destroying)) {
--- End diff --
@koushik-das If there is failure in destroying snapshot then we should not delete db entry so that it can be used to detect which snapshots failed to delete successfully.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by anshul1886 <gi...@git.apache.org>.
GitHub user anshul1886 reopened a pull request:
https://github.com/apache/cloudstack/pull/1867
CLOUDSTACK-9706: Added snapshots cleanup in start and storage GC thre\u2026
\u2026ad if they are failed to cleanup during DeleteSnapshot command
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/anshul1886/cloudstack-1 CLOUDSTACK-9706
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1867.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1867
----
commit e9245d3ac1a888505e9d2c1e05841791e29c51ec
Author: Anshul Gangwar <an...@citrix.com>
Date: 2016-05-09T07:15:31Z
CLOUDSTACK-9706: Added snapshots cleanup in start and storage GC thread if they are failed to cleanup during DeleteSnapshot command
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1867: CLOUDSTACK-9706: Added snapshots cleanup in start an...
Posted by cloudmonger <gi...@git.apache.org>.
Github user cloudmonger commented on the issue:
https://github.com/apache/cloudstack/pull/1867
### ACS CI BVT Run
**Sumarry:**
Build Number 471
Hypervisor xenserver
NetworkType Advanced
Passed=148
Failed=17
Skipped=10
_Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
**Failed tests:**
* test_vpc_vpn.py
* test_01_redundant_vpc_site2site_vpn Failing since 2 runs
* test_01_vpc_site2site_vpn Failing since 2 runs
* test_snapshots.py
* test_02_list_snapshots_with_removed_data_store Failed
* test_02_list_snapshots_with_removed_data_store Failing since 2 runs
* test_outofbandmanagement.py
* test_oobm_background_powerstate_sync Failed
* test_oobm_enabledisable_across_clusterzones Failed
* test_oobm_issue_power_cycle Failed
* test_oobm_issue_power_off Failed
* test_oobm_issue_power_on Failed
* test_oobm_issue_power_reset Failed
* test_oobm_issue_power_soft Failed
* test_oobm_issue_power_status Failed
* test_oobm_multiple_mgmt_server_ownership Failed
* test_oobm_zchange_password Failed
* test_volumes.py
* ContextSuite context=TestCreateVolume>:setup Failing since 13 runs
* test_internal_lb.py
* ContextSuite context=TestInternalLb>:setup Failed
* test_routers_network_ops.py
* test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false Failing since 2 runs
**Skipped tests:**
test_01_test_vm_volume_snapshot
test_vm_nic_adapter_vmxnet3
test_01_primary_storage_iscsi
test_static_role_account_acls
test_11_ss_nfs_version_on_ssvm
test_nested_virtualization_vmware
test_3d_gpu_support
test_deploy_vgpu_enabled_vm
test_06_copy_template
test_06_copy_iso
**Passed test suits:**
test_deploy_vm_with_userdata.py
test_affinity_groups_projects.py
test_portable_publicip.py
test_vpc_router_nics.py
test_over_provisioning.py
test_global_settings.py
test_guest_vlan_range.py
test_scale_vm.py
test_service_offerings.py
test_router_dhcphosts.py
test_routers_iptables_default_policy.py
test_loadbalance.py
test_routers.py
test_reset_vm_on_reboot.py
test_deploy_vms_with_varied_deploymentplanners.py
test_network.py
test_router_dns.py
test_non_contigiousvlan.py
test_login.py
test_deploy_vm_iso.py
test_list_ids_parameter.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_regions.py
test_affinity_groups.py
test_network_acl.py
test_pvlan.py
test_nic.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_password_server.py
test_dynamicroles.py
test_vm_life_cycle.py
test_disk_offerings.py
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1867#discussion_r106134695
--- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java ---
@@ -194,18 +194,22 @@ protected boolean deleteSnapshotChain(SnapshotInfo snapshot) {
}
}
if (!deleted) {
- boolean r = snapshotSvr.deleteSnapshot(snapshot);
- if (r) {
- // delete snapshot in cache if there is
- List<SnapshotInfo> cacheSnaps = snapshotDataFactory.listSnapshotOnCache(snapshot.getId());
- for (SnapshotInfo cacheSnap : cacheSnaps) {
- s_logger.debug("Delete snapshot " + snapshot.getId() + " from image cache store: " + cacheSnap.getDataStore().getName());
- cacheSnap.delete();
+ try {
+ boolean r = snapshotSvr.deleteSnapshot(snapshot);
+ if (r) {
+ // delete snapshot in cache if there is
+ List<SnapshotInfo> cacheSnaps = snapshotDataFactory.listSnapshotOnCache(snapshot.getId());
+ for (SnapshotInfo cacheSnap : cacheSnaps) {
+ s_logger.debug("Delete snapshot " + snapshot.getId() + " from image cache store: " + cacheSnap.getDataStore().getName());
+ cacheSnap.delete();
+ }
}
- }
- if (!resultIsSet) {
- result = r;
- resultIsSet = true;
+ if (!resultIsSet) {
+ result = r;
+ resultIsSet = true;
+ }
+ } catch (Exception e){
--- End diff --
Can you put a comment here as well as to why there is catch all so that the intent is clear? Also there are some minor formatting issues, please fix them.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1867: CLOUDSTACK-9706: Added snapshots cleanup in start an...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on the issue:
https://github.com/apache/cloudstack/pull/1867
@koushik-das Made the changes as per review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 closed the pull request at:
https://github.com/apache/cloudstack/pull/1867
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1867#discussion_r103894706
--- Diff: server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java ---
@@ -1181,6 +1181,17 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
@Override
public boolean start() {
+ //destroy snapshots in destroying state
+ List<SnapshotVO> dsnapshots = _snapshotDao.listAllByStatus(Snapshot.State.Destroying);
--- End diff --
Simply use snapshots instead of 'dsnapshots'.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by anshul1886 <gi...@git.apache.org>.
GitHub user anshul1886 reopened a pull request:
https://github.com/apache/cloudstack/pull/1867
CLOUDSTACK-9706: Added snapshots cleanup in start and storage GC thre\u2026
\u2026ad if they are failed to cleanup during DeleteSnapshot command
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/anshul1886/cloudstack-1 CLOUDSTACK-9706
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1867.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1867
----
commit c68931fc64718d8c7f22d401f624ae649effdf94
Author: Anshul Gangwar <an...@citrix.com>
Date: 2016-05-09T07:15:31Z
CLOUDSTACK-9706: Added snapshots cleanup in start and storage GC thread if they are failed to cleanup during DeleteSnapshot command
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1867#discussion_r106387348
--- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java ---
@@ -191,7 +191,8 @@ public void processEvent(ObjectInDataStoreStateMachine.Event event) {
s_logger.debug("Failed to update state:" + e.toString());
throw new CloudRuntimeException("Failed to update state: " + e.toString());
} finally {
- if (event == ObjectInDataStoreStateMachine.Event.OperationFailed) {
+ DataObjectInStore obj = objectInStoreMgr.findObject(this, this.getDataStore());
+ if (event == ObjectInDataStoreStateMachine.Event.OperationFailed && !obj.getState().equals(ObjectInDataStoreStateMachine.State.Destroying)) {
--- End diff --
@koushik-das obj cannot be null as that method is kind of called on that object itself. That method is fetching it from DB for some more details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1867: CLOUDSTACK-9706: Added snapshots cleanup in start an...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on the issue:
https://github.com/apache/cloudstack/pull/1867
@koushik-das Failing tests doesn't seems to be related to this patch. Rebased against latest master to trigger the build.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1867: CLOUDSTACK-9706: Added snapshots cleanup in start an...
Posted by cloudmonger <gi...@git.apache.org>.
Github user cloudmonger commented on the issue:
https://github.com/apache/cloudstack/pull/1867
### ACS CI BVT Run
**Sumarry:**
Build Number 388
Hypervisor xenserver
NetworkType Advanced
Passed=104
Failed=1
Skipped=7
_Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
**Failed tests:**
* test_routers_network_ops.py
* test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true Failing since 2 runs
**Skipped tests:**
test_01_test_vm_volume_snapshot
test_vm_nic_adapter_vmxnet3
test_static_role_account_acls
test_11_ss_nfs_version_on_ssvm
test_nested_virtualization_vmware
test_3d_gpu_support
test_deploy_vgpu_enabled_vm
**Passed test suits:**
test_deploy_vm_with_userdata.py
test_affinity_groups_projects.py
test_portable_publicip.py
test_over_provisioning.py
test_global_settings.py
test_scale_vm.py
test_service_offerings.py
test_routers_iptables_default_policy.py
test_loadbalance.py
test_routers.py
test_reset_vm_on_reboot.py
test_deploy_vms_with_varied_deploymentplanners.py
test_network.py
test_router_dns.py
test_non_contigiousvlan.py
test_login.py
test_deploy_vm_iso.py
test_list_ids_parameter.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_regions.py
test_affinity_groups.py
test_network_acl.py
test_pvlan.py
test_volumes.py
test_nic.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_vm_life_cycle.py
test_disk_offerings.py
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1867#discussion_r106387626
--- Diff: server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java ---
@@ -1181,6 +1181,17 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
@Override
public boolean start() {
+ //destroy snapshots in destroying state
+ List<SnapshotVO> dsnapshots = _snapshotDao.listAllByStatus(Snapshot.State.Destroying);
--- End diff --
@koushik-das Will make the change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 closed the pull request at:
https://github.com/apache/cloudstack/pull/1867
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1867#discussion_r106388631
--- Diff: server/src/com/cloud/storage/StorageManagerImpl.java ---
@@ -1078,6 +1082,16 @@ public void cleanupStorage(boolean recurring) {
}
}
+ //destroy snapshots in destroying state in snapshot_store_ref
+ List<SnapshotDataStoreVO> ssSnapshots = _snapshotStoreDao.listByState(ObjectInDataStoreStateMachine.State.Destroying);
+ for(SnapshotDataStoreVO ssSnapshotVO : ssSnapshots){
+ try {
+ _snapshotService.deleteSnapshot(snapshotFactory.getSnapshot(ssSnapshotVO.getSnapshotId(), DataStoreRole.Image));
--- End diff --
@koushik-das If there is any failure in deleteSnapshot() then cleanup will be retried. DB entry for such snapshots will only be removed on successful deletion. Destroying check introduced in SnapshotObject class is to make sure that DB entry doesn't gets deleted.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1867#discussion_r106384378
--- Diff: server/src/com/cloud/storage/StorageManagerImpl.java ---
@@ -1078,6 +1082,16 @@ public void cleanupStorage(boolean recurring) {
}
}
+ //destroy snapshots in destroying state in snapshot_store_ref
+ List<SnapshotDataStoreVO> ssSnapshots = _snapshotStoreDao.listByState(ObjectInDataStoreStateMachine.State.Destroying);
+ for(SnapshotDataStoreVO ssSnapshotVO : ssSnapshots){
+ try {
+ _snapshotService.deleteSnapshot(snapshotFactory.getSnapshot(ssSnapshotVO.getSnapshotId(), DataStoreRole.Image));
--- End diff --
@koushik-das If there is any failure then the db entry will remain Destroying and will only be updated if there is successful deletion. Condition which is checking Destroying state of object in SnapshotObject class is meant for this purpose.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1867: CLOUDSTACK-9706: Added snapshots cleanup in start an...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the issue:
https://github.com/apache/cloudstack/pull/1867
Code changes LGTM
@anshul1886 Please check on travis failure
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1867: CLOUDSTACK-9706: Added snapshots cleanup in start an...
Posted by cloudmonger <gi...@git.apache.org>.
Github user cloudmonger commented on the issue:
https://github.com/apache/cloudstack/pull/1867
### ACS CI BVT Run
**Sumarry:**
Build Number 322
Hypervisor xenserver
NetworkType Advanced
Passed=103
Failed=1
Skipped=7
_Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
**Failed tests:**
* test_routers_network_ops.py
* test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false Failed
**Skipped tests:**
test_01_test_vm_volume_snapshot
test_vm_nic_adapter_vmxnet3
test_static_role_account_acls
test_11_ss_nfs_version_on_ssvm
test_nested_virtualization_vmware
test_3d_gpu_support
test_deploy_vgpu_enabled_vm
**Passed test suits:**
test_deploy_vm_with_userdata.py
test_affinity_groups_projects.py
test_portable_publicip.py
test_over_provisioning.py
test_global_settings.py
test_scale_vm.py
test_service_offerings.py
test_routers_iptables_default_policy.py
test_loadbalance.py
test_routers.py
test_reset_vm_on_reboot.py
test_deploy_vms_with_varied_deploymentplanners.py
test_network.py
test_router_dns.py
test_non_contigiousvlan.py
test_login.py
test_deploy_vm_iso.py
test_list_ids_parameter.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_regions.py
test_affinity_groups.py
test_network_acl.py
test_pvlan.py
test_volumes.py
test_nic.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_vm_life_cycle.py
test_disk_offerings.py
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1867#discussion_r106137098
--- Diff: server/src/com/cloud/storage/StorageManagerImpl.java ---
@@ -1078,6 +1082,16 @@ public void cleanupStorage(boolean recurring) {
}
}
+ //destroy snapshots in destroying state in snapshot_store_ref
+ List<SnapshotDataStoreVO> ssSnapshots = _snapshotStoreDao.listByState(ObjectInDataStoreStateMachine.State.Destroying);
+ for(SnapshotDataStoreVO ssSnapshotVO : ssSnapshots){
+ try {
+ _snapshotService.deleteSnapshot(snapshotFactory.getSnapshot(ssSnapshotVO.getSnapshotId(), DataStoreRole.Image));
--- End diff --
deleteSnapshot() is going to send an agent command to cleanup the snapshot from secondary, if there is any failure will cleanup be retried or the DB entry is simply updated?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1867#discussion_r106386503
--- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java ---
@@ -194,18 +194,22 @@ protected boolean deleteSnapshotChain(SnapshotInfo snapshot) {
}
}
if (!deleted) {
- boolean r = snapshotSvr.deleteSnapshot(snapshot);
- if (r) {
- // delete snapshot in cache if there is
- List<SnapshotInfo> cacheSnaps = snapshotDataFactory.listSnapshotOnCache(snapshot.getId());
- for (SnapshotInfo cacheSnap : cacheSnaps) {
- s_logger.debug("Delete snapshot " + snapshot.getId() + " from image cache store: " + cacheSnap.getDataStore().getName());
- cacheSnap.delete();
+ try {
+ boolean r = snapshotSvr.deleteSnapshot(snapshot);
+ if (r) {
+ // delete snapshot in cache if there is
+ List<SnapshotInfo> cacheSnaps = snapshotDataFactory.listSnapshotOnCache(snapshot.getId());
+ for (SnapshotInfo cacheSnap : cacheSnaps) {
+ s_logger.debug("Delete snapshot " + snapshot.getId() + " from image cache store: " + cacheSnap.getDataStore().getName());
+ cacheSnap.delete();
+ }
}
- }
- if (!resultIsSet) {
- result = r;
- resultIsSet = true;
+ if (!resultIsSet) {
+ result = r;
+ resultIsSet = true;
+ }
+ } catch (Exception e){
--- End diff --
@koushik-das Its a catch all as we will retry to delete snapshot again in next attempt if it fails to delete so just logging in and carry on. Also it is introduced to make sure that all snapshots gets chance to delete.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1867: CLOUDSTACK-9706: Added snapshots cleanup in s...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 closed the pull request at:
https://github.com/apache/cloudstack/pull/1867
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---