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 2020/10/12 04:15:34 UTC
[GitHub] [cloudstack] Pearl1594 opened a new pull request #4395: support for handling incremental snaps (on DB entries) on xen
Pearl1594 opened a new pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395
## Description
PR attempts to fix and enhance the following:
- On Xen, there are instances when the child snapshot is basically only a database entry with the file path pointing to the parent snapshot - handle this scenario such that a new entry is created for such incremental snapshots and delete the snapshot from source datastore during data migration
- Wrt scaling down of SSVM, revised the logic to prevent quick scaling down of the SSVMs
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Bug fix (non-breaking change which fixes an issue)
- [X] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
## Screenshots (if appropriate):
## How Has This Been Tested?
- Conducted tests on xen of files
Tests on xen for snapshots with child snaps having install path same as that as parent snapshot
```
Before Migration:
-----------------------
MariaDB [cloud]> select * from snapshot_store_ref where store_role = 'Image' and volume_id =11 and state <> 'Destroyed' order by parent_snapshot_id\G
*************************** 1. row ***************************
id: 497
store_id: 1
snapshot_id: 63
created: 2020-10-10 07:00:08
last_updated: NULL
job_id: NULL
store_role: Image
size: 52428800
physical_size: 52531712
parent_snapshot_id: 0
install_path: snapshots/2/11/d05a2c2f-6885-4932-b5a0-d11f82129a48.vhd
state: Ready
update_count: 2
ref_cnt: 0
updated: 2020-10-10 13:25:24
volume_id: 11
*************************** 2. row ***************************
id: 375
store_id: 1
snapshot_id: 67
created: 2020-10-10 07:00:09
last_updated: NULL
job_id: NULL
store_role: Image
size: 52428800
physical_size: 0
parent_snapshot_id: 63
install_path: snapshots/2/11/d05a2c2f-6885-4932-b5a0-d11f82129a48.vhd
state: Ready
update_count: 2
ref_cnt: 0
updated: 2020-10-08 06:54:57
volume_id: 11
*************************** 3. row ***************************
id: 381
store_id: 1
snapshot_id: 70
created: 2020-10-10 07:00:11
last_updated: NULL
job_id: NULL
store_role: Image
size: 52428800
physical_size: 0
parent_snapshot_id: 67
install_path: snapshots/2/11/d05a2c2f-6885-4932-b5a0-d11f82129a48.vhd
state: Ready
update_count: 2
ref_cnt: 0
updated: 2020-10-08 06:55:14
volume_id: 11
*************************** 4. row ***************************
id: 472
store_id: 1
snapshot_id: 74
created: 2020-10-10 07:32:57
last_updated: NULL
job_id: NULL
store_role: Image
size: 52428800
physical_size: 0
parent_snapshot_id: 70
install_path: snapshots/2/11/d05a2c2f-6885-4932-b5a0-d11f82129a48.vhd
state: Ready
update_count: 2
ref_cnt: 0
updated: 2020-10-10 07:32:57
volume_id: 11
After migration:
---------------
MariaDB [cloud]> select * from snapshot_store_ref where store_role = 'Image' and volume_id =11 and state <> 'Destroyed' order by parent_snapshot_id\G
*************************** 1. row ***************************
id: 510
store_id: 5
snapshot_id: 63
created: 2020-10-10 07:00:08
last_updated: NULL
job_id: NULL
store_role: Image
size: 52428800
physical_size: 52531712
parent_snapshot_id: 0
install_path: snapshots/2/11/d05a2c2f-6885-4932-b5a0-d11f82129a48.vhd
state: Ready
update_count: 2
ref_cnt: 0
updated: 2020-10-12 04:11:10
volume_id: 11
*************************** 2. row ***************************
id: 375
store_id: 5
snapshot_id: 67
created: 2020-10-10 07:00:09
last_updated: NULL
job_id: NULL
store_role: Image
size: 52428800
physical_size: 0
parent_snapshot_id: 63
install_path: snapshots/2/11/d05a2c2f-6885-4932-b5a0-d11f82129a48.vhd
state: Ready
update_count: 2
ref_cnt: 0
updated: 2020-10-08 06:54:57
volume_id: 11
*************************** 3. row ***************************
id: 381
store_id: 5
snapshot_id: 70
created: 2020-10-10 07:00:11
last_updated: NULL
job_id: NULL
store_role: Image
size: 52428800
physical_size: 0
parent_snapshot_id: 67
install_path: snapshots/2/11/d05a2c2f-6885-4932-b5a0-d11f82129a48.vhd
state: Ready
update_count: 2
ref_cnt: 0
updated: 2020-10-08 06:55:14
volume_id: 11
*************************** 4. row ***************************
id: 472
store_id: 5
snapshot_id: 74
created: 2020-10-10 07:32:57
last_updated: NULL
job_id: NULL
store_role: Image
size: 52428800
physical_size: 0
parent_snapshot_id: 70
install_path: snapshots/2/11/d05a2c2f-6885-4932-b5a0-d11f82129a48.vhd
state: Ready
update_count: 2
ref_cnt: 0
updated: 2020-10-10 07:32:57
volume_id: 11
```
Tested on xen, vmware and KVM
Tests for SSVM scale down:
Updated following global setting values:
secstorage.capacity.standby: 4
secstorage.session.max: 10
1. Added 7 entries to the cmd_exec_log table, to simulate scaling up operation - Successfully scaled - the new SSVM spawned continues to exist until the number of cmds drops below threshold
2. Deleted entries from cmd_exec_log table, such that only 4 entries were present in the table - successfully triggered scale down - as the total number of cmds in the table has dropped below half the defined max limit (10/2 = 5)
3. Simulated another scale up operation by adding entries to the cmd_exec log table, added entries to the second (new) SSVM however the total number of commands in the table is below threshold - does NOT scale down SSVM as a command is running on it - as expected
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-708310870
Tests LGTM
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#discussion_r506044504
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -1300,15 +1300,17 @@ protected Answer copyFromNfsToNfs(CopyCommand cmd) {
try {
File srcFile = new File(getDir(srcStore.getUrl(), _nfsVersion), srcData.getPath());
File destFile = new File(getDir(destStore.getUrl(), _nfsVersion), destData.getPath());
- ImageFormat format = getTemplateFormat(srcFile.getName());
if (srcFile == null) {
- return new CopyCmdAnswer("Can't find src file:" + srcFile);
+ return new CopyCmdAnswer("Can't find source file to initiate file transfer");
Review comment:
adding path and datastore id to the log might help in debugging
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#discussion_r505367305
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java
##########
@@ -391,12 +391,16 @@ public void copyAsync(DataObject srcdata, DataObject destData, AsyncCompletionCa
private Answer sendToLeastBusyEndpoint(List<EndPoint> eps, CopyCommand cmd) {
Answer answer = null;
EndPoint endPoint = null;
- Long epId = ssvmWithLeastMigrateJobs();
+ List<Long> epIds = ssvmWithLeastMigrateJobs();
+ Long epId = null;
+ if (!epIds.isEmpty()) {
+ epId = epIds.get(0);
+ }
if (epId == null) {
Review comment:
@Pearl1594 seems `epId` is no longer required for the check here. Can check with empty `epIds` ? for end point, can use the first item from this 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#discussion_r505509639
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -1300,16 +1300,16 @@ protected Answer copyFromNfsToNfs(CopyCommand cmd) {
try {
File srcFile = new File(getDir(srcStore.getUrl(), _nfsVersion), srcData.getPath());
File destFile = new File(getDir(destStore.getUrl(), _nfsVersion), destData.getPath());
- ImageFormat format = getTemplateFormat(srcFile.getName());
if (srcFile == null) {
- return new CopyCmdAnswer("Can't find src file:" + srcFile);
+ return new CopyCmdAnswer("Can't find src file");
}
+ ImageFormat format = getTemplateFormat(srcFile.getName());
if (srcData instanceof TemplateObjectTO || srcData instanceof VolumeObjectTO) {
File srcDir = null;
if (srcFile.isFile() || srcFile.getName().contains(".")) {
srcDir = new File(srcFile.getParent());
- } else if (!srcFile.isFile() && !srcFile.isDirectory()) {
+ } else if (!srcFile.isDirectory()) {
Review comment:
is this supposed to be **!**isDir()? or isDir() plain?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-706858100
@Pearl1594 a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware67, centos7 mgmt + kvmcentos7) 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-707200659
<b>Trillian test result (tid-2922)</b>
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37494 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4395-t2922-xenserver-71.zip
Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 84 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_scale_vm | `Failure` | 10.42 | test_scale_vm.py
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland removed a comment on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
DaanHoogland removed a comment on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-708221012
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#discussion_r506056156
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -1300,15 +1300,17 @@ protected Answer copyFromNfsToNfs(CopyCommand cmd) {
try {
File srcFile = new File(getDir(srcStore.getUrl(), _nfsVersion), srcData.getPath());
File destFile = new File(getDir(destStore.getUrl(), _nfsVersion), destData.getPath());
- ImageFormat format = getTemplateFormat(srcFile.getName());
if (srcFile == null) {
- return new CopyCmdAnswer("Can't find src file:" + srcFile);
+ return new CopyCmdAnswer("Can't find source file at path: "+ srcData.getPath() +"on datastore: "+ srcDataStore.getUuid() +" to initiate file transfer");
Review comment:
make sure spaces are properly aligned on the log code, rest changes LGTM
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-708221673
@blueorangutan centos7 xenserver-71 keepEnv
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-709711821
@Pearl1594 a Jenkins job has been kicked to build packages. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-707178666
<b>Trillian test result (tid-2924)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35291 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4395-t2924-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_outofbandmanagement.py
Smoke tests completed. 85 look OK, 0 have error(s)
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland removed a comment on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
DaanHoogland removed a comment on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-708383746
@blueorangutan centos7 xenserver-71 keepEnv
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-709711551
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#discussion_r505370866
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -1309,6 +1309,8 @@ protected Answer copyFromNfsToNfs(CopyCommand cmd) {
File srcDir = null;
if (srcFile.isFile() || srcFile.getName().contains(".")) {
srcDir = new File(srcFile.getParent());
+ } else if (!srcFile.isFile() && !srcFile.isDirectory()) {
Review comment:
`!srcFile.isFile()` is always true here
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-708910089
Code changes LGTM
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-707034824
cc @vladimirpetrov @PaulAngus
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd merged pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-709784077
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-709784471
@Pearl1594 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-709729045
@Pearl1594 a Jenkins job has been kicked to build packages. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-706857921
@blueorangutan test matrix
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-708270058
@blueorangutan centos7 xenserver-71 keepEnv
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4395: support for handling incremental snaps (on DB entries) on xen
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-706848115
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-710130277
<b>Trillian test result (tid-2977)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36707 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4395-t2977-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 83 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 312.21 | test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 293.38 | test_vpc_redundant.py
test_hostha_kvm_host_fencing | `Error` | 166.69 | test_hostha_kvm.py
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#discussion_r505352050
##########
File path: engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/SecondaryStorageServiceImpl.java
##########
@@ -198,6 +204,39 @@ protected Void migrateDataCallBack(AsyncCallbackDispatcher<SecondaryStorageServi
return null;
}
+ private void updateDataObject(DataObject srcData, DataObject destData) {
+ if (destData instanceof SnapshotInfo) {
+ SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findBySourceSnapshot(srcData.getId(), DataStoreRole.Image);
+ SnapshotDataStoreVO destSnapshotStore = snapshotStoreDao.findBySnapshot(srcData.getId(), DataStoreRole.Image);
+ if (snapshotStore != null && destSnapshotStore != null) {
+ destSnapshotStore.setPhysicalSize(snapshotStore.getPhysicalSize());
+ destSnapshotStore.setCreated(snapshotStore.getCreated());
+ if (snapshotStore.getParentSnapshotId() != destSnapshotStore.getParentSnapshotId()) {
+ destSnapshotStore.setParentSnapshotId(snapshotStore.getParentSnapshotId());
+ }
+ snapshotStoreDao.update(destSnapshotStore.getId(), destSnapshotStore);
+ }
+ }
+
+ if (destData instanceof VolumeInfo) {
+ VolumeDataStoreVO srcVolume = volumeDataStoreDao.findByStoreVolume(srcData.getDataStore().getId(), srcData.getId());
+ VolumeDataStoreVO destVolume = volumeDataStoreDao.findByStoreVolume(destData.getDataStore().getId(), destData.getId());
+ if (srcVolume != null && destVolume != null) {
+ destVolume.setPhysicalSize(srcVolume.getPhysicalSize());
+ destVolume.setCreated(srcVolume.getCreated());
+ volumeDataStoreDao.update(destVolume.getId(), destVolume);
+ }
+ }
+
+ if (destData instanceof TemplateInfo) {
Review comment:
@Pearl1594 better use if-else / switch-case. The control will come here even when `destData` is of type SnapshotInfo. If this method is being called for any other destData instance, better add a debug log for unsupported instance.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#discussion_r505495972
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java
##########
@@ -391,12 +391,16 @@ public void copyAsync(DataObject srcdata, DataObject destData, AsyncCompletionCa
private Answer sendToLeastBusyEndpoint(List<EndPoint> eps, CopyCommand cmd) {
Answer answer = null;
EndPoint endPoint = null;
- Long epId = ssvmWithLeastMigrateJobs();
+ List<Long> epIds = ssvmWithLeastMigrateJobs();
+ Long epId = null;
+ if (!epIds.isEmpty()) {
+ epId = epIds.get(0);
+ }
if (epId == null) {
Review comment:
@DaanHoogland the last method _ssvmWithLeastMigrateJobs()_ returns null if it fails to pick ssvm and the null check seems to be valid. The same updated method now returns an empty list if fails. So list with empty check is fine, 'epId' may not be required.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-709725743
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2191
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#discussion_r505488497
##########
File path: engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/SecondaryStorageServiceImpl.java
##########
@@ -198,6 +204,39 @@ protected Void migrateDataCallBack(AsyncCallbackDispatcher<SecondaryStorageServi
return null;
}
+ private void updateDataObject(DataObject srcData, DataObject destData) {
+ if (destData instanceof SnapshotInfo) {
+ SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findBySourceSnapshot(srcData.getId(), DataStoreRole.Image);
+ SnapshotDataStoreVO destSnapshotStore = snapshotStoreDao.findBySnapshot(srcData.getId(), DataStoreRole.Image);
+ if (snapshotStore != null && destSnapshotStore != null) {
+ destSnapshotStore.setPhysicalSize(snapshotStore.getPhysicalSize());
+ destSnapshotStore.setCreated(snapshotStore.getCreated());
+ if (snapshotStore.getParentSnapshotId() != destSnapshotStore.getParentSnapshotId()) {
+ destSnapshotStore.setParentSnapshotId(snapshotStore.getParentSnapshotId());
+ }
+ snapshotStoreDao.update(destSnapshotStore.getId(), destSnapshotStore);
+ }
+ }
+
+ if (destData instanceof VolumeInfo) {
+ VolumeDataStoreVO srcVolume = volumeDataStoreDao.findByStoreVolume(srcData.getDataStore().getId(), srcData.getId());
+ VolumeDataStoreVO destVolume = volumeDataStoreDao.findByStoreVolume(destData.getDataStore().getId(), destData.getId());
+ if (srcVolume != null && destVolume != null) {
+ destVolume.setPhysicalSize(srcVolume.getPhysicalSize());
+ destVolume.setCreated(srcVolume.getCreated());
+ volumeDataStoreDao.update(destVolume.getId(), destVolume);
+ }
+ }
+
+ if (destData instanceof TemplateInfo) {
Review comment:
agree, it might also be usefull to extract the processing bits in a (template) method to reduce complexity. (no blocker)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on a change in pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on a change in pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#discussion_r506046243
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -1351,7 +1353,7 @@ protected Answer copyFromNfsToNfs(CopyCommand cmd) {
if (srcFile.isFile()) {
newVol.setPath(destData.getPath() + File.separator + srcFile.getName());
} else {
- newVol.setPath(destData.getPath());
+ newVol.setPath(srcData.getPath());
Review comment:
Have verified this
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -1300,15 +1300,17 @@ protected Answer copyFromNfsToNfs(CopyCommand cmd) {
try {
File srcFile = new File(getDir(srcStore.getUrl(), _nfsVersion), srcData.getPath());
File destFile = new File(getDir(destStore.getUrl(), _nfsVersion), destData.getPath());
- ImageFormat format = getTemplateFormat(srcFile.getName());
if (srcFile == null) {
- return new CopyCmdAnswer("Can't find src file:" + srcFile);
+ return new CopyCmdAnswer("Can't find source file to initiate file transfer");
Review comment:
done
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-707211245
<b>Trillian test result (tid-2923)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38523 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4395-t2923-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 83 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 304.20 | test_vpc_redundant.py
test_hostha_kvm_host_fencing | `Error` | 99.43 | test_hostha_kvm.py
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-709781214
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2192
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-709727322
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland removed a comment on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
DaanHoogland removed a comment on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-708270058
@blueorangutan centos7 xenserver-71 keepEnv
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-706848267
@Pearl1594 a Jenkins job has been kicked to build packages. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-708221012
@blueorangutan centos7 kvm-centos7 keepEnv
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#discussion_r505508960
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -1300,16 +1300,16 @@ protected Answer copyFromNfsToNfs(CopyCommand cmd) {
try {
File srcFile = new File(getDir(srcStore.getUrl(), _nfsVersion), srcData.getPath());
File destFile = new File(getDir(destStore.getUrl(), _nfsVersion), destData.getPath());
- ImageFormat format = getTemplateFormat(srcFile.getName());
if (srcFile == null) {
- return new CopyCmdAnswer("Can't find src file:" + srcFile);
+ return new CopyCmdAnswer("Can't find src file");
Review comment:
maybe rephrase to "no src file given/to work with" or something?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#discussion_r505489877
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java
##########
@@ -391,12 +391,16 @@ public void copyAsync(DataObject srcdata, DataObject destData, AsyncCompletionCa
private Answer sendToLeastBusyEndpoint(List<EndPoint> eps, CopyCommand cmd) {
Answer answer = null;
EndPoint endPoint = null;
- Long epId = ssvmWithLeastMigrateJobs();
+ List<Long> epIds = ssvmWithLeastMigrateJobs();
+ Long epId = null;
+ if (!epIds.isEmpty()) {
+ epId = epIds.get(0);
+ }
if (epId == null) {
Review comment:
@sureshanaparti is this a remark to style? if epIds is empty epId is null so the check is needed in some way.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#discussion_r505367305
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java
##########
@@ -391,12 +391,16 @@ public void copyAsync(DataObject srcdata, DataObject destData, AsyncCompletionCa
private Answer sendToLeastBusyEndpoint(List<EndPoint> eps, CopyCommand cmd) {
Answer answer = null;
EndPoint endPoint = null;
- Long epId = ssvmWithLeastMigrateJobs();
+ List<Long> epIds = ssvmWithLeastMigrateJobs();
+ Long epId = null;
+ if (!epIds.isEmpty()) {
+ epId = epIds.get(0);
+ }
if (epId == null) {
Review comment:
@Pearl1594 seems `epId` is no longer required for the check here. Can check with empty `epIds` ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#discussion_r506045091
##########
File path: services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
##########
@@ -1351,7 +1353,7 @@ protected Answer copyFromNfsToNfs(CopyCommand cmd) {
if (srcFile.isFile()) {
newVol.setPath(destData.getPath() + File.separator + srcFile.getName());
} else {
- newVol.setPath(destData.getPath());
+ newVol.setPath(srcData.getPath());
Review comment:
check if this causes any regression
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-708383746
@blueorangutan centos7 xenserver-71 keepEnv
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4395: support for data migration of incremental snaps on xen
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4395:
URL: https://github.com/apache/cloudstack/pull/4395#issuecomment-706857478
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2156
----------------------------------------------------------------
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:
users@infra.apache.org