Re: [PR] CLVM enhancements and fixes [cloudstack]
harikrishna-patnala merged PR #12617: URL: https://github.com/apache/cloudstack/pull/12617 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4702862999 [SF] Trillian test result (tid-16306) Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8 Total time taken: 50471 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12617-t16306-kvm-ol8.zip Smoke tests completed. 151 look OK, 0 have errors, 0 did not run Only failed and skipped 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4700747816 @Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4700744106 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4700734098 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18254 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
sonarqubecloud[bot] commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4700711071 ## [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) **Quality Gate failed** Failed conditions  [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=12617&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [32.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=12617&metric=new_coverage&view=list) (required ≥ 40%) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4700661864 @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4700659012 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4700320581 [SF] Trillian Build Failed (tid-16305) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
sonarqubecloud[bot] commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4700320106 ## [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) **Quality Gate failed** Failed conditions  [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=12617&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [32.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=12617&metric=new_coverage&view=list) (required ≥ 40%) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4700316627 @Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4700314301 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4700291207 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18251 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4700202568 @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4700198302 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4697952186 [SF] Trillian test result (tid-16300) Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8 Total time taken: 50741 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12617-t16300-kvm-ol8.zip Smoke tests completed. 151 look OK, 0 have errors, 0 did not run Only failed and skipped 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4693525587 @Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4693507929 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4693307426 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18241 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
sonarqubecloud[bot] commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4693277145 ## [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) **Quality Gate failed** Failed conditions  [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=12617&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [32.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=12617&metric=new_coverage&view=list) (required ≥ 40%) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4692914817 @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4692900751 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4691619006 [SF] Trillian test result (tid-16292) Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8 Total time taken: 50205 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12617-t16292-kvm-ol8.zip Smoke tests completed. 151 look OK, 0 have errors, 0 did not run Only failed and skipped 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4685661032 @Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4685654727 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4685649484 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18231 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
sonarqubecloud[bot] commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4685593924 ## [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) **Quality Gate failed** Failed conditions  [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=12617&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [33.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=12617&metric=new_coverage&view=list) (required ≥ 40%) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4685195430 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4685203768 @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4680520112 [SF] Trillian test result (tid-16284) Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8 Total time taken: 52854 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12617-t16284-kvm-ol8.zip Smoke tests completed. 149 look OK, 2 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_03_ping_in_ssvm_success | `Failure` | 15.53 | test_diagnostics.py test_05_ping_in_cpvm_success | `Failure` | 15.53 | test_diagnostics.py test_isolate_network_password_server | `Failure` | 12.93 | test_password_server.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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-467106 [SF] Trillian test result (tid-16267) Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8 Total time taken: 171330 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12617-t16267-kvm-ol8.zip Smoke tests completed. 90 look OK, 61 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py test_02_upgrade_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py test_04_autoscale_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py test_05_basic_lifecycle_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py test_06_delete_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py test_08_upgrade_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py test_10_vpc_tier_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py test_11_test_unmanaged_cluster_lifecycle | `Error` | 0.00 | test_kubernetes_clusters.py test_12_test_deploy_cluster_different_offerings_per_node_type | `Failure` | 0.00 | test_kubernetes_clusters.py test_01_add_delete_kubernetes_supported_version | `Error` | 1802.26 | test_kubernetes_supported_versions.py ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py ContextSuite context=TestListVolumes>:setup | `Error` | 0.00 | test_list_volumes.py ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py ContextSuite context=TestMetrics>:setup | `Error` | 0.00 | test_metrics_api.py test_01_native_to_native_network_migration | `Error` | 81.17 | test_migration.py test_02_native_to_native_vpc_migration | `Error` | 97.37 | test_migration.py test_nic_secondaryip_add_remove | `Error` | 1518.76 | test_multipleips_per_nic.py ContextSuite context=TestNestedVirtualization>:setup | `Error` | 0.00 | test_nested_virtualization.py ContextSuite context=TestNetworkACL>:setup | `Error` | 0.00 | test_network_acl.py ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py ContextSuite context=TestIpv6Network>:setup | `Error` | 0.00 | test_network_ipv6.py test_03_network_operations_on_created_vm_of_otheruser | `Error` | 63.98 | test_network_permissions.py test_03_network_operations_on_created_vm_of_otheruser | `Error` | 63.99 | test_network_permissions.py test_04_deploy_vm_for_other_user_and_test_vm_operations | `Failure` | 66.03 | test_network_permissions.py ContextSuite context=TestNetworkPermissions>:teardown | `Error` | 2.38 | test_network_permissions.py test_delete_account | `Error` | 1517.58 | test_network.py test_delete_network_while_vm_on_it | `Error` | 1.19 | test_network.py test_deploy_vm_l2network | `Error` | 1.59 | test_network.py test_l2network_restart | `Error` | 2.33 | test_network.py ContextSuite context=TestPortForwarding>:setup | `Error` | 3.55 | test_network.py ContextSuite context=TestPublicIP>:setup | `Error` | 6.11 | test_network.py test_reboot_router | `Failure` | 0.08 | test_network.py test_releaseIP | `Error` | 3.06 | test_network.py test_releaseIP_using_IP | `Error` | 3.08 | test_network.py ContextSuite context=TestRouterRules>:setup | `Error` | 3.15 | test_network.py test_01_deployVMInSharedNetwork | `Failure` | 64.19 | test_network.py test_03_destroySharedNetwork | `Failure` | 1.09 | test_network.py ContextSuite context=TestSharedNetwork>:teardown | `Error` | 1.20 | test_network.py ContextSuite context=TestSharedNetworkWithConfigDrive>:setup | `Error` | 1519.05 | test_network.py ContextSuite context=TestAdapterTypeForNic>:setup | `Error` | 0.00 | test_nic_adapter_type.py test_01_nic | `Error` | 112.95 | test_nic.py ContextSuite context=TestNonStrictAffinityGroups>:setup | `Error` | 0.00 | test_nonstrict_affinity_group.py test_03_deploy_and_destroy_VM_and_verify_network_resources_persist | `Failure` | 6.77 | test_persistent_network.py test_03_deploy_and_destroy_VM_and_verify_network_resources_persist | `Error` | 6.77 | test_persistent_network.py ContextSuite context=TestL2PersistentNetworks>:teardown | `Error` | 6.87 | test_persistent_network.py ContextSuite context=TestPortForwardingRules>:setup | `Error` | 0.00 | test_portforwardingrules.py test_01_add_primary_storage_disabled_host | `Error` | 41.03 | test_primary_storage.py test_01_primary_storage_nfs | `Error` | 0.26 | test_primary_storage.py ContextSuite context=TestStorageTags>:setup | `Error` | 0.43 |
Re: [PR] CLVM enhancements and fixes [cloudstack]
sonarqubecloud[bot] commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4674421208 ## [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) **Quality Gate failed** Failed conditions  [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=12617&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [33.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=12617&metric=new_coverage&view=list) (required ≥ 40%) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4674403890 @Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4674385289 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4674357742 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18221 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4673976769 @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4673964991 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4673923666 [SF] Trillian test result (tid-16274) Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8 Total time taken: 56161 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12617-t16274-kvm-ol8.zip Smoke tests completed. 151 look OK, 0 have errors, 0 did not run Only failed and skipped 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4673702739 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18218 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
sonarqubecloud[bot] commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4673457632 ## [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) **Quality Gate failed** Failed conditions  [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=12617&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [33.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=12617&metric=new_coverage&view=list) (required ≥ 40%) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
winterhazel commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3383645679
##
server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java:
##
@@ -1659,6 +1662,9 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume)
throws ResourceAllocationExc
if (backupSnapToSecondary) {
if (!isKvmAndFileBasedStorage) {
backupSnapshotToSecondary(payload.getAsyncBackup(),
snapshotStrategy, snapshotOnPrimary, payload.getZoneIds(),
payload.getStoragePoolIds());
+if (!payload.getAsyncBackup() &&
(storagePool.getPoolType() == StoragePoolType.CLVM || storagePool.getPoolType()
== StoragePoolType.CLVM_NG)) {
Review Comment:
Use `ClvmPoolManager.isClvmPoolType` here
##
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##
@@ -2738,21 +2746,42 @@ private Volume orchestrateAttachVolumeToVM(Long vmId,
Long volumeId, Long device
logger.trace(String.format("is it needed to move the volume: %b?",
moveVolumeNeeded));
}
-if (moveVolumeNeeded) {
+// Check if CLVM lock transfer is needed (even if moveVolumeNeeded is
false)
+// This handles the case where the volume is already on the correct
storage pool
+// but the VM is running on a different host, requiring only a lock
transfer
+boolean isClvmLockTransferNeeded = !moveVolumeNeeded &&
+isClvmLockTransferRequired(newVolumeOnPrimaryStorage,
existingVolumeOfVm, vm);
+
+if (isClvmLockTransferNeeded) {
+// CLVM lock transfer - no data copy, no pool change needed
+newVolumeOnPrimaryStorage = executeLightweightLockMigration(
+newVolumeOnPrimaryStorage, vm, existingVolumeOfVm,
+"CLVM lock transfer", "same pool to different host");
+} else if (moveVolumeNeeded) {
PrimaryDataStoreInfo primaryStore =
(PrimaryDataStoreInfo)newVolumeOnPrimaryStorage.getDataStore();
if (primaryStore.isLocal()) {
throw new CloudRuntimeException(
"Failed to attach local data volume " +
volumeToAttach.getName() + " to VM " + vm.getDisplayName() + " as migration of
local data volume is not allowed");
}
-StoragePoolVO vmRootVolumePool =
_storagePoolDao.findById(existingVolumeOfVm.getPoolId());
-try {
-HypervisorType volumeToAttachHyperType =
_volsDao.getHypervisorType(volumeToAttach.getId());
-newVolumeOnPrimaryStorage =
_volumeMgr.moveVolume(newVolumeOnPrimaryStorage,
vmRootVolumePool.getDataCenterId(), vmRootVolumePool.getPodId(),
vmRootVolumePool.getClusterId(),
-volumeToAttachHyperType);
-} catch (ConcurrentOperationException |
StorageUnavailableException e) {
-logger.debug("move volume failed", e);
-throw new CloudRuntimeException("move volume failed", e);
+boolean isClvmLightweightMigration =
isClvmLightweightMigrationNeeded(
+newVolumeOnPrimaryStorage, existingVolumeOfVm, vm);
+
+if (isClvmLightweightMigration) {
+newVolumeOnPrimaryStorage = executeLightweightLockMigration(
+newVolumeOnPrimaryStorage, vm, existingVolumeOfVm,
+"CLVM lightweight migration", "different pools, same
VG");
+} else {
+StoragePoolVO vmRootVolumePool =
_storagePoolDao.findById(existingVolumeOfVm.getPoolId());
+
+try {
+HypervisorType volumeToAttachHyperType =
_volsDao.getHypervisorType(volumeToAttach.getId());
+newVolumeOnPrimaryStorage =
_volumeMgr.moveVolume(newVolumeOnPrimaryStorage,
vmRootVolumePool.getDataCenterId(), vmRootVolumePool.getPodId(),
vmRootVolumePool.getClusterId(),
+volumeToAttachHyperType);
+} catch (ConcurrentOperationException |
StorageUnavailableException e) {
+logger.debug("move volume failed", e);
+throw new CloudRuntimeException("move volume failed", e);
+}
}
Review Comment:
This should be a straightfoward refactor. I would prefer having it on this
one already, but its up to you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4673272959 @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4673253876 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3390412744
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ClvmStorageAdaptor.java:
##
@@ -0,0 +1,1072 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.hypervisor.kvm.storage;
+
+import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
+
+import java.io.File;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.hypervisor.kvm.resource.LibvirtConnection;
+import com.cloud.hypervisor.kvm.resource.LibvirtStoragePoolDef;
+import com.cloud.hypervisor.kvm.resource.LibvirtStorageVolumeDef;
+import com.cloud.storage.Storage;
+import com.cloud.storage.Storage.StoragePoolType;
+import com.cloud.storage.StorageLayer;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.OutputInterpreter;
+import com.cloud.utils.script.Script;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonParser;
+import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
+import org.joda.time.Duration;
+import org.libvirt.Connect;
+import org.libvirt.LibvirtException;
+import org.libvirt.StoragePool;
+import org.libvirt.StorageVol;
+
+/**
+ * Storage adaptor for CLVM and CLVM_NG pool types.
+ * Extends LibvirtStorageAdaptor and overrides methods with CLVM-specific
logic,
+ * using direct LVM commands instead of libvirt for volume operations.
+ */
+public class ClvmStorageAdaptor extends LibvirtStorageAdaptor {
+
+public ClvmStorageAdaptor(StorageLayer storage) {
+super(storage);
+}
+
+@Override
+public StoragePoolType getStoragePoolType() {
+// Registered manually for both CLVM and CLVM_NG in
KVMStoragePoolManager
+return null;
+}
+
+@Override
+public KVMStoragePool createStoragePool(String name, String host, int
port, String path,
+String userInfo, StoragePoolType type, Map
details, boolean isPrimaryStorage) {
+logger.info("Attempting to create CLVM/CLVM_NG storage pool {} in
libvirt", name);
+
+Connect conn;
+try {
+conn = LibvirtConnection.getConnection();
+} catch (LibvirtException e) {
+throw new CloudRuntimeException(e.toString());
+}
+
+StoragePool sp = createCLVMStoragePool(conn, name, host, path);
+if (sp == null) {
+logger.info("Falling back to virtual CLVM/CLVM_NG pool without
libvirt for: {}", name);
+return createVirtualClvmPool(name, host, path, type, details);
+}
+
+try {
+if (!isPrimaryStorage) {
+incStoragePoolRefCount(name);
+}
+// CLVM/CLVM_NG pools are kept inactive in libvirt; we use direct
LVM commands
+return getStoragePool(name);
+} catch (Exception e) {
+decStoragePoolRefCount(name);
+throw new CloudRuntimeException("Failed to create CLVM storage
pool: " + name, e);
+}
+}
+
+@Override
+public KVMStoragePool getStoragePool(String uuid, boolean refreshInfo) {
+logger.info("Fetching CLVM/CLVM_NG storage pool {} ", uuid);
+try {
+Connect conn = LibvirtConnection.getConnection();
+StoragePool storage = conn.storagePoolLookupByUUIDString(uuid);
+
+LibvirtStoragePoolDef spd = getStoragePoolDef(conn, storage);
+if (spd == null) {
+throw new CloudRuntimeException("Unable to parse storage pool
definition for pool " + uuid);
+}
+
+// CLVM pools in libvirt are always LOGICAL type
+StoragePoolType type = StoragePoolType.CLVM;
+
+// Do NOT activate the pool — CLVM/CLVM_NG pools stay inactive in
libvirt
+LibvirtStoragePool pool = new LibvirtStoragePool(uuid,
storage.getName(), type, this, storage);
+pool.setLocalPath(spd.getTargetPath());
+
+// Always read capacity from LVM directly
+String vgName = storage.getName();
+
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3390406146
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##
@@ -6820,4 +6827,242 @@ public String getHypervisorPath() {
public String getGuestCpuArch() {
return guestCpuArch;
}
+
+/**
+ * CLVM volume state for migration operations on source host
+ */
+public enum ClvmVolumeState {
+/** Shared mode (-asy) - used before migration to allow both hosts to
access volume */
+SHARED("-asy", "shared", "Before migration: activating in shared
mode"),
+
+/** Deactivate (-an) - used after successful migration to release
volume on source */
+DEACTIVATE("-an", "deactivated", "After successful migration:
deactivating volume"),
+
+/** Exclusive mode (-aey) - used after failed migration to revert to
original exclusive state */
+EXCLUSIVE("-aey", "exclusive", "After failed migration: reverting to
exclusive mode");
+
+private final String lvchangeFlag;
+private final String description;
+private final String logMessage;
+
+ClvmVolumeState(String lvchangeFlag, String description, String
logMessage) {
+this.lvchangeFlag = lvchangeFlag;
+this.description = description;
+this.logMessage = logMessage;
+}
+
+public String getLvchangeFlag() {
+return lvchangeFlag;
+}
+
+public String getDescription() {
+return description;
+}
+
+public String getLogMessage() {
+return logMessage;
+}
+}
+
+public static void modifyClvmVolumesStateForMigration(List disks,
LibvirtComputingResource resource,
+ VirtualMachineTO
vmSpec, ClvmVolumeState state) {
+for (DiskDef disk : disks) {
+if (isClvmVolume(disk, resource, vmSpec)) {
+String volumePath = disk.getDiskPath();
+try {
+modifyClvmVolumeState(volumePath, state.getLvchangeFlag(),
state.getDescription(), state.getLogMessage());
+} catch (Exception e) {
+LOGGER.error("[CLVM Migration] Exception while setting
volume [{}] to {} state: {}",
+volumePath, state.getDescription(),
e.getMessage(), e);
+}
+}
+}
+}
+
+private static void modifyClvmVolumeState(String volumePath, String
lvchangeFlag,
+ String stateDescription, String
logMessage) {
+try {
+LOGGER.info("{} for volume [{}]", logMessage, volumePath);
+
+Script cmd = new Script("lvchange", Duration.standardSeconds(300),
LOGGER);
+cmd.add(lvchangeFlag);
+cmd.add(volumePath);
+
+String result = cmd.execute();
+if (result != null) {
+String errorMsg = String.format(
+"Failed to set volume [%s] to %s state. Command
result: %s",
+volumePath, stateDescription, result);
+LOGGER.error(errorMsg);
+throw new CloudRuntimeException(errorMsg);
+} else {
+LOGGER.info("Successfully set volume [{}] to {} state.",
+volumePath, stateDescription);
+}
+} catch (CloudRuntimeException e) {
+throw e;
+} catch (Exception e) {
+String errorMsg = String.format(
+"Exception while setting volume [%s] to %s state: %s",
+volumePath, stateDescription, e.getMessage());
+LOGGER.error(errorMsg, e);
+throw new CloudRuntimeException(errorMsg, e);
+}
+}
+
+public static void activateClvmVolumeExclusive(String volumePath) {
Review Comment:
as explained above
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##
@@ -6820,4 +6827,242 @@ public String getHypervisorPath() {
public String getGuestCpuArch() {
return guestCpuArch;
}
+
+/**
+ * CLVM volume state for migration operations on source host
+ */
+public enum ClvmVolumeState {
+/** Shared mode (-asy) - used before migration to allow both hosts to
access volume */
+SHARED("-asy", "shared", "Before migration: activating in shared
mode"),
+
+/** Deactivate (-an) - used after successful migration to release
volume on source */
+DEACTIVATE("-an", "deactivated", "After successful migration:
deactivating volume"),
+
+/** Exclusive mode (-aey) - used after failed migration to revert to
original exclusive state */
+EXCLUSIVE("-aey", "exclusive", "After failed migration: reverting to
exclusive mode");
+
+
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3390404701
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##
@@ -6820,4 +6827,242 @@ public String getHypervisorPath() {
public String getGuestCpuArch() {
return guestCpuArch;
}
+
+/**
+ * CLVM volume state for migration operations on source host
+ */
+public enum ClvmVolumeState {
+/** Shared mode (-asy) - used before migration to allow both hosts to
access volume */
+SHARED("-asy", "shared", "Before migration: activating in shared
mode"),
+
+/** Deactivate (-an) - used after successful migration to release
volume on source */
+DEACTIVATE("-an", "deactivated", "After successful migration:
deactivating volume"),
+
+/** Exclusive mode (-aey) - used after failed migration to revert to
original exclusive state */
+EXCLUSIVE("-aey", "exclusive", "After failed migration: reverting to
exclusive mode");
+
+private final String lvchangeFlag;
+private final String description;
+private final String logMessage;
+
+ClvmVolumeState(String lvchangeFlag, String description, String
logMessage) {
+this.lvchangeFlag = lvchangeFlag;
+this.description = description;
+this.logMessage = logMessage;
+}
+
+public String getLvchangeFlag() {
+return lvchangeFlag;
+}
+
+public String getDescription() {
+return description;
+}
+
+public String getLogMessage() {
+return logMessage;
+}
+}
+
+public static void modifyClvmVolumesStateForMigration(List disks,
LibvirtComputingResource resource,
Review Comment:
these methods don't rely on any instance field of LibvirtComputingResource,
so having them static makes sense.
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##
@@ -6820,4 +6827,242 @@ public String getHypervisorPath() {
public String getGuestCpuArch() {
return guestCpuArch;
}
+
+/**
+ * CLVM volume state for migration operations on source host
+ */
+public enum ClvmVolumeState {
+/** Shared mode (-asy) - used before migration to allow both hosts to
access volume */
+SHARED("-asy", "shared", "Before migration: activating in shared
mode"),
+
+/** Deactivate (-an) - used after successful migration to release
volume on source */
+DEACTIVATE("-an", "deactivated", "After successful migration:
deactivating volume"),
+
+/** Exclusive mode (-aey) - used after failed migration to revert to
original exclusive state */
+EXCLUSIVE("-aey", "exclusive", "After failed migration: reverting to
exclusive mode");
+
+private final String lvchangeFlag;
+private final String description;
+private final String logMessage;
+
+ClvmVolumeState(String lvchangeFlag, String description, String
logMessage) {
+this.lvchangeFlag = lvchangeFlag;
+this.description = description;
+this.logMessage = logMessage;
+}
+
+public String getLvchangeFlag() {
+return lvchangeFlag;
+}
+
+public String getDescription() {
+return description;
+}
+
+public String getLogMessage() {
+return logMessage;
+}
+}
+
+public static void modifyClvmVolumesStateForMigration(List disks,
LibvirtComputingResource resource,
+ VirtualMachineTO
vmSpec, ClvmVolumeState state) {
+for (DiskDef disk : disks) {
+if (isClvmVolume(disk, resource, vmSpec)) {
+String volumePath = disk.getDiskPath();
+try {
+modifyClvmVolumeState(volumePath, state.getLvchangeFlag(),
state.getDescription(), state.getLogMessage());
+} catch (Exception e) {
+LOGGER.error("[CLVM Migration] Exception while setting
volume [{}] to {} state: {}",
+volumePath, state.getDescription(),
e.getMessage(), e);
+}
+}
+}
+}
+
+private static void modifyClvmVolumeState(String volumePath, String
lvchangeFlag,
Review Comment:
same 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3389667999
##
engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java:
##
@@ -2320,7 +2434,26 @@ String getVolumeBackingFile(VolumeInfo srcVolumeInfo) {
return null;
}
-private void handlePostMigration(boolean success, Map srcVolumeInfoToDestVolumeInfo, VirtualMachineTO vmTO, Host
destHost) {
+private void sendClvmLockCommand(long hostId, StoragePoolVO pool,
VolumeInfo volumeInfo,
+ClvmLockTransferCommand.Operation operation) {
+String vgName = pool.getPath();
+if (vgName.startsWith("/")) {
+vgName = vgName.substring(1);
+}
+String lvPath = String.format("/dev/%s/%s", vgName,
volumeInfo.getPath());
+try {
+Answer answer = agentManager.send(hostId,
+new ClvmLockTransferCommand(operation, lvPath,
volumeInfo.getUuid()));
+if (answer == null || !answer.getResult()) {
+String details = answer != null ? answer.getDetails() : "null
answer";
+logger.warn("CLVM lock command [{}] failed for LV [{}] on host
[{}]: {}", operation, lvPath, hostId, details);
Review Comment:
The locking here is not about making the device accessible for the migration
itself, that's handled earlier (pre-migration ACTIVATE_SHARED, which
hard-fails). These post-migration ACTIVATE_EXCLUSIVE calls are state
normalization.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3389085487
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java:
##
@@ -1920,6 +2131,15 @@ private SnapshotObjectTO
takeIncrementalVolumeSnapshotOfStoppedVm(SnapshotObject
logger.debug("Taking incremental volume snapshot of volume [{}].
Snapshot will be copied to [{}].", volumeObjectTo,
ObjectUtils.defaultIfNull(secondaryPool, primaryPool));
try {
+// For CLVM_NG incremental snapshots, validate bitmap before
proceeding
+/*
+SnapshotObjectTO bitmapValidationResult =
validateClvmNgBitmapAndFallbackIfNeeded(snapshotObjectTO, primaryPool,
+secondaryPool, secondaryPoolUrl, snapshotName,
volumeObjectTo, conn, wait);
+if (bitmapValidationResult != null) {
+return bitmapValidationResult;
+}
+ */
Review Comment:
Thanks, will do - I was working on a separate PR to support incremental
snaps for CLVM pool type, but the bitmap solution may not be suitable. So yes,
I'll clean it up. Thanks for the review, much appreciated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4666171975 @Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4666164097 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4666130850 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18209 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
sonarqubecloud[bot] commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4666099694 ## [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) **Quality Gate failed** Failed conditions  [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=12617&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [33.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=12617&metric=new_coverage&view=list) (required ≥ 40%) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4665900309 @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4665893341 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4664728859 [SF] Trillian test result (tid-16265) Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8 Total time taken: 94885 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12617-t16265-kvm-ol8.zip Smoke tests completed. 136 look OK, 15 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_CRUD_operations_userdata | `Error` | 1522.99 | test_register_userdata.py test_deploy_vm_with_registered_userdata | `Error` | 5.58 | test_register_userdata.py test_deploy_vm_with_registered_userdata_with_override_policy_allow | `Error` | 5.55 | test_register_userdata.py test_deploy_vm_with_registered_userdata_with_override_policy_append | `Error` | 6.48 | test_register_userdata.py test_deploy_vm_with_registered_userdata_with_override_policy_deny | `Error` | 5.56 | test_register_userdata.py test_deploy_vm_with_registered_userdata_with_params | `Error` | 6.65 | test_register_userdata.py test_link_and_unlink_userdata_to_template | `Error` | 5.73 | test_register_userdata.py test_user_userdata_crud | `Error` | 9.72 | test_register_userdata.py ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py test_03_register_template | `Error` | 1.13 | test_resource_names.py ContextSuite context=TestRestoreVM>:setup | `Error` | 0.00 | test_restore_vm.py ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py ContextSuite context=TestRouterIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py ContextSuite context=TestVPCIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py ContextSuite context=TestIsolatedNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py ContextSuite context=TestCpuCapServiceOfferings>:setup | `Error` | 0.00 | test_service_offerings.py ContextSuite context=TestServiceOfferings>:setup | `Error` | 0.30 | test_service_offerings.py ContextSuite context=TestSetSourceNatIp>:setup | `Error` | 0.00 | test_set_sourcenat.py ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py ContextSuite context=TestSnapshotStandaloneBackup>:setup | `Error` | 0.00 | test_snapshots.py ContextSuite context=TestSslOffloading>:setup | `Error` | 0.00 | test_ssl_offloading.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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
sonarqubecloud[bot] commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4664392780 ## [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) **Quality Gate failed** Failed conditions  [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=12617&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [34.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=12617&metric=new_coverage&view=list) (required ≥ 40%) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3383633597
##
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##
@@ -2738,21 +2746,42 @@ private Volume orchestrateAttachVolumeToVM(Long vmId,
Long volumeId, Long device
logger.trace(String.format("is it needed to move the volume: %b?",
moveVolumeNeeded));
}
-if (moveVolumeNeeded) {
+// Check if CLVM lock transfer is needed (even if moveVolumeNeeded is
false)
+// This handles the case where the volume is already on the correct
storage pool
+// but the VM is running on a different host, requiring only a lock
transfer
+boolean isClvmLockTransferNeeded = !moveVolumeNeeded &&
+isClvmLockTransferRequired(newVolumeOnPrimaryStorage,
existingVolumeOfVm, vm);
+
+if (isClvmLockTransferNeeded) {
+// CLVM lock transfer - no data copy, no pool change needed
+newVolumeOnPrimaryStorage = executeLightweightLockMigration(
+newVolumeOnPrimaryStorage, vm, existingVolumeOfVm,
+"CLVM lock transfer", "same pool to different host");
+} else if (moveVolumeNeeded) {
PrimaryDataStoreInfo primaryStore =
(PrimaryDataStoreInfo)newVolumeOnPrimaryStorage.getDataStore();
if (primaryStore.isLocal()) {
throw new CloudRuntimeException(
"Failed to attach local data volume " +
volumeToAttach.getName() + " to VM " + vm.getDisplayName() + " as migration of
local data volume is not allowed");
}
-StoragePoolVO vmRootVolumePool =
_storagePoolDao.findById(existingVolumeOfVm.getPoolId());
-try {
-HypervisorType volumeToAttachHyperType =
_volsDao.getHypervisorType(volumeToAttach.getId());
-newVolumeOnPrimaryStorage =
_volumeMgr.moveVolume(newVolumeOnPrimaryStorage,
vmRootVolumePool.getDataCenterId(), vmRootVolumePool.getPodId(),
vmRootVolumePool.getClusterId(),
-volumeToAttachHyperType);
-} catch (ConcurrentOperationException |
StorageUnavailableException e) {
-logger.debug("move volume failed", e);
-throw new CloudRuntimeException("move volume failed", e);
+boolean isClvmLightweightMigration =
isClvmLightweightMigrationNeeded(
+newVolumeOnPrimaryStorage, existingVolumeOfVm, vm);
+
+if (isClvmLightweightMigration) {
+newVolumeOnPrimaryStorage = executeLightweightLockMigration(
+newVolumeOnPrimaryStorage, vm, existingVolumeOfVm,
+"CLVM lightweight migration", "different pools, same
VG");
+} else {
+StoragePoolVO vmRootVolumePool =
_storagePoolDao.findById(existingVolumeOfVm.getPoolId());
+
+try {
+HypervisorType volumeToAttachHyperType =
_volsDao.getHypervisorType(volumeToAttach.getId());
+newVolumeOnPrimaryStorage =
_volumeMgr.moveVolume(newVolumeOnPrimaryStorage,
vmRootVolumePool.getDataCenterId(), vmRootVolumePool.getPodId(),
vmRootVolumePool.getClusterId(),
+volumeToAttachHyperType);
+} catch (ConcurrentOperationException |
StorageUnavailableException e) {
+logger.debug("move volume failed", e);
+throw new CloudRuntimeException("move volume failed", e);
+}
}
Review Comment:
Would it be ok if I addressed this in another PR - with other enhancements
that are yet to come.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3383644746
##
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##
@@ -2748,6 +2777,204 @@ private Volume orchestrateAttachVolumeToVM(Long vmId,
Long volumeId, Long device
return newVol;
}
+/**
+ * Helper method to get storage pools for volume and VM.
+ *
+ * @param volumeToAttach The volume being attached
+ * @param vmExistingVolume The VM's existing volume
+ * @return Pair of StoragePoolVO objects (volumePool, vmPool), or null if
either pool is missing
+ */
+private Pair
getStoragePoolsForVolumeAttachment(VolumeInfo volumeToAttach, VolumeVO
vmExistingVolume) {
+if (volumeToAttach == null || vmExistingVolume == null) {
+return null;
+}
+
+StoragePoolVO volumePool =
_storagePoolDao.findById(volumeToAttach.getPoolId());
+StoragePoolVO vmPool =
_storagePoolDao.findById(vmExistingVolume.getPoolId());
+
+if (volumePool == null || vmPool == null) {
+return null;
+}
+
+return new Pair<>(volumePool, vmPool);
+}
+
+/**
+ * Checks if both storage pools are CLVM type (CLVM or CLVM_NG).
+ * Delegates to VolumeService for the actual check.
+ *
+ * @param volumePool Storage pool for the volume
+ * @param vmPool Storage pool for the VM
+ * @return true if both pools are CLVM type (CLVM or CLVM_NG)
+ */
+private boolean areBothPoolsClvmType(StoragePoolVO volumePool,
StoragePoolVO vmPool) {
+return volService.areBothPoolsClvmType(volumePool.getPoolType(),
vmPool.getPoolType());
+}
+
+/**
+ * Determines if a CLVM volume needs lightweight lock migration instead of
full data copy.
+ *
+ * Lightweight migration is needed when:
+ * 1. Volume is on CLVM storage
+ * 2. Source and destination are in the same Volume Group
+ * 3. Only the host/lock needs to change (not the storage pool)
+ *
+ * @param volumeToAttach The volume being attached
+ * @param vmExistingVolume The VM's existing volume (typically root volume)
+ * @param vm The VM to attach the volume to
+ * @return true if lightweight CLVM lock migration should be used
+ */
+private boolean isClvmLightweightMigrationNeeded(VolumeInfo
volumeToAttach, VolumeVO vmExistingVolume, UserVmVO vm) {
+Pair pools =
getStoragePoolsForVolumeAttachment(volumeToAttach, vmExistingVolume);
+if (pools == null) {
+return false;
+}
+
+StoragePoolVO volumePool = pools.first();
+StoragePoolVO vmPool = pools.second();
+
+return
volService.isLightweightMigrationNeeded(volumePool.getPoolType(),
vmPool.getPoolType(),
+volumePool.getPath(), vmPool.getPath());
+}
+
+/**
+ * Determines if a CLVM volume requires lock transfer when already on the
correct storage pool.
+ *
+ * Lock transfer is needed when:
+ * 1. Volume is already on the same CLVM storage pool as VM's volumes
+ * 2. But the volume lock is held by a different host than where the VM is
running
+ * 3. Only the lock needs to change (no pool change, no data copy)
+ *
+ * @param volumeToAttach The volume being attached
+ * @param vmExistingVolume The VM's existing volume (typically root volume)
+ * @param vm The VM to attach the volume to
+ * @return true if CLVM lock transfer is needed (but not full migration)
+ */
+private boolean isClvmLockTransferRequired(VolumeInfo volumeToAttach,
VolumeVO vmExistingVolume, UserVmVO vm) {
+if (vm == null) {
+return false;
+}
+
+Pair pools =
getStoragePoolsForVolumeAttachment(volumeToAttach, vmExistingVolume);
+if (pools == null) {
+return false;
+}
+
+StoragePoolVO volumePool = pools.first();
+StoragePoolVO vmPool = pools.second();
+
+Long vmHostId = vm.getHostId();
+if (vmHostId == null) {
+vmHostId = vm.getLastHostId();
+}
+
+return volService.isLockTransferRequired(volumeToAttach,
volumePool.getPoolType(), vmPool.getPoolType(),
+volumePool.getId(), vmPool.getId(), vmHostId);
+}
+
+/**
+ * Determines the destination host for CLVM lock migration.
+ *
+ * If VM is running, uses the VM's current host.
+ * If VM is stopped, picks an available UP host from the storage pool's
cluster.
+ *
+ * @param vm The VM
+ * @param vmExistingVolume The VM's existing volume (to determine cluster)
+ * @return Host ID, or null if cannot be determined
+ */
+private Long determineClvmLockDestinationHost(UserVmVO vm, VolumeVO
vmExistingVolume) {
+Long destHostId = vm.getHostId();
+if (destHostId != null) {
+return destHostId;
+}
+
+if (vmExistingVolume != null && vmExistingVo
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3383633597
##
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##
@@ -2738,21 +2746,42 @@ private Volume orchestrateAttachVolumeToVM(Long vmId,
Long volumeId, Long device
logger.trace(String.format("is it needed to move the volume: %b?",
moveVolumeNeeded));
}
-if (moveVolumeNeeded) {
+// Check if CLVM lock transfer is needed (even if moveVolumeNeeded is
false)
+// This handles the case where the volume is already on the correct
storage pool
+// but the VM is running on a different host, requiring only a lock
transfer
+boolean isClvmLockTransferNeeded = !moveVolumeNeeded &&
+isClvmLockTransferRequired(newVolumeOnPrimaryStorage,
existingVolumeOfVm, vm);
+
+if (isClvmLockTransferNeeded) {
+// CLVM lock transfer - no data copy, no pool change needed
+newVolumeOnPrimaryStorage = executeLightweightLockMigration(
+newVolumeOnPrimaryStorage, vm, existingVolumeOfVm,
+"CLVM lock transfer", "same pool to different host");
+} else if (moveVolumeNeeded) {
PrimaryDataStoreInfo primaryStore =
(PrimaryDataStoreInfo)newVolumeOnPrimaryStorage.getDataStore();
if (primaryStore.isLocal()) {
throw new CloudRuntimeException(
"Failed to attach local data volume " +
volumeToAttach.getName() + " to VM " + vm.getDisplayName() + " as migration of
local data volume is not allowed");
}
-StoragePoolVO vmRootVolumePool =
_storagePoolDao.findById(existingVolumeOfVm.getPoolId());
-try {
-HypervisorType volumeToAttachHyperType =
_volsDao.getHypervisorType(volumeToAttach.getId());
-newVolumeOnPrimaryStorage =
_volumeMgr.moveVolume(newVolumeOnPrimaryStorage,
vmRootVolumePool.getDataCenterId(), vmRootVolumePool.getPodId(),
vmRootVolumePool.getClusterId(),
-volumeToAttachHyperType);
-} catch (ConcurrentOperationException |
StorageUnavailableException e) {
-logger.debug("move volume failed", e);
-throw new CloudRuntimeException("move volume failed", e);
+boolean isClvmLightweightMigration =
isClvmLightweightMigrationNeeded(
+newVolumeOnPrimaryStorage, existingVolumeOfVm, vm);
+
+if (isClvmLightweightMigration) {
+newVolumeOnPrimaryStorage = executeLightweightLockMigration(
+newVolumeOnPrimaryStorage, vm, existingVolumeOfVm,
+"CLVM lightweight migration", "different pools, same
VG");
+} else {
+StoragePoolVO vmRootVolumePool =
_storagePoolDao.findById(existingVolumeOfVm.getPoolId());
+
+try {
+HypervisorType volumeToAttachHyperType =
_volsDao.getHypervisorType(volumeToAttach.getId());
+newVolumeOnPrimaryStorage =
_volumeMgr.moveVolume(newVolumeOnPrimaryStorage,
vmRootVolumePool.getDataCenterId(), vmRootVolumePool.getPodId(),
vmRootVolumePool.getClusterId(),
+volumeToAttachHyperType);
+} catch (ConcurrentOperationException |
StorageUnavailableException e) {
+logger.debug("move volume failed", e);
+throw new CloudRuntimeException("move volume failed", e);
+}
}
Review Comment:
Would it be ok if I addressed this is another PR - with other enhancements
that are yet to come.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4663657234 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18208 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
winterhazel commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3383498299
##
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##
@@ -2767,6 +2796,203 @@ private Volume orchestrateAttachVolumeToVM(Long vmId,
Long volumeId, Long device
return newVol;
}
+/**
+ * Helper method to get storage pools for volume and VM.
+ *
+ * @param volumeToAttach The volume being attached
+ * @param vmExistingVolume The VM's existing volume
+ * @return Pair of StoragePoolVO objects (volumePool, vmPool), or null if
either pool is missing
+ */
+private Pair
getStoragePoolsForVolumeAttachment(VolumeInfo volumeToAttach, VolumeVO
vmExistingVolume) {
+if (volumeToAttach == null || vmExistingVolume == null) {
+return null;
+}
+
+StoragePoolVO volumePool =
_storagePoolDao.findById(volumeToAttach.getPoolId());
+StoragePoolVO vmPool =
_storagePoolDao.findById(vmExistingVolume.getPoolId());
+
+if (volumePool == null || vmPool == null) {
+return null;
+}
+
+return new Pair<>(volumePool, vmPool);
+}
+
+/**
+ * Checks if both storage pools are CLVM type (CLVM or CLVM_NG).
+ * Delegates to VolumeService for the actual check.
+ *
+ * @param volumePool Storage pool for the volume
+ * @param vmPool Storage pool for the VM
+ * @return true if both pools are CLVM type (CLVM or CLVM_NG)
+ */
+private boolean areBothPoolsClvmType(StoragePoolVO volumePool,
StoragePoolVO vmPool) {
Review Comment:
This one is also unused
##
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##
@@ -2767,6 +2796,203 @@ private Volume orchestrateAttachVolumeToVM(Long vmId,
Long volumeId, Long device
return newVol;
}
+/**
+ * Helper method to get storage pools for volume and VM.
+ *
+ * @param volumeToAttach The volume being attached
+ * @param vmExistingVolume The VM's existing volume
+ * @return Pair of StoragePoolVO objects (volumePool, vmPool), or null if
either pool is missing
+ */
+private Pair
getStoragePoolsForVolumeAttachment(VolumeInfo volumeToAttach, VolumeVO
vmExistingVolume) {
+if (volumeToAttach == null || vmExistingVolume == null) {
+return null;
+}
+
+StoragePoolVO volumePool =
_storagePoolDao.findById(volumeToAttach.getPoolId());
+StoragePoolVO vmPool =
_storagePoolDao.findById(vmExistingVolume.getPoolId());
+
+if (volumePool == null || vmPool == null) {
+return null;
+}
+
+return new Pair<>(volumePool, vmPool);
+}
+
+/**
+ * Checks if both storage pools are CLVM type (CLVM or CLVM_NG).
+ * Delegates to VolumeService for the actual check.
+ *
+ * @param volumePool Storage pool for the volume
+ * @param vmPool Storage pool for the VM
+ * @return true if both pools are CLVM type (CLVM or CLVM_NG)
+ */
+private boolean areBothPoolsClvmType(StoragePoolVO volumePool,
StoragePoolVO vmPool) {
+return volService.areBothPoolsClvmType(volumePool.getPoolType(),
vmPool.getPoolType());
+}
+
+/**
+ * Determines if a CLVM volume needs lightweight lock migration instead of
full data copy.
+ *
+ * Lightweight migration is needed when:
+ * 1. Volume is on CLVM storage
+ * 2. Source and destination are in the same Volume Group
+ * 3. Only the host/lock needs to change (not the storage pool)
+ *
+ * @param volumeToAttach The volume being attached
+ * @param vmExistingVolume The VM's existing volume (typically root volume)
+ * @param vm The VM to attach the volume to
+ * @return true if lightweight CLVM lock migration should be used
+ */
+private boolean isClvmLightweightMigrationNeeded(VolumeInfo
volumeToAttach, VolumeVO vmExistingVolume, UserVmVO vm) {
Review Comment:
Parameter `vm` is unused here
##
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##
@@ -2738,21 +2746,42 @@ private Volume orchestrateAttachVolumeToVM(Long vmId,
Long volumeId, Long device
logger.trace(String.format("is it needed to move the volume: %b?",
moveVolumeNeeded));
}
-if (moveVolumeNeeded) {
+// Check if CLVM lock transfer is needed (even if moveVolumeNeeded is
false)
+// This handles the case where the volume is already on the correct
storage pool
+// but the VM is running on a different host, requiring only a lock
transfer
+boolean isClvmLockTransferNeeded = !moveVolumeNeeded &&
+isClvmLockTransferRequired(newVolumeOnPrimaryStorage,
existingVolumeOfVm, vm);
+
+if (isClvmLockTransferNeeded) {
+// CLVM lock transfer - no data copy, no pool change needed
+newVolumeOnPrimaryStorage = executeLightweightLockMigration(
Re: [PR] CLVM enhancements and fixes [cloudstack]
winterhazel commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3383434984
##
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##
@@ -2748,6 +2777,204 @@ private Volume orchestrateAttachVolumeToVM(Long vmId,
Long volumeId, Long device
return newVol;
}
+/**
+ * Helper method to get storage pools for volume and VM.
+ *
+ * @param volumeToAttach The volume being attached
+ * @param vmExistingVolume The VM's existing volume
+ * @return Pair of StoragePoolVO objects (volumePool, vmPool), or null if
either pool is missing
+ */
+private Pair
getStoragePoolsForVolumeAttachment(VolumeInfo volumeToAttach, VolumeVO
vmExistingVolume) {
+if (volumeToAttach == null || vmExistingVolume == null) {
+return null;
+}
+
+StoragePoolVO volumePool =
_storagePoolDao.findById(volumeToAttach.getPoolId());
+StoragePoolVO vmPool =
_storagePoolDao.findById(vmExistingVolume.getPoolId());
+
+if (volumePool == null || vmPool == null) {
+return null;
+}
+
+return new Pair<>(volumePool, vmPool);
+}
+
+/**
+ * Checks if both storage pools are CLVM type (CLVM or CLVM_NG).
+ * Delegates to VolumeService for the actual check.
+ *
+ * @param volumePool Storage pool for the volume
+ * @param vmPool Storage pool for the VM
+ * @return true if both pools are CLVM type (CLVM or CLVM_NG)
+ */
+private boolean areBothPoolsClvmType(StoragePoolVO volumePool,
StoragePoolVO vmPool) {
+return volService.areBothPoolsClvmType(volumePool.getPoolType(),
vmPool.getPoolType());
+}
+
+/**
+ * Determines if a CLVM volume needs lightweight lock migration instead of
full data copy.
+ *
+ * Lightweight migration is needed when:
+ * 1. Volume is on CLVM storage
+ * 2. Source and destination are in the same Volume Group
+ * 3. Only the host/lock needs to change (not the storage pool)
+ *
+ * @param volumeToAttach The volume being attached
+ * @param vmExistingVolume The VM's existing volume (typically root volume)
+ * @param vm The VM to attach the volume to
+ * @return true if lightweight CLVM lock migration should be used
+ */
+private boolean isClvmLightweightMigrationNeeded(VolumeInfo
volumeToAttach, VolumeVO vmExistingVolume, UserVmVO vm) {
+Pair pools =
getStoragePoolsForVolumeAttachment(volumeToAttach, vmExistingVolume);
+if (pools == null) {
+return false;
+}
+
+StoragePoolVO volumePool = pools.first();
+StoragePoolVO vmPool = pools.second();
+
+return
volService.isLightweightMigrationNeeded(volumePool.getPoolType(),
vmPool.getPoolType(),
+volumePool.getPath(), vmPool.getPath());
+}
+
+/**
+ * Determines if a CLVM volume requires lock transfer when already on the
correct storage pool.
+ *
+ * Lock transfer is needed when:
+ * 1. Volume is already on the same CLVM storage pool as VM's volumes
+ * 2. But the volume lock is held by a different host than where the VM is
running
+ * 3. Only the lock needs to change (no pool change, no data copy)
+ *
+ * @param volumeToAttach The volume being attached
+ * @param vmExistingVolume The VM's existing volume (typically root volume)
+ * @param vm The VM to attach the volume to
+ * @return true if CLVM lock transfer is needed (but not full migration)
+ */
+private boolean isClvmLockTransferRequired(VolumeInfo volumeToAttach,
VolumeVO vmExistingVolume, UserVmVO vm) {
+if (vm == null) {
+return false;
+}
+
+Pair pools =
getStoragePoolsForVolumeAttachment(volumeToAttach, vmExistingVolume);
+if (pools == null) {
+return false;
+}
+
+StoragePoolVO volumePool = pools.first();
+StoragePoolVO vmPool = pools.second();
+
+Long vmHostId = vm.getHostId();
+if (vmHostId == null) {
+vmHostId = vm.getLastHostId();
+}
+
+return volService.isLockTransferRequired(volumeToAttach,
volumePool.getPoolType(), vmPool.getPoolType(),
+volumePool.getId(), vmPool.getId(), vmHostId);
+}
+
+/**
+ * Determines the destination host for CLVM lock migration.
+ *
+ * If VM is running, uses the VM's current host.
+ * If VM is stopped, picks an available UP host from the storage pool's
cluster.
+ *
+ * @param vm The VM
+ * @param vmExistingVolume The VM's existing volume (to determine cluster)
+ * @return Host ID, or null if cannot be determined
+ */
+private Long determineClvmLockDestinationHost(UserVmVO vm, VolumeVO
vmExistingVolume) {
+Long destHostId = vm.getHostId();
+if (destHostId != null) {
+return destHostId;
+}
+
+if (vmExistingVolume != null && vmExisting
Re: [PR] CLVM enhancements and fixes [cloudstack]
JoaoJandre commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3383432189
##
engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java:
##
@@ -2132,6 +2157,23 @@ public void copyAsync(Map
volumeDataStoreMap, VirtualMach
throw new AgentUnavailableException("Operation timed out",
destHost.getId());
}
+for (VolumeInfo vol : samePoolClvmVolumes) {
Review Comment:
I see, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4663328168 @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4663311779 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on code in PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3382965170 ## engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java: ## @@ -144,12 +144,16 @@ protected boolean isDestinationNfsPrimaryStorageClusterWide(Map
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4662570788 > I was only able to review half the files. I will do the rest later. Also, could you point to a documentation somewhere on how to setup a CLVM_NG storage to add it to ACS? I was not able to find it online It can be found at: https://github.com/apache/cloudstack-documentation/pull/637 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3382812573
##
engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java:
##
@@ -2211,19 +2253,56 @@ public void copyAsync(Map
volumeDataStoreMap, VirtualMach
}
}
+private void prepareDisksForMigrationForClvm(VirtualMachineTO vmTO,
Map volumeDataStoreMap, Host srcHost) {
+// For CLVM/CLVM_NG source pools, convert volumes from exclusive to
shared mode
+// on the source host BEFORE PrepareForMigrationCommand on the
destination.
+boolean hasClvmSource = volumeDataStoreMap.keySet().stream()
+.map(v -> _storagePoolDao.findById(v.getPoolId()))
+.anyMatch(p -> p != null && (p.getPoolType() ==
StoragePoolType.CLVM || p.getPoolType() == StoragePoolType.CLVM_NG));
+if (hasClvmSource && srcHost.getHypervisorType() ==
HypervisorType.KVM) {
+logger.info("CLVM/CLVM_NG source pool detected for VM [{}],
sending PreMigrationCommand to source host [{}] to convert volumes to shared
mode.", vmTO.getName(), srcHost.getId());
+PreMigrationCommand preMigCmd = new PreMigrationCommand(vmTO,
vmTO.getName());
+try {
+Answer preMigAnswer = agentManager.send(srcHost.getId(),
preMigCmd);
+if (preMigAnswer == null || !preMigAnswer.getResult()) {
+String details = preMigAnswer != null ?
preMigAnswer.getDetails() : "null answer returned";
+logger.warn("PreMigrationCommand failed for CLVM/CLVM_NG
VM [{}] on source host [{}]: {}. Migration will continue but may fail if
volumes are exclusively locked.", vmTO.getName(), srcHost.getId(), details);
Review Comment:
No - if it fails, it will behave like any VM migration failure. The source
VM / volume goes back to original state.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3382805679
##
engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java:
##
@@ -2132,6 +2157,23 @@ public void copyAsync(Map
volumeDataStoreMap, VirtualMach
throw new AgentUnavailableException("Operation timed out",
destHost.getId());
}
+for (VolumeInfo vol : samePoolClvmVolumes) {
Review Comment:
at L2061 - that check is present, which is the one that updates 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3382794955
##
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##
@@ -6441,6 +6507,32 @@ private Pair
findClusterAndHostIdForVm(VirtualMachine vm) {
return findClusterAndHostIdForVm(vm, false);
}
+private boolean hasClvmVolumes(long vmId) {
+List volumes = _volsDao.findByInstance(vmId);
+return volumes.stream()
+.map(v -> _storagePoolDao.findById(v.getPoolId()))
+.anyMatch(pool -> pool != null &&
ClvmPoolManager.isClvmPoolType(pool.getPoolType()));
+}
+
+private void executePreMigrationCommand(VirtualMachineTO to, String
vmInstanceName, long srcHostId) {
+logger.info("Sending PreMigrationCommand to source host {} for VM {}
with CLVM volumes", srcHostId, vmInstanceName);
+final PreMigrationCommand preMigCmd = new PreMigrationCommand(to,
vmInstanceName);
+Answer preMigAnswer = null;
+try {
+preMigAnswer = _agentMgr.send(srcHostId, preMigCmd);
+if (preMigAnswer == null || !preMigAnswer.getResult()) {
+final String details = preMigAnswer != null ?
preMigAnswer.getDetails() : "null answer returned";
Review Comment:
next retry will proceed .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3382789400
##
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##
@@ -3320,6 +3334,29 @@ protected void migrate(final VMInstanceVO vm, final long
srcHostId, final Deploy
}
}
+private void executePostMigrationCommand(VMInstanceVO vm, VirtualMachineTO
to, long dstHostId) {
+if (vm.getHypervisorType() == HypervisorType.KVM &&
hasClvmVolumes(vm.getId())) {
+try {
+logger.info("Executing post-migration tasks for VM {} with
CLVM volumes on destination host {}", vm.getInstanceName(), dstHostId);
+final PostMigrationCommand postMigrationCommand = new
PostMigrationCommand(to, vm.getInstanceName());
+final Answer postMigrationAnswer = _agentMgr.send(dstHostId,
postMigrationCommand);
+
+if (postMigrationAnswer == null ||
!postMigrationAnswer.getResult()) {
+final String details = postMigrationAnswer != null ?
postMigrationAnswer.getDetails() : "null answer returned";
+logger.warn("Post-migration tasks failed for VM {} on
destination host {}: {}. Migration completed but some cleanup may be needed.",
+vm.getInstanceName(), dstHostId, details);
+} else {
+logger.info("Successfully completed post-migration tasks
for VM {} on destination host {}", vm.getInstanceName(), dstHostId);
+}
+} catch (Exception e) {
+logger.warn("Exception during post-migration tasks for VM {}
on destination host {}: {}. Migration completed but some cleanup may be
needed.",
+vm.getInstanceName(), dstHostId, e.getMessage(), e);
+}
+}
+
+updateClvmLockHostForVmVolumes(vm.getId(), dstHostId);
Review Comment:
if it does, in the next re-try it will get corrected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
JoaoJandre commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3382315110
##
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##
@@ -3150,6 +3158,11 @@ protected void migrate(final VMInstanceVO vm, final long
srcHostId, final Deploy
updateOverCommitRatioForVmProfile(profile,
dest.getHost().getClusterId());
final VirtualMachineTO to = toVmTO(profile);
+
+if (vm.getHypervisorType() == HypervisorType.KVM &&
hasClvmVolumes(vm.getId())) {
+executePreMigrationCommand(to, vm.getInstanceName(), srcHostId);
+}
Review Comment:
I think this check could be inside the executePreMigrationCommand, like it
is done for the executePostMigrationCommand method.
##
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##
@@ -3320,6 +3334,29 @@ protected void migrate(final VMInstanceVO vm, final long
srcHostId, final Deploy
}
}
+private void executePostMigrationCommand(VMInstanceVO vm, VirtualMachineTO
to, long dstHostId) {
+if (vm.getHypervisorType() == HypervisorType.KVM &&
hasClvmVolumes(vm.getId())) {
Review Comment:
We could invert the logic and return early, reducing indentation.
##
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##
@@ -3320,6 +3334,29 @@ protected void migrate(final VMInstanceVO vm, final long
srcHostId, final Deploy
}
}
+private void executePostMigrationCommand(VMInstanceVO vm, VirtualMachineTO
to, long dstHostId) {
+if (vm.getHypervisorType() == HypervisorType.KVM &&
hasClvmVolumes(vm.getId())) {
+try {
+logger.info("Executing post-migration tasks for VM {} with
CLVM volumes on destination host {}", vm.getInstanceName(), dstHostId);
Review Comment:
Could we log the host uuid instead?
##
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##
@@ -6441,6 +6507,32 @@ private Pair
findClusterAndHostIdForVm(VirtualMachine vm) {
return findClusterAndHostIdForVm(vm, false);
}
+private boolean hasClvmVolumes(long vmId) {
+List volumes = _volsDao.findByInstance(vmId);
+return volumes.stream()
+.map(v -> _storagePoolDao.findById(v.getPoolId()))
+.anyMatch(pool -> pool != null &&
ClvmPoolManager.isClvmPoolType(pool.getPoolType()));
+}
+
+private void executePreMigrationCommand(VirtualMachineTO to, String
vmInstanceName, long srcHostId) {
+logger.info("Sending PreMigrationCommand to source host {} for VM {}
with CLVM volumes", srcHostId, vmInstanceName);
Review Comment:
same about host uuid
##
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java:
##
@@ -788,6 +805,121 @@ private String getVolumeIdentificationInfos(Volume
volume) {
return String.format("uuid: %s, name: %s", volume.getUuid(),
volume.getName());
}
+/**
+ * Updates the CLVM_LOCK_HOST_ID for a migrated volume if applicable.
+ * For CLVM volumes that are attached to a VM, this updates the lock host
tracking
+ * to point to the VM's current host after volume migration.
+ *
+ * @param migratedVolume The volume that was migrated
+ * @param destPool The destination storage pool
+ * @param operationType Description of the operation (e.g., "migrated",
"live-migrated") for logging
+ */
+private void updateClvmLockHostAfterMigration(Volume migratedVolume,
StoragePool destPool, String operationType) {
+if (migratedVolume == null || destPool == null) {
+return;
+}
+
+StoragePoolVO pool = _storagePoolDao.findById(destPool.getId());
+if (pool == null ||
!ClvmPoolManager.isClvmPoolType(pool.getPoolType())) {
+return;
+}
+
+if (migratedVolume.getInstanceId() == null) {
+return;
+}
+
+VMInstanceVO vm =
vmInstanceDao.findById(migratedVolume.getInstanceId());
+if (vm == null || vm.getHostId() == null) {
+return;
+}
+
+clvmPoolManager.setClvmLockHostId(migratedVolume.getId(),
vm.getHostId());
+logger.debug("Updated CLVM_LOCK_HOST_ID for {} volume {} to host {}
where VM {} is running",
+operationType, migratedVolume.getUuid(), vm.getHostId(),
vm.getInstanceName());
+}
+
+/**
+ * Retrieves the CLVM lock host ID from any existing volume of the
specified VM.
+ * This is useful when attaching a new volume to a stopped VM - we want to
maintain
+ * consistency by using the same host that manages the VM's other CLVM
volumes.
+ *
+ * @param vmId The ID of the VM
+ * @return The host ID if found, null otherwise
+ */
+private Long getClvmLockHostFromVmVolumes(Long vmId) {
+if
Re: [PR] CLVM enhancements and fixes [cloudstack]
sonarqubecloud[bot] commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4654276252 ## [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) **Quality Gate failed** Failed conditions  [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=12617&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [34.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=12617&metric=new_coverage&view=list) (required ≥ 40%) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4654211612 @Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4654202221 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4654189643 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18198 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4653908265 @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4653893205 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3376567340
##
engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java:
##
@@ -493,6 +645,31 @@ public EndPoint select(DataObject object, StorageAction
action, boolean encrypti
}
case DELETEVOLUME: {
VolumeInfo volume = (VolumeInfo) object;
+
+// For CLVM volumes, route to the host holding the exclusive
lock
+if (volume.getHypervisorType() ==
Hypervisor.HypervisorType.KVM) {
+DataStore store = volume.getDataStore();
+if (store.getRole() == DataStoreRole.Primary) {
+StoragePoolVO pool =
_storagePoolDao.findById(store.getId());
+if (pool != null &&
ClvmPoolManager.isClvmPoolType(pool.getPoolType())) {
+Long lockHostId = getClvmLockHostId(volume);
+if (lockHostId != null) {
+logger.info("Routing CLVM volume {} deletion
to lock holder host {}",
+volume.getUuid(), lockHostId);
+EndPoint ep =
getEndPointFromHostId(lockHostId);
+if (ep != null) {
+return ep;
+}
+logger.warn("Could not get endpoint for CLVM
lock host {}, falling back to default selection",
+lockHostId);
+} else {
+logger.debug("No CLVM lock host tracked for
volume {}, using default endpoint selection",
+volume.getUuid());
+}
+}
+}
+}
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
winterhazel commented on code in PR #12617:
URL: https://github.com/apache/cloudstack/pull/12617#discussion_r3375913541
##
engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java:
##
@@ -493,6 +645,31 @@ public EndPoint select(DataObject object, StorageAction
action, boolean encrypti
}
case DELETEVOLUME: {
VolumeInfo volume = (VolumeInfo) object;
+
+// For CLVM volumes, route to the host holding the exclusive
lock
+if (volume.getHypervisorType() ==
Hypervisor.HypervisorType.KVM) {
+DataStore store = volume.getDataStore();
+if (store.getRole() == DataStoreRole.Primary) {
+StoragePoolVO pool =
_storagePoolDao.findById(store.getId());
+if (pool != null &&
ClvmPoolManager.isClvmPoolType(pool.getPoolType())) {
+Long lockHostId = getClvmLockHostId(volume);
+if (lockHostId != null) {
+logger.info("Routing CLVM volume {} deletion
to lock holder host {}",
+volume.getUuid(), lockHostId);
+EndPoint ep =
getEndPointFromHostId(lockHostId);
+if (ep != null) {
+return ep;
+}
+logger.warn("Could not get endpoint for CLVM
lock host {}, falling back to default selection",
+lockHostId);
+} else {
+logger.debug("No CLVM lock host tracked for
volume {}, using default endpoint selection",
+volume.getUuid());
+}
+}
+}
+}
Review Comment:
A similar logic is executed many times throughout this file. We could use a
single method instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4652659318 @Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4652643360 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4652524478 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18195 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
sonarqubecloud[bot] commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4652516607 ## [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) **Quality Gate failed** Failed conditions  [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=12617&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [33.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=12617&metric=new_coverage&view=list) (required ≥ 40%) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4652053593 @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4652038881 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
github-actions[bot] commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4646240239 This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4637607129 [SF] Trillian test result (tid-16254) Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8 Total time taken: 49433 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12617-t16254-kvm-ol8.zip Smoke tests completed. 151 look OK, 0 have errors, 0 did not run Only failed and skipped 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4632986204 @Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4632980499 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
sonarqubecloud[bot] commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4632967182 ## [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) **Quality Gate failed** Failed conditions  [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=12617&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [33.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=12617&metric=new_coverage&view=list) (required ≥ 40%) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4632908100 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18165 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4632533444 @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
Pearl1594 commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4632521069 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4597095671 [SF] Trillian test result (tid-16231) Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8 Total time taken: 51829 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12617-t16231-kvm-ol8.zip Smoke tests completed. 151 look OK, 0 have errors, 0 did not run Only failed and skipped 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4590449504 @RosiKyu a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
RosiKyu commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4590438570 @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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
sonarqubecloud[bot] commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4564598786 ## [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) **Quality Gate failed** Failed conditions  [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=12617&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [32.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=12617&metric=new_coverage&view=list) (required ≥ 40%) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12617) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4564564323 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18079 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] CLVM enhancements and fixes [cloudstack]
blueorangutan commented on PR #12617: URL: https://github.com/apache/cloudstack/pull/12617#issuecomment-4564214415 @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
