You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/02/19 08:08:51 UTC

[GitHub] [cloudstack] ustcweizhou opened a new pull request #4378: server: Optional destination host when migrate a vm

ustcweizhou opened a new pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378


   ## Description
   <!--- Describe your changes in detail -->
   
   For now, destination host is required when migrate a vm.
   Make it optional and let cloudstack to choose an destination host.
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   ## Types of changes
   <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [ ] Bug fix (non-breaking change which fixes an issue)
   - [X] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   ## Screenshots (if appropriate):
   
   ## How Has This Been Tested?
   <!-- Please describe in detail how you tested your changes. -->
   <!-- Include details of your testing environment, and the tests you ran to -->
   <!-- see how your change affects other areas of the code, etc. -->
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-884407075


   @blueorangutan test centos7 vmware65


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-884495340


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + vmware-65u2) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-734753038


   @blueorangutan test matrix


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r655318680



##########
File path: ui/src/views/compute/MigrateWizard.vue
##########
@@ -168,6 +170,12 @@ export default {
         this.hosts.sort((a, b) => {
           return b.suitableformigration - a.suitableformigration
         })
+        for (const key in this.hosts) {
+          if (this.hosts[key].suitableformigration && !this.hosts[key].requiresstoragemigration) {
+            this.hosts.unshift({ id: -1, name: 'autoselect', suitableformigration: true, requiresstoragemigration: false })

Review comment:
       @shwstppr done.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-889051399


   <b>Trillian test result (tid-1396)</b>
   Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 55002 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1396-xenserver-71.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 82 look OK, 7 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_vpc_privategw_acl | `Failure` | 4.64 | test_privategw_acl.py
   test_02_vpc_privategw_static_routes | `Failure` | 4.60 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 5.67 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 7.75 | test_privategw_acl.py
   test_02_isolate_network_FW_PF_default_routes_egress_false | `Failure` | 118.13 | test_routers_network_ops.py
   test_01_create_vm_snapshots | `Failure` | 600.42 | test_vm_snapshots.py
   test_02_revert_vm_snapshots | `Failure` | 600.44 | test_vm_snapshots.py
   test_03_delete_vm_snapshots | `Failure` | 0.01 | test_vm_snapshots.py
   test_01_create_volume | `Failure` | 191.04 | test_volumes.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 644.07 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Failure` | 1029.30 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 529.99 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 671.81 | test_vpc_redundant.py
   test_02_VPC_default_routes | `Failure` | 1343.03 | test_vpc_router_nics.py
   test_01_vpc_site2site_vpn | `Failure` | 46.23 | test_vpc_vpn.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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-868551178


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-871127720


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-890250136


   <b>Trillian test result (tid-1419)</b>
   Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 116451 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1419-xenserver-71.zip
   Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_iso.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_iso.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py
   Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_nic_adapter_type.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_primary_storage.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_templates.py
   Intermittent failure detected: /marvin/tests/smoke/test_usage.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
   Smoke tests completed. 54 look OK, 35 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestInternalLb>:setup | `Error` | 0.00 | test_internal_lb.py
   ContextSuite context=TestAccounts>:setup | `Error` | 0.00 | test_accounts.py
   ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 0.00 | test_accounts.py
   test_DeleteDomain | `Error` | 9.02 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 5.22 | test_accounts.py
   ContextSuite context=TestRemoveUserFromAccount>:setup | `Error` | 5.29 | test_accounts.py
   ContextSuite context=TestDomainsServiceOfferings>:setup | `Error` | 1545.14 | test_domain_service_offerings.py
   ContextSuite context=TestDeployVmWithAffinityGroup>:setup | `Error` | 0.00 | test_affinity_groups_projects.py
   ContextSuite context=TestAsyncJob>:setup | `Error` | 0.00 | test_async_job.py
   test_01_create_iso_with_checksum_sha1 | `Error` | 66.38 | test_iso.py
   test_02_create_iso_with_checksum_sha256 | `Error` | 66.37 | test_iso.py
   test_03_create_iso_with_checksum_md5 | `Error` | 66.38 | test_iso.py
   test_04_create_iso_with_no_checksum | `Error` | 66.37 | test_iso.py
   test_01_create_iso | `Failure` | 1557.13 | test_iso.py
   ContextSuite context=TestISO>:setup | `Error` | 3085.47 | test_iso.py
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   ContextSuite context=TestDeployVMFromISO>:setup | `Error` | 0.00 | test_deploy_vm_iso.py
   test_list_clusters_metrics | `Error` | 1562.94 | test_metrics_api.py
   test_list_vms_metrics | `Error` | 0.14 | test_metrics_api.py
   ContextSuite context=TestDeployVmWithVariedPlanners>:setup | `Error` | 0.00 | test_deploy_vms_with_varied_deploymentplanners.py
   ContextSuite context=TestDeployVmWithUserData>:setup | `Error` | 0.00 | test_deploy_vm_with_userdata.py
   ContextSuite context=TestRemoteDiagnostics>:setup | `Error` | 0.00 | test_diagnostics.py
   ContextSuite context=TestNetworkACL>:setup | `Error` | 0.00 | test_network_acl.py
   test_delete_account | `Error` | 1519.74 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 1.10 | test_network.py
   test_deploy_vm_l2network | `Error` | 1.10 | test_network.py
   test_l2network_restart | `Error` | 4.37 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 5.49 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 1.07 | test_network.py
   test_reboot_router | `Failure` | 0.07 | test_network.py
   test_releaseIP | `Error` | 0.46 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 0.64 | test_network.py
   test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_02_deploy_and_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_basic_lifecycle_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_06_deploy_invalid_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_01_add_delete_kubernetes_supported_version | `Error` | 1805.65 | test_kubernetes_supported_versions.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 1519.74 | test_multipleips_per_nic.py
   ContextSuite context=TestNestedVirtualization>:setup | `Error` | 0.00 | test_nested_virtualization.py
   ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py
   ContextSuite context=TestPortForwardingRules>:setup | `Error` | 0.00 | test_portforwardingrules.py
   ContextSuite context=TestRVPCSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVPCSite2SiteVPNMultipleOptions>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcRemoteAccessVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestPrivateGwACL>:setup | `Error` | 0.00 | test_privategw_acl.py
   test_09_project_suspend | `Error` | 1.08 | test_projects.py
   test_10_project_activation | `Error` | 1.06 | test_projects.py
   ContextSuite context=TestVPCNics>:setup | `Error` | 0.00 | test_vpc_router_nics.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
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 387.61 | test_routers_network_ops.py
   test_02_create_template_with_checksum_sha1 | `Error` | 65.61 | test_templates.py
   test_03_create_template_with_checksum_sha256 | `Error` | 65.38 | test_templates.py
   test_04_create_template_with_checksum_md5 | `Error` | 65.39 | test_templates.py
   test_05_create_template_with_no_checksum | `Error` | 65.41 | test_templates.py
   test_04_extract_template | `Failure` | 128.30 | test_templates.py
   ContextSuite context=TestISOUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestLBRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   test_01_volume_usage | `Failure` | 688.66 | test_usage.py
   ContextSuite context=TestDeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   test_10_attachAndDetach_iso | `Failure` | 1559.57 | test_vm_life_cycle.py
   ContextSuite context=TestChangeServiceOfferingForVmWithSnapshots>:setup | `Error` | 0.00 | test_vm_snapshots.py
   ContextSuite context=TestCreateVolume>:setup | `Error` | 0.00 | test_volumes.py
   test_06_download_detached_volume | `Failure` | 320.85 | test_volumes.py
   ContextSuite context=TestVPCRedundancy>:setup | `Error` | 0.00 | test_vpc_redundant.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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595754539



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVMCmd.java
##########
@@ -169,9 +165,9 @@ public void execute() {
 
         try {
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
+            if (getStoragePoolId() == null) {

Review comment:
       @ustcweizhou whenever host / storage pool is not specified in the cmd, instead of directly picking a suitable dest host to migrate to, I think it is better to confirm it (from the user) through a API cmd param (something like _chooseHost_  / _selectHost_). If user is not OK to auto select host, then throw appropriate message.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-836279756


   > Tested on VMware (single cluster, 2x ESXi hosts), **worked fine for both user and systemvms**.
   > 
   > Suggestion for small UI improvement to not show `NaN GB`:
   > 
   > ```
   > --- a/ui/src/views/compute/MigrateWizard.vue
   > +++ b/ui/src/views/compute/MigrateWizard.vue
   > @@ -45,7 +45,9 @@
   >            v-else />
   >        </div>
   >        <div slot="memused" slot-scope="record">
   > -        {{ record.memoryused | byteToGigabyte }} GB
   > +        <span v-if="record.memoryused | byteToGigabyte">
   > +          {{ record.memoryused | byteToGigabyte }} GB
   > +        </span>
   >        </div>
   >        <div slot="memoryallocatedpercentage" slot-scope="record">
   >          {{ record.memoryallocatedpercentage }}
   > ```
   > 
   > Also, in UI should we hide/disable autoselect when there are no suitable hosts found or if all hosts require storage migration? @weizhouapache
   
   @shwstppr cool, thanks. good ideas !


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-704204634


   @weizhouapache a little functional nit; in a bigger env where several admins are working one might want to disable this behaviour so two admins trying to empty a host each don't cross each other. Agree?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-718061196


   @ustcweizhou can you address the conflict?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-868580680


   @blueorangutan test matrix


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-800286788


   @blueorangutan package


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-868551865


   @sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-871650991


   <b>Trillian test result (tid-1173)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 40952 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1173-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 87 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestVPCRedundancy>:setup | `Error` | 0.00 | test_vpc_redundant.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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-884494719


   @blueorangutan test centos7 vmware-65u2


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-888557647


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-734986807


   <b>Trillian test result (tid-3280)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 36832 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t3280-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 85 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 290.90 | test_kubernetes_clusters.py
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595830908



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java
##########
@@ -122,34 +122,34 @@ public String getEventDescription() {
 
     @Override
     public void execute() {
-        if (getHostId() == null && getStorageId() == null) {
-            throw new InvalidParameterValueException("Either hostId or storageId must be specified");
-        }
-
         if (getHostId() != null && getStorageId() != null) {
             throw new InvalidParameterValueException("Only one of hostId and storageId can be specified");
         }
+
         try {
             //FIXME : Should not be calling UserVmService to migrate all types of VMs - need a generic VM layer
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
-                Host destinationHost = _resourceService.getHost(getHostId());
-                if (destinationHost == null) {
-                    throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
-                }
-                if (destinationHost.getType() != Host.Type.Routing) {
-                    throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
-                }
-                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
-                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());
-            } else if (getStorageId() != null) {
+            if (getStorageId() != null) {
                 // OfflineMigration performed when this parameter is specified
                 StoragePool destStoragePool = _storageService.getStoragePool(getStorageId());
                 if (destStoragePool == null) {
                     throw new InvalidParameterValueException("Unable to find the storage pool to migrate the VM");
                 }
                 CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to storage pool Id: " + getStorageId());
                 migratedVm = _userVmService.vmStorageMigration(getVirtualMachineId(), destStoragePool);
+            } else {
+                Host destinationHost = null;
+                if (getHostId() != null) {
+                    destinationHost =_resourceService.getHost(getHostId());
+                    if (destinationHost == null) {
+                        throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
+                    }
+                    if (destinationHost.getType() != Host.Type.Routing) {
+                        throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
+                    }
+                }
+                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
+                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());

Review comment:
       @shwstppr I have a question to you, since you are the author of #4385.
   why we use different method in vm migration and systemvm migration ?
   
   MigrateSystemVMCmd.java: migrateVirtualMachineWithVolume and vmStorageMigration
   MigrateVMCmd.java: migrateVirtualMachine and vmStorageMigration
   
   can they use same methods ?
    
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-884382360


   <b>Trillian test result (tid-1334)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 34992 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1334-kvm-centos7.zip
   Smoke tests completed. 89 look OK, 0 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-871114830


   <b>Trillian test result (tid-1164)</b>
   Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 38596 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1164-xenserver-71.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Smoke tests completed. 88 look OK, 0 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-870797761


   @blueorangutan test matrix


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-800129730


   @blueorangutan package


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-733606315


   @blueorangutan package 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595131183



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java
##########
@@ -126,30 +126,30 @@ public void execute() {
             throw new InvalidParameterValueException("Either hostId or storageId must be specified");

Review comment:
       @weizhouapache do you need to remove this condition else either hostid or storageid must be provided




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-873010203


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 459


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-872994778


   > @weizhouapache between global settings configuration or the AUTO_SELECT I would prefer the last one. I think that keeping it in the API would be easier for users to find this out due to the API documentation.
   
   @GabrielBrascher @sureshanaparti 
   we have 3 options
   (1) autoselect is 'true' by default
   (2) autoselect is 'false' by default (current behavior)
   (3) a global setting. if autoselect is not set in api request, then honor the global setting.
   
   what's your opinion ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-885910652


   <b>Trillian test result (tid-1360)</b>
   Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 38545 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1360-xenserver-71.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_templates.py
   Smoke tests completed. 89 look OK, 0 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r502264412



##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -5627,6 +5629,50 @@ public VirtualMachine migrateVirtualMachine(Long vmId, Host destinationHost) thr
 
         // check if migrating to same host
         long srcHostId = vm.getHostId();
+
+        DeployDestination dest = null;
+        if (destinationHost == null) {
+            vm.setLastHostId(null); // Do not check last host
+            final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm);
+            final Host host = _hostDao.findById(srcHostId);
+            final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), null, null, null, null, null);

Review comment:
       @ustcweizhou finding a suitable host anywhere within the zone might result in returning a host in a different pod or cluster. This might also need storage migration. Will that be an issue? Especially when VM is having data disks attached. Also, cross-cluster at present fails on VMware when storage pools are cluster scoped. #4385 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-871129472


   > @ustcweizhou this is something interesting. It could also help CloudStack choosing the best options than we (humans), as CloudStack could then migrate VMs balancing the cluster.
   > 
   > What do you think of making it optional without an `AUTO_SELECT`?
   > 
   > In such a case, Host & Storage would be optional and if both host & storage are null then it would proceed to the automatic destination selection. It would be similar to when deploying a VM without specifying the host.
   > 
   > In my opinion, if someone migrates a VM without choosing a destination then the person does not care about the destination.
   > 
   > This could be a problem if someone runs the command by mistake, but it is not the end of the world (the user would then be able to migrate the VM again to another host).
   
   @GabrielBrascher actually there is no AUTO_SELECT at the beginning.
   It was added per @sureshanaparti 's remarks.
   
   I can add a global setting . does it look good 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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-883960799


   @blueorangutan test matrix


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-871147540


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-888408071


   <b>Trillian test result (tid-1388)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 51540 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1388-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 85 look OK, 4 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_05_ping_in_cpvm_success | `Failure` | 4.13 | test_diagnostics.py
   test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 3622.50 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 95.70 | test_kubernetes_clusters.py
   test_04_rvpc_privategw_static_routes | `Failure` | 1134.77 | test_privategw_acl.py
   test_02_VPC_default_routes | `Failure` | 969.59 | test_vpc_router_nics.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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-876188809


   > Trillian test result (tid-1216)
   > Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   > Total time taken: 36995 seconds
   > Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1216-vmware-65u2.zip
   > Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   > Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
   > Smoke tests completed. 87 look OK, 1 have error(s)
   > Only failed tests results shown below:
   > 
   > Test	Result	Time (s)	Test File
   > test_01_migrate_VM_and_root_volume	`Error`	107.14	test_vm_life_cycle.py
   > test_02_migrate_VM_with_two_data_disks	`Error`	85.96	test_vm_life_cycle.py
   
   these two test failures should be fixed by #5170


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-800287106


   @shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. [S]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-733606703


   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-733639870


   Packaging result: ✖centos7 ✔centos8 ✔debian. JID-2436


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-868950402


   <b>Trillian test result (tid-1106)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 62413 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1106-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_usage.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 85 look OK, 3 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 3608.05 | test_kubernetes_clusters.py
   test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 24.07 | test_kubernetes_clusters.py
   test_04_basic_lifecycle_kubernetes_cluster | `Failure` | 3614.38 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 0.06 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 44.73 | test_kubernetes_clusters.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 364.95 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 358.83 | test_routers_network_ops.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 542.84 | test_vpc_redundant.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 542.86 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 467.94 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 505.76 | test_vpc_redundant.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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-875869797


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + vmware-65u2) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache closed pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-800311084


   Packaging result: :heavy_multiplication_x: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 137


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-864948905






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-884001439


   <b>Trillian Build Failed (tid-1333)<b/>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-887906471


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 663


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-876159187


   <b>Trillian test result (tid-1216)</b>
   Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 36995 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1216-vmware-65u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
   Smoke tests completed. 87 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_migrate_VM_and_root_volume | `Error` | 107.14 | test_vm_life_cycle.py
   test_02_migrate_VM_with_two_data_disks | `Error` | 85.96 | test_vm_life_cycle.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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r678182541



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java
##########
@@ -91,6 +97,10 @@ public Long getStorageId() {
         return storageId;
     }
 
+    public Boolean isAutoSelect() {
+        return autoSelect != null ? autoSelect : true;

Review comment:
       @nvazquez done. thanks for suggestion.
   
   use `BooleanUtils.isNotFalse`
   https://commons.apache.org/proper/commons-lang/javadocs/api-release/org/apache/commons/lang3/BooleanUtils.html#isNotFalse-java.lang.Boolean-




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-889174586


   @blueorangutan test matrix


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-886402354


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595769435



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java
##########
@@ -122,34 +122,34 @@ public String getEventDescription() {
 
     @Override
     public void execute() {
-        if (getHostId() == null && getStorageId() == null) {
-            throw new InvalidParameterValueException("Either hostId or storageId must be specified");
-        }
-
         if (getHostId() != null && getStorageId() != null) {
             throw new InvalidParameterValueException("Only one of hostId and storageId can be specified");
         }
+
         try {
             //FIXME : Should not be calling UserVmService to migrate all types of VMs - need a generic VM layer
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
-                Host destinationHost = _resourceService.getHost(getHostId());
-                if (destinationHost == null) {
-                    throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
-                }
-                if (destinationHost.getType() != Host.Type.Routing) {
-                    throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
-                }
-                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
-                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());
-            } else if (getStorageId() != null) {
+            if (getStorageId() != null) {
                 // OfflineMigration performed when this parameter is specified
                 StoragePool destStoragePool = _storageService.getStoragePool(getStorageId());
                 if (destStoragePool == null) {
                     throw new InvalidParameterValueException("Unable to find the storage pool to migrate the VM");
                 }
                 CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to storage pool Id: " + getStorageId());
                 migratedVm = _userVmService.vmStorageMigration(getVirtualMachineId(), destStoragePool);
+            } else {
+                Host destinationHost = null;
+                if (getHostId() != null) {
+                    destinationHost =_resourceService.getHost(getHostId());
+                    if (destinationHost == null) {
+                        throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
+                    }
+                    if (destinationHost.getType() != Host.Type.Routing) {
+                        throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
+                    }
+                }
+                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
+                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());

Review comment:
       @weizhouapache change added in UserVmManagerImpl to find suitable host is only for `migrateVirtualMachine` method while we call `migrateVirtualMachineWithVolume`
   Currently we will get NPE onhttps://github.com/apache/cloudstack/blob/f5aed32ebddef19b9b2f1bed724bf0b16c69f0b6/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java#L6289




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-783805651


   <b>Trillian test result (tid-3592)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 33516 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t3592-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_iso.py
   Smoke tests completed. 85 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_04_extract_Iso | `Failure` | 128.42 | test_iso.py
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-883961345


   @weizhouapache a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-888558064


   @weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-865639752


   @shwstppr a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595754539



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVMCmd.java
##########
@@ -169,9 +165,9 @@ public void execute() {
 
         try {
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
+            if (getStoragePoolId() == null) {

Review comment:
       @ustcweizhou whenever host / storage pool is not specified in the cmd, instead of directly picking a suitable dest host to migrate to, I think it is better to confirm it (from the user) through a API cmd param (something like _chooseHost_  / _selectHost_).




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-783473246


   @shwstppr a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-868973909


   <b>Trillian test result (tid-1112)</b>
   Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 61368 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1112-xenserver-71.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 84 look OK, 4 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_04_rvpc_privategw_static_routes | `Failure` | 1230.82 | test_privategw_acl.py
   test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 3618.01 | test_kubernetes_clusters.py
   test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 3613.80 | test_kubernetes_clusters.py
   test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   test_04_basic_lifecycle_kubernetes_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 51.13 | test_kubernetes_clusters.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 378.32 | test_routers_network_ops.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 617.78 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 655.97 | test_vpc_redundant.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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-864952180


   @weizhouapache a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-800827807


   Packaging result: :heavy_multiplication_x: centos7 :heavy_multiplication_x: centos8 :heavy_check_mark: debian. SL-JID 144


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-767544294


   > @ustcweizhou needs changes in new UI
   
   @shwstppr rebased with master.
   fixed conflicts.
   made primate ui change, and
   fixed a bug found in testing.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] GabrielBrascher commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-871721320


   @weizhouapache between global settings configuration or the AUTO_SELECT I would prefer the last one. I think that keeping it in the API would be easier for users to find this out due to the API documentation.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-887899266


   @nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-895351108


   @nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-888326527


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-868582985


   @weizhouapache a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-887898964


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-888571147


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 674


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595809154



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java
##########
@@ -122,34 +122,34 @@ public String getEventDescription() {
 
     @Override
     public void execute() {
-        if (getHostId() == null && getStorageId() == null) {
-            throw new InvalidParameterValueException("Either hostId or storageId must be specified");
-        }
-
         if (getHostId() != null && getStorageId() != null) {
             throw new InvalidParameterValueException("Only one of hostId and storageId can be specified");
         }
+
         try {
             //FIXME : Should not be calling UserVmService to migrate all types of VMs - need a generic VM layer
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
-                Host destinationHost = _resourceService.getHost(getHostId());
-                if (destinationHost == null) {
-                    throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
-                }
-                if (destinationHost.getType() != Host.Type.Routing) {
-                    throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
-                }
-                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
-                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());
-            } else if (getStorageId() != null) {
+            if (getStorageId() != null) {
                 // OfflineMigration performed when this parameter is specified
                 StoragePool destStoragePool = _storageService.getStoragePool(getStorageId());
                 if (destStoragePool == null) {
                     throw new InvalidParameterValueException("Unable to find the storage pool to migrate the VM");
                 }
                 CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to storage pool Id: " + getStorageId());
                 migratedVm = _userVmService.vmStorageMigration(getVirtualMachineId(), destStoragePool);
+            } else {
+                Host destinationHost = null;
+                if (getHostId() != null) {
+                    destinationHost =_resourceService.getHost(getHostId());
+                    if (destinationHost == null) {
+                        throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
+                    }
+                    if (destinationHost.getType() != Host.Type.Routing) {
+                        throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
+                    }
+                }
+                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
+                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());

Review comment:
       @shwstppr I tested with 4.15. did not notice that we have some changes in master.
   https://github.com/apache/cloudstack/blob/4.15/api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java#L120




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-783790767


   <b>Trillian test result (tid-3591)</b>
   Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 31169 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t3591-xenserver-71.zip
   Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_usage.py
   Smoke tests completed. 85 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_scale_vm | `Failure` | 10.26 | test_scale_vm.py
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-783411156


   Packaging result: ✔centos7 ✖centos8 ✖debian. JID-2785


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r658697689



##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -5895,8 +5897,46 @@ public VirtualMachine migrateVirtualMachine(Long vmId, Host destinationHost) thr
             throw new InvalidParameterValueException("Cannot migrate VM, host with id: " + srcHostId + " for VM not found");
         }
 
+        DeployDestination dest = null;
+        if (destinationHost == null) {
+            vm.setLastHostId(null); // Last host does not have higher priority in vm migration
+            final ServiceOfferingVO offering = _offeringDao.findById(vm.getId(), vm.getServiceOfferingId());
+            final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, null, offering, null, null);
+            final Host host = _hostDao.findById(srcHostId);
+            final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), host.getPodId(), host.getClusterId(), null, null, null);
+            ExcludeList excludes = new ExcludeList();
+            excludes.addHost(srcHostId);
+            try {
+                dest = _planningMgr.planDeployment(profile, plan, excludes, null);
+            } catch (final AffinityConflictException e2) {
+                s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2);
+                throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict");
+            } catch (final InsufficientServerCapacityException e3) {
+                throw new CloudRuntimeException("Unable to find a server to migrate the vm to");
+            }
+        } else {
+            dest = checkVmMigrationDestination(vm, srcHost, destinationHost);
+        }
 
-        if (destinationHost.getId() == srcHostId) {
+        // If no suitable destination found then throw exception
+        if (dest == null) {
+            throw new CloudRuntimeException("Unable to find suitable destination to migrate VM " + vm.getInstanceName());
+        }
+
+        UserVmVO uservm = _vmDao.findById(vmId);
+        if (uservm != null) {
+            collectVmDiskStatistics(uservm);
+            collectVmNetworkStatistics(uservm);
+        }
+        _itMgr.migrate(vm.getUuid(), srcHostId, dest);

Review comment:
       and this, please?

##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -5895,8 +5897,46 @@ public VirtualMachine migrateVirtualMachine(Long vmId, Host destinationHost) thr
             throw new InvalidParameterValueException("Cannot migrate VM, host with id: " + srcHostId + " for VM not found");
         }
 
+        DeployDestination dest = null;
+        if (destinationHost == null) {
+            vm.setLastHostId(null); // Last host does not have higher priority in vm migration
+            final ServiceOfferingVO offering = _offeringDao.findById(vm.getId(), vm.getServiceOfferingId());
+            final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, null, offering, null, null);
+            final Host host = _hostDao.findById(srcHostId);
+            final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), host.getPodId(), host.getClusterId(), null, null, null);
+            ExcludeList excludes = new ExcludeList();
+            excludes.addHost(srcHostId);
+            try {
+                dest = _planningMgr.planDeployment(profile, plan, excludes, null);
+            } catch (final AffinityConflictException e2) {
+                s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2);
+                throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict");
+            } catch (final InsufficientServerCapacityException e3) {
+                throw new CloudRuntimeException("Unable to find a server to migrate the vm to");
+            }
+        } else {
+            dest = checkVmMigrationDestination(vm, srcHost, destinationHost);
+        }

Review comment:
       can you factor this out to a separate method, please




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-868430780


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-870940074


   <b>Trillian Build Failed (tid-1165)<b/>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-734994315


   <b>Trillian test result (tid-3283)</b>
   Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 38250 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t3283-vmware-65u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Smoke tests completed. 85 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 796.51 | test_kubernetes_clusters.py
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-865639383


   @blueorangutan test matrix


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-871139108


   <b>Trillian Build Failed (tid-1172)<b/>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-864948548


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-734753555


   @DaanHoogland a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-870556507


   > Trillian test result (tid-1114)
   > Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   > Total time taken: 39823 seconds
   > Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1114-vmware-65u2.zip
   > Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   > Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   > Smoke tests completed. 87 look OK, 1 have error(s)
   > Only failed tests results shown below:
   > 
   > Test	Result	Time (s)	Test File
   > test_01_migrate_VM_and_root_volume	`Error`	106.91	test_vm_life_cycle.py
   > test_02_migrate_VM_with_two_data_disks	`Error`	63.30	test_vm_life_cycle.py
   
   @weizhouapache can you check these test failures, and retrigger the tests please ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache closed pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-868992556


   <b>Trillian test result (tid-1113)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 73059 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1113-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_login.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 84 look OK, 4 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 3602.09 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 98.92 | test_kubernetes_clusters.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 344.94 | test_routers_network_ops.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 929.82 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 530.51 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 500.09 | test_vpc_redundant.py
   test_01_redundant_vpc_site2site_vpn | `Failure` | 354.05 | test_vpc_vpn.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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r596658730



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVMCmd.java
##########
@@ -169,9 +165,9 @@ public void execute() {
 
         try {
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
+            if (getStoragePoolId() == null) {

Review comment:
       @sureshanaparti yes, I am currently working on the coding, same as you said.
   
   ps: cloudstack supports projectid=-1, so it is feasible to support hostid=-1 but it might have big impact on other apis.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache edited a comment on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache edited a comment on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-804146365


   @shwstppr @sureshanaparti added a parameter 'autoselect' to migratevm and migratesystemvm apis.
   
   ![image](https://user-images.githubusercontent.com/57355700/112272733-dad29800-8c7c-11eb-812e-7f1ab734acb9.png)
   
   it is not valid for api migratevmwithvolume


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-887926860


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-866358312


   <b>Trillian test result (tid-1032)</b>
   Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 50867 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1032-vmware-65u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_root_resize.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 85 look OK, 3 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 414.03 | test_routers_network_ops.py
   test_01_migrate_VM_and_root_volume | `Error` | 100.30 | test_vm_life_cycle.py
   test_02_migrate_VM_with_two_data_disks | `Error` | 67.56 | test_vm_life_cycle.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 667.89 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 579.44 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 576.39 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 576.40 | test_vpc_redundant.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595843031



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java
##########
@@ -122,34 +122,34 @@ public String getEventDescription() {
 
     @Override
     public void execute() {
-        if (getHostId() == null && getStorageId() == null) {
-            throw new InvalidParameterValueException("Either hostId or storageId must be specified");
-        }
-
         if (getHostId() != null && getStorageId() != null) {
             throw new InvalidParameterValueException("Only one of hostId and storageId can be specified");
         }
+
         try {
             //FIXME : Should not be calling UserVmService to migrate all types of VMs - need a generic VM layer
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
-                Host destinationHost = _resourceService.getHost(getHostId());
-                if (destinationHost == null) {
-                    throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
-                }
-                if (destinationHost.getType() != Host.Type.Routing) {
-                    throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
-                }
-                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
-                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());
-            } else if (getStorageId() != null) {
+            if (getStorageId() != null) {
                 // OfflineMigration performed when this parameter is specified
                 StoragePool destStoragePool = _storageService.getStoragePool(getStorageId());
                 if (destStoragePool == null) {
                     throw new InvalidParameterValueException("Unable to find the storage pool to migrate the VM");
                 }
                 CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to storage pool Id: " + getStorageId());
                 migratedVm = _userVmService.vmStorageMigration(getVirtualMachineId(), destStoragePool);
+            } else {
+                Host destinationHost = null;
+                if (getHostId() != null) {
+                    destinationHost =_resourceService.getHost(getHostId());
+                    if (destinationHost == null) {
+                        throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
+                    }
+                    if (destinationHost.getType() != Host.Type.Routing) {
+                        throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
+                    }
+                }
+                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
+                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());

Review comment:
       @weizhouapache For user VMs we have two different APIs, migrateVirtualMachine and migrateVirtualMachineWithVolume. Depending on the need for storage migration requirement appropriate API can be used.
   While working on #4385 choice was to add a new API migrateSystemVmWithVolume or using `migrateVirtualMachineWithVolume` method while code making the decision if volume needs to be migrated.
   
   MigrateVMCmd can use the `migrateVirtualMachineWithVolume` method but it might need some additional checks and testing.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r655297475



##########
File path: ui/src/views/compute/MigrateWizard.vue
##########
@@ -168,6 +170,12 @@ export default {
         this.hosts.sort((a, b) => {
           return b.suitableformigration - a.suitableformigration
         })
+        for (const key in this.hosts) {
+          if (this.hosts[key].suitableformigration && !this.hosts[key].requiresstoragemigration) {
+            this.hosts.unshift({ id: -1, name: 'autoselect', suitableformigration: true, requiresstoragemigration: false })

Review comment:
       @weizhouapache do we need to add this string - `autoselect` in translation?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595993441



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java
##########
@@ -122,34 +122,34 @@ public String getEventDescription() {
 
     @Override
     public void execute() {
-        if (getHostId() == null && getStorageId() == null) {
-            throw new InvalidParameterValueException("Either hostId or storageId must be specified");
-        }
-
         if (getHostId() != null && getStorageId() != null) {
             throw new InvalidParameterValueException("Only one of hostId and storageId can be specified");
         }
+
         try {
             //FIXME : Should not be calling UserVmService to migrate all types of VMs - need a generic VM layer
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
-                Host destinationHost = _resourceService.getHost(getHostId());
-                if (destinationHost == null) {
-                    throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
-                }
-                if (destinationHost.getType() != Host.Type.Routing) {
-                    throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
-                }
-                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
-                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());
-            } else if (getStorageId() != null) {
+            if (getStorageId() != null) {
                 // OfflineMigration performed when this parameter is specified
                 StoragePool destStoragePool = _storageService.getStoragePool(getStorageId());
                 if (destStoragePool == null) {
                     throw new InvalidParameterValueException("Unable to find the storage pool to migrate the VM");
                 }
                 CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to storage pool Id: " + getStorageId());
                 migratedVm = _userVmService.vmStorageMigration(getVirtualMachineId(), destStoragePool);
+            } else {
+                Host destinationHost = null;
+                if (getHostId() != null) {
+                    destinationHost =_resourceService.getHost(getHostId());
+                    if (destinationHost == null) {
+                        throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
+                    }
+                    if (destinationHost.getType() != Host.Type.Routing) {
+                        throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
+                    }
+                }
+                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
+                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());

Review comment:
       @shwstppr I was a bit confused.
   
   I will change to the following behavior: if hostid is not specified, then use migratevirtualmachine instead of migratevirtualmachinewithvolume. sounds ok ?
   
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595826330



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVMCmd.java
##########
@@ -169,9 +165,9 @@ public void execute() {
 
         try {
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
+            if (getStoragePoolId() == null) {

Review comment:
       @sureshanaparti ok.
   what do you think if we regard hostid=-1 as autoselect ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez merged pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
nvazquez merged pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-883939904


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-885523964


   @blueorangutan test matrix


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-895055655


   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595808483



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java
##########
@@ -122,34 +122,34 @@ public String getEventDescription() {
 
     @Override
     public void execute() {
-        if (getHostId() == null && getStorageId() == null) {
-            throw new InvalidParameterValueException("Either hostId or storageId must be specified");
-        }
-
         if (getHostId() != null && getStorageId() != null) {
             throw new InvalidParameterValueException("Only one of hostId and storageId can be specified");
         }
+
         try {
             //FIXME : Should not be calling UserVmService to migrate all types of VMs - need a generic VM layer
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
-                Host destinationHost = _resourceService.getHost(getHostId());
-                if (destinationHost == null) {
-                    throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
-                }
-                if (destinationHost.getType() != Host.Type.Routing) {
-                    throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
-                }
-                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
-                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());
-            } else if (getStorageId() != null) {
+            if (getStorageId() != null) {
                 // OfflineMigration performed when this parameter is specified
                 StoragePool destStoragePool = _storageService.getStoragePool(getStorageId());
                 if (destStoragePool == null) {
                     throw new InvalidParameterValueException("Unable to find the storage pool to migrate the VM");
                 }
                 CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to storage pool Id: " + getStorageId());
                 migratedVm = _userVmService.vmStorageMigration(getVirtualMachineId(), destStoragePool);
+            } else {
+                Host destinationHost = null;
+                if (getHostId() != null) {
+                    destinationHost =_resourceService.getHost(getHostId());
+                    if (destinationHost == null) {
+                        throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
+                    }
+                    if (destinationHost.getType() != Host.Type.Routing) {
+                        throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
+                    }
+                }
+                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
+                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());

Review comment:
       @shwstppr I will test it.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache closed pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-870689315


   @weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r658547304



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVMCmd.java
##########
@@ -155,6 +161,8 @@ public void execute() {
                 throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
             }
             CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
+        } else if (! isAutoSelect()) {
+            throw new InvalidParameterValueException("Please specify a host or storage as destination, or pass 'autoselect=true' to automatically select a destination host which do not require storage migration");

Review comment:
       @weizhouapache just to check, auto select here picks the suitable host for VM migration. In the similar way, any possibility to pick suitable storage ? If so, can this API have both parameters: autoselecthost & autoselectstorage (only one of them should be true).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-888836314


   <b>Trillian test result (tid-1397)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 34958 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1397-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_templates.py
   Smoke tests completed. 89 look OK, 0 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r662869903



##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -5895,8 +5897,46 @@ public VirtualMachine migrateVirtualMachine(Long vmId, Host destinationHost) thr
             throw new InvalidParameterValueException("Cannot migrate VM, host with id: " + srcHostId + " for VM not found");
         }
 
+        DeployDestination dest = null;
+        if (destinationHost == null) {
+            vm.setLastHostId(null); // Last host does not have higher priority in vm migration
+            final ServiceOfferingVO offering = _offeringDao.findById(vm.getId(), vm.getServiceOfferingId());
+            final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, null, offering, null, null);
+            final Host host = _hostDao.findById(srcHostId);
+            final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), host.getPodId(), host.getClusterId(), null, null, null);
+            ExcludeList excludes = new ExcludeList();
+            excludes.addHost(srcHostId);
+            try {
+                dest = _planningMgr.planDeployment(profile, plan, excludes, null);
+            } catch (final AffinityConflictException e2) {
+                s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2);
+                throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict");
+            } catch (final InsufficientServerCapacityException e3) {
+                throw new CloudRuntimeException("Unable to find a server to migrate the vm to");
+            }
+        } else {
+            dest = checkVmMigrationDestination(vm, srcHost, destinationHost);
+        }
 
-        if (destinationHost.getId() == srcHostId) {
+        // If no suitable destination found then throw exception
+        if (dest == null) {
+            throw new CloudRuntimeException("Unable to find suitable destination to migrate VM " + vm.getInstanceName());
+        }
+
+        UserVmVO uservm = _vmDao.findById(vmId);
+        if (uservm != null) {
+            collectVmDiskStatistics(uservm);
+            collectVmNetworkStatistics(uservm);
+        }
+        _itMgr.migrate(vm.getUuid(), srcHostId, dest);

Review comment:
       sorry, this wasn't clear and also is far less interesting but you could
   ```suggestion
       private void collectStatisticsBeforeMigration(Long vmId)
           UserVmVO uservm = _vmDao.findById(vmId);
           if (uservm != null) {
               collectVmDiskStatistics(uservm);
               collectVmNetworkStatistics(uservm);
           }
       }
   ```
   or 
   ```suggestion
       private void collectStatisticsAndMigrate(Long vmId)
       {
           UserVmVO uservm = _vmDao.findById(vmId);
           if (uservm != null) {
               collectVmDiskStatistics(uservm);
               collectVmNetworkStatistics(uservm);
           }
           _itMgr.migrate(vm.getUuid(), srcHostId, dest);
       }
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-864952004


   @blueorangutan ui


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-873397808


   <b>Trillian test result (tid-1196)</b>
   Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 77615 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1196-vmware-65u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
   Smoke tests completed. 76 look OK, 12 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.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=TestServiceOfferings>:setup | `Error` | 1511.17 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_01_migrate_VM_and_root_volume | `Error` | 111.98 | test_vm_life_cycle.py
   test_02_migrate_VM_with_two_data_disks | `Error` | 84.81 | test_vm_life_cycle.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 658.73 | test_vpc_redundant.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 658.75 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 721.88 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 613.53 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 613.55 | test_vpc_redundant.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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r502628997



##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -5627,6 +5629,50 @@ public VirtualMachine migrateVirtualMachine(Long vmId, Host destinationHost) thr
 
         // check if migrating to same host
         long srcHostId = vm.getHostId();
+
+        DeployDestination dest = null;
+        if (destinationHost == null) {
+            vm.setLastHostId(null); // Do not check last host
+            final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm);
+            final Host host = _hostDao.findById(srcHostId);
+            final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), null, null, null, null, null);

Review comment:
       @shwstppr you are right. 
   I will refactor it.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-885077062


   <b>Trillian test result (tid-1348)</b>
   Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 37172 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1348-vmware-65u2.zip
   Smoke tests completed. 89 look OK, 0 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-800145816


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_multiplication_x: debian. SL-JID 135


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595843031



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java
##########
@@ -122,34 +122,34 @@ public String getEventDescription() {
 
     @Override
     public void execute() {
-        if (getHostId() == null && getStorageId() == null) {
-            throw new InvalidParameterValueException("Either hostId or storageId must be specified");
-        }
-
         if (getHostId() != null && getStorageId() != null) {
             throw new InvalidParameterValueException("Only one of hostId and storageId can be specified");
         }
+
         try {
             //FIXME : Should not be calling UserVmService to migrate all types of VMs - need a generic VM layer
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
-                Host destinationHost = _resourceService.getHost(getHostId());
-                if (destinationHost == null) {
-                    throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
-                }
-                if (destinationHost.getType() != Host.Type.Routing) {
-                    throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
-                }
-                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
-                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());
-            } else if (getStorageId() != null) {
+            if (getStorageId() != null) {
                 // OfflineMigration performed when this parameter is specified
                 StoragePool destStoragePool = _storageService.getStoragePool(getStorageId());
                 if (destStoragePool == null) {
                     throw new InvalidParameterValueException("Unable to find the storage pool to migrate the VM");
                 }
                 CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to storage pool Id: " + getStorageId());
                 migratedVm = _userVmService.vmStorageMigration(getVirtualMachineId(), destStoragePool);
+            } else {
+                Host destinationHost = null;
+                if (getHostId() != null) {
+                    destinationHost =_resourceService.getHost(getHostId());
+                    if (destinationHost == null) {
+                        throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
+                    }
+                    if (destinationHost.getType() != Host.Type.Routing) {
+                        throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
+                    }
+                }
+                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
+                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());

Review comment:
       @weizhouapache For user VMs we have two different APIs, migrateVirtualMachine and migrateVirtualMachineWithVolume. Depending on the need for storage migration requirement appropriate API can be used.
   While working on #4385 choice was to add a new API migrateSystemVmWithVolume or using `migrateVirtualMachineWithVolume` method while code making the decision if volume needs to be migrated.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-735230162


   <b>Trillian test result (tid-3282)</b>
   Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 95091 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t3282-xenserver-71.zip
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_primary_storage.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_secondary_storage.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_deployment_planner.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Smoke tests completed. 76 look OK, 10 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_l2network_restart | `Error` | 3709.22 | test_network.py
   test_l2network_restart | `Error` | 3711.03 | test_network.py
   ContextSuite context=TestL2Networks>:teardown | `Error` | 3712.74 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 7379.44 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 3739.68 | test_network.py
   test_releaseIP | `Error` | 3607.99 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 7313.90 | test_network.py
   ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py
   test_01_create_delete_portforwarding_fornonvpc | `Error` | 3668.44 | test_portforwardingrules.py
   test_01_add_primary_storage_disabled_host | `Error` | 5.38 | test_primary_storage.py
   test_01_primary_storage_nfs | `Error` | 0.17 | test_primary_storage.py
   ContextSuite context=TestStorageTags>:setup | `Error` | 0.29 | test_primary_storage.py
   ContextSuite context=TestCrossDomainAccountAdd>:teardown | `Error` | 35.36 | test_projects.py
   test_01_scale_vm | `Failure` | 2.18 | test_scale_vm.py
   test_01_sys_vm_start | `Failure` | 0.09 | test_secondary_storage.py
   test_02_list_snapshots_with_removed_data_store | `Error` | 1.19 | test_snapshots.py
   test_01_deploy_vm_on_specific_host | `Error` | 37.34 | test_vm_deployment_planner.py
   test_04_deploy_vm_on_host_override_pod_and_cluster | `Error` | 6.41 | test_vm_deployment_planner.py
   test_11_migrate_vm | `Error` | 5.34 | test_vm_life_cycle.py
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd closed pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
rhtyd closed pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-888205479


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-868431097


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache closed pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-886402193


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-883940417


   @weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-864944437






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-889175088


   @weizhouapache a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-885449573


   @weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-836204689


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 540


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-864956342


   UI build: :heavy_check_mark:
   Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/4378 (SL-JID-270)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-875869415


   > Sorry for the late reply, @ustcweizhou.
   > 
   > > @GabrielBrascher @sureshanaparti
   > > we have 3 options
   > > (1) autoselect is 'true' by default
   > > (2) autoselect is 'false' by default (current behavior)
   > > (3) a global setting. if autoselect is not set in api request, then honor the global setting.
   > > what's your opinion ?
   > 
   > I like to give autonomy to Admins thus I would go with **(1)**, but I do understand why **(2)** is considered.
   > 
   > Regarding **(3)**, I personally prefer to avoid adding new global settings, if possible. Keeping **(1)** or **(2)** would update the API documentation which would keep users/admins in the same page with the API changes.
   
   @GabrielBrascher thanks for reply. both are ok to me.
   @sureshanaparti your opinion ?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-886402015


   @blueorangutan test centos7 vmware-65u2


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache edited a comment on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache edited a comment on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-868444598


   > changes look good, but these are made to a lt of suspect code.
   > Also my original functional doubt is unanswerred:
   > 
   > * one operator makes one host/cluster free or less occupied
   > * another one makes another host/cluster free
   > * they end out planning VM-tennis
   
   @DaanHoogland yeah, but it could happen even if users migrate vms manually.
   
   the choose of destination host in this pr, is same as what cloudstack does when start a vm (capacity check, affinity group check, tag check, ordering, etc).
   
   migrate vm without hostid -> simiar as start vm without hostid
   migrate vm with hostid -> simiar as start vm with hostid
   
   We/community provide features, but it is up to users to choose a proper solution which meet their requirements.
   in your example, users can migrate vms with destination host(again, it is up to them), or use other options like host tags.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-888579417


   @blueorangutan test matrix


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-889736816


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-884407818


   @weizhouapache unsupported parameters provided. Supported mgmt server os are: `centos7, centos6, alma8, ubuntu18, suse15, ubuntu20, rocky8, centos8`. Supported hypervisors are: `kvm-centos6, kvm-centos7, kvm-centos8, kvm-rocky8, kvm-alma8, kvm-ubuntu18, kvm-ubuntu20, kvm-suse15, vmware-55u3, vmware-60u2, vmware-65u2, vmware-67u3, vmware-70u1, xenserver-65sp1, xenserver-71, xenserver-74, xcpng74, xcpng76, xcpng80, xcpng81`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r658619185



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVMCmd.java
##########
@@ -155,6 +161,8 @@ public void execute() {
                 throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
             }
             CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
+        } else if (! isAutoSelect()) {
+            throw new InvalidParameterValueException("Please specify a host or storage as destination, or pass 'autoselect=true' to automatically select a destination host which do not require storage migration");

Review comment:
       @sureshanaparti
   this pr works only if storage migraiotn is not required.
   it can be improved in a new pr.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-872995397


   @weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-766730273


   @ustcweizhou needs changes in new UI


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-872995078


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-783370267


   @blueorangutan package


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r596645655



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVMCmd.java
##########
@@ -169,9 +165,9 @@ public void execute() {
 
         try {
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
+            if (getStoragePoolId() == null) {

Review comment:
       @weizhouapache host id in the cmd is UUID, so i think it is better to take input from another param when host id is null. If host id is not null, ignore auto selection.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-868569205


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 371


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-871945452


   > @weizhouapache between global settings configuration or the AUTO_SELECT I would prefer the last one. I think that keeping it in the API would be easier for users to find this out due to the API documentation.
   
   @GabrielBrascher do you mean we change the default value to true ?
   
   @sureshanaparti what's your opinion ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-887305871






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-886006770


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-884681161


   @blueorangutan test centos7 vmware-65u2


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-889734877


   <b>Trillian test result (tid-1420)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 60279 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1420-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
   Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_iso.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_iso.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py
   Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 64 look OK, 25 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestAccounts>:setup | `Error` | 0.00 | test_accounts.py
   ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 0.00 | test_accounts.py
   test_DeleteDomain | `Error` | 3.97 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 3.96 | test_accounts.py
   ContextSuite context=TestRemoveUserFromAccount>:setup | `Error` | 4.98 | test_accounts.py
   ContextSuite context=TestInternalLb>:setup | `Error` | 0.00 | test_internal_lb.py
   test_01_nic | `Error` | 136.29 | test_nic.py
   test_01_1_create_iso_with_checksum_sha1_negative | `Error` | 66.40 | test_iso.py
   test_01_create_iso_with_checksum_sha1 | `Error` | 66.37 | test_iso.py
   test_01_create_iso_with_checksum_sha1 | `Error` | 66.37 | test_iso.py
   test_02_1_create_iso_with_checksum_sha256_negative | `Error` | 66.42 | test_iso.py
   test_04_extract_Iso | `Failure` | 128.25 | test_iso.py
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   ContextSuite context=TestDeployVMFromISO>:setup | `Error` | 0.00 | test_deploy_vm_iso.py
   test_list_vms_metrics | `Error` | 0.67 | test_metrics_api.py
   ContextSuite context=TestDeployVmWithVariedPlanners>:setup | `Error` | 0.00 | test_deploy_vms_with_varied_deploymentplanners.py
   ContextSuite context=TestDeployVmWithUserData>:setup | `Error` | 0.00 | test_deploy_vm_with_userdata.py
   test_03_ping_in_ssvm_success | `Failure` | 15.74 | test_diagnostics.py
   test_08_arping_in_ssvm | `Failure` | 5.15 | test_diagnostics.py
   test_09_arping_in_cpvm | `Failure` | 5.15 | test_diagnostics.py
   test_13_retrieve_vr_default_files | `Failure` | 129.28 | test_diagnostics.py
   test_14_retrieve_vr_one_file | `Failure` | 128.38 | test_diagnostics.py
   test_15_retrieve_ssvm_default_files | `Failure` | 129.41 | test_diagnostics.py
   test_16_retrieve_ssvm_single_file | `Failure` | 128.38 | test_diagnostics.py
   test_17_retrieve_cpvm_default_files | `Failure` | 129.41 | test_diagnostics.py
   test_18_retrieve_cpvm_single_file | `Failure` | 129.28 | test_diagnostics.py
   ContextSuite context=TestNetworkACL>:setup | `Error` | 0.00 | test_network_acl.py
   test_delete_account | `Error` | 0.45 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 1.10 | test_network.py
   test_deploy_vm_l2network | `Error` | 1.10 | test_network.py
   test_l2network_restart | `Error` | 2.22 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 3.37 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 1.02 | test_network.py
   test_reboot_router | `Failure` | 0.04 | test_network.py
   test_releaseIP | `Error` | 0.43 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 0.48 | test_network.py
   ContextSuite context=TestDomainsServiceOfferings>:setup | `Error` | 1553.54 | test_domain_service_offerings.py
   test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_04_basic_lifecycle_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_06_deploy_invalid_kubernetes_ha_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Failure` | 0.05 | test_multipleips_per_nic.py
   ContextSuite context=TestNestedVirtualization>:setup | `Error` | 0.00 | test_nested_virtualization.py
   ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py
   test_01_redundant_vpc_site2site_vpn | `Failure` | 359.18 | test_vpc_vpn.py
   ContextSuite context=TestPortForwardingRules>:setup | `Error` | 0.00 | test_portforwardingrules.py
   ContextSuite context=TestPrivateGwACL>:setup | `Error` | 0.00 | test_privategw_acl.py
   test_09_project_suspend | `Error` | 1.13 | test_projects.py
   test_10_project_activation | `Error` | 1.06 | test_projects.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_hostha_enable_ha_when_host_in_maintenance | `Error` | 301.88 | test_hostha_kvm.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r662889066



##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -5895,8 +5897,46 @@ public VirtualMachine migrateVirtualMachine(Long vmId, Host destinationHost) thr
             throw new InvalidParameterValueException("Cannot migrate VM, host with id: " + srcHostId + " for VM not found");
         }
 
+        DeployDestination dest = null;
+        if (destinationHost == null) {
+            vm.setLastHostId(null); // Last host does not have higher priority in vm migration
+            final ServiceOfferingVO offering = _offeringDao.findById(vm.getId(), vm.getServiceOfferingId());
+            final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, null, offering, null, null);
+            final Host host = _hostDao.findById(srcHostId);
+            final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), host.getPodId(), host.getClusterId(), null, null, null);
+            ExcludeList excludes = new ExcludeList();
+            excludes.addHost(srcHostId);
+            try {
+                dest = _planningMgr.planDeployment(profile, plan, excludes, null);
+            } catch (final AffinityConflictException e2) {
+                s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2);
+                throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict");
+            } catch (final InsufficientServerCapacityException e3) {
+                throw new CloudRuntimeException("Unable to find a server to migrate the vm to");
+            }
+        } else {
+            dest = checkVmMigrationDestination(vm, srcHost, destinationHost);
+        }
 
-        if (destinationHost.getId() == srcHostId) {
+        // If no suitable destination found then throw exception
+        if (dest == null) {
+            throw new CloudRuntimeException("Unable to find suitable destination to migrate VM " + vm.getInstanceName());
+        }
+
+        UserVmVO uservm = _vmDao.findById(vmId);
+        if (uservm != null) {
+            collectVmDiskStatistics(uservm);
+            collectVmNetworkStatistics(uservm);
+        }
+        _itMgr.migrate(vm.getUuid(), srcHostId, dest);

Review comment:
       @DaanHoogland clear to me now. 
   I will add a new method to collect vm network/disk statistics which will be used before stopping/migrating vm.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r596633019



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java
##########
@@ -122,34 +122,34 @@ public String getEventDescription() {
 
     @Override
     public void execute() {
-        if (getHostId() == null && getStorageId() == null) {
-            throw new InvalidParameterValueException("Either hostId or storageId must be specified");
-        }
-
         if (getHostId() != null && getStorageId() != null) {
             throw new InvalidParameterValueException("Only one of hostId and storageId can be specified");
         }
+
         try {
             //FIXME : Should not be calling UserVmService to migrate all types of VMs - need a generic VM layer
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
-                Host destinationHost = _resourceService.getHost(getHostId());
-                if (destinationHost == null) {
-                    throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
-                }
-                if (destinationHost.getType() != Host.Type.Routing) {
-                    throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
-                }
-                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
-                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());
-            } else if (getStorageId() != null) {
+            if (getStorageId() != null) {
                 // OfflineMigration performed when this parameter is specified
                 StoragePool destStoragePool = _storageService.getStoragePool(getStorageId());
                 if (destStoragePool == null) {
                     throw new InvalidParameterValueException("Unable to find the storage pool to migrate the VM");
                 }
                 CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to storage pool Id: " + getStorageId());
                 migratedVm = _userVmService.vmStorageMigration(getVirtualMachineId(), destStoragePool);
+            } else {
+                Host destinationHost = null;
+                if (getHostId() != null) {
+                    destinationHost =_resourceService.getHost(getHostId());
+                    if (destinationHost == null) {
+                        throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
+                    }
+                    if (destinationHost.getType() != Host.Type.Routing) {
+                        throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
+                    }
+                }
+                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
+                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());

Review comment:
       @weizhouapache Yes, works fine in that case. Tested,
   
   ```
   (localcloud) SBCM5> > migrate systemvm virtualmachineid=c3737243-93b2-4369-bf60-08ac217e3465
   {
     "systemvm": {
       "agentstate": "Up",
       "created": "2021-03-16T11:22:18+0000",
       "dns1": "10.0.32.1",
       "dns2": "8.8.8.8",
       "gateway": "10.0.48.1",
       "hostid": "2e5b053f-539e-45c7-80f3-0fe3f0291c05",
       "hostname": "10.0.33.141",
       "hypervisor": "VMware",
       "id": "c3737243-93b2-4369-bf60-08ac217e3465",
       "linklocalmacaddress": "02:00:5e:cd:00:01",
       "name": "s-1-VM",
       "podid": "57382744-92f2-4610-a873-733de71eb142",
       "podname": "Pod1",
       "privateip": "10.0.36.123",
       "privatemacaddress": "1e:00:47:00:00:17",
       "privatenetmask": "255.255.240.0",
       "publicip": "10.0.52.121",
       "publicmacaddress": "1e:00:70:00:00:01",
       "publicnetmask": "255.255.240.0",
       "state": "Running",
       "systemvmtype": "secondarystoragevm",
       "templateid": "ff29ad45-6b1d-4c8e-aa6e-74f41b483134",
       "templatename": "SystemVM Template (vSphere)",
       "version": "4.16.0.0-SNAPSHOT",
       "zoneid": "8f79f870-abc3-4d46-bb0e-cdb695ca45cd",
       "zonename": "pr4378-t146-vmware-67u3"
     }
   }
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-733871353


   Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2438


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-895350214


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595905675



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java
##########
@@ -122,34 +122,34 @@ public String getEventDescription() {
 
     @Override
     public void execute() {
-        if (getHostId() == null && getStorageId() == null) {
-            throw new InvalidParameterValueException("Either hostId or storageId must be specified");
-        }
-
         if (getHostId() != null && getStorageId() != null) {
             throw new InvalidParameterValueException("Only one of hostId and storageId can be specified");
         }
+
         try {
             //FIXME : Should not be calling UserVmService to migrate all types of VMs - need a generic VM layer
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
-                Host destinationHost = _resourceService.getHost(getHostId());
-                if (destinationHost == null) {
-                    throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
-                }
-                if (destinationHost.getType() != Host.Type.Routing) {
-                    throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
-                }
-                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
-                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());
-            } else if (getStorageId() != null) {
+            if (getStorageId() != null) {
                 // OfflineMigration performed when this parameter is specified
                 StoragePool destStoragePool = _storageService.getStoragePool(getStorageId());
                 if (destStoragePool == null) {
                     throw new InvalidParameterValueException("Unable to find the storage pool to migrate the VM");
                 }
                 CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to storage pool Id: " + getStorageId());
                 migratedVm = _userVmService.vmStorageMigration(getVirtualMachineId(), destStoragePool);
+            } else {
+                Host destinationHost = null;
+                if (getHostId() != null) {
+                    destinationHost =_resourceService.getHost(getHostId());
+                    if (destinationHost == null) {
+                        throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
+                    }
+                    if (destinationHost.getType() != Host.Type.Routing) {
+                        throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
+                    }
+                }
+                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
+                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());

Review comment:
       @shwstppr we pass a empty hashmap in `migrateVirtualMachineWithVolume `, see below
   https://github.com/apache/cloudstack/blob/master/api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java#L144
   
   so systemvm migration actually works without volume ?
   I changed it back to 4.15, systemvm migration works
   https://github.com/apache/cloudstack/blob/4.15/api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java#L120
   
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r655297475



##########
File path: ui/src/views/compute/MigrateWizard.vue
##########
@@ -168,6 +170,12 @@ export default {
         this.hosts.sort((a, b) => {
           return b.suitableformigration - a.suitableformigration
         })
+        for (const key in this.hosts) {
+          if (this.hosts[key].suitableformigration && !this.hosts[key].requiresstoragemigration) {
+            this.hosts.unshift({ id: -1, name: 'autoselect', suitableformigration: true, requiresstoragemigration: false })

Review comment:
       @weizhouapache do we need to add this string - `autoselect` in translation?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r662924155



##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -5895,8 +5897,46 @@ public VirtualMachine migrateVirtualMachine(Long vmId, Host destinationHost) thr
             throw new InvalidParameterValueException("Cannot migrate VM, host with id: " + srcHostId + " for VM not found");
         }
 
+        DeployDestination dest = null;
+        if (destinationHost == null) {
+            vm.setLastHostId(null); // Last host does not have higher priority in vm migration
+            final ServiceOfferingVO offering = _offeringDao.findById(vm.getId(), vm.getServiceOfferingId());
+            final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, null, offering, null, null);
+            final Host host = _hostDao.findById(srcHostId);
+            final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), host.getPodId(), host.getClusterId(), null, null, null);
+            ExcludeList excludes = new ExcludeList();
+            excludes.addHost(srcHostId);
+            try {
+                dest = _planningMgr.planDeployment(profile, plan, excludes, null);
+            } catch (final AffinityConflictException e2) {
+                s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2);
+                throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict");
+            } catch (final InsufficientServerCapacityException e3) {
+                throw new CloudRuntimeException("Unable to find a server to migrate the vm to");
+            }
+        } else {
+            dest = checkVmMigrationDestination(vm, srcHost, destinationHost);
+        }
 
-        if (destinationHost.getId() == srcHostId) {
+        // If no suitable destination found then throw exception
+        if (dest == null) {
+            throw new CloudRuntimeException("Unable to find suitable destination to migrate VM " + vm.getInstanceName());
+        }
+
+        UserVmVO uservm = _vmDao.findById(vmId);
+        if (uservm != null) {
+            collectVmDiskStatistics(uservm);
+            collectVmNetworkStatistics(uservm);
+        }
+        _itMgr.migrate(vm.getUuid(), srcHostId, dest);

Review comment:
       thanks for review and suggestions @DaanHoogland 

##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -5895,8 +5897,46 @@ public VirtualMachine migrateVirtualMachine(Long vmId, Host destinationHost) thr
             throw new InvalidParameterValueException("Cannot migrate VM, host with id: " + srcHostId + " for VM not found");
         }
 
+        DeployDestination dest = null;
+        if (destinationHost == null) {
+            vm.setLastHostId(null); // Last host does not have higher priority in vm migration
+            final ServiceOfferingVO offering = _offeringDao.findById(vm.getId(), vm.getServiceOfferingId());
+            final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, null, offering, null, null);
+            final Host host = _hostDao.findById(srcHostId);
+            final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), host.getPodId(), host.getClusterId(), null, null, null);
+            ExcludeList excludes = new ExcludeList();
+            excludes.addHost(srcHostId);
+            try {
+                dest = _planningMgr.planDeployment(profile, plan, excludes, null);
+            } catch (final AffinityConflictException e2) {
+                s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2);
+                throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict");
+            } catch (final InsufficientServerCapacityException e3) {
+                throw new CloudRuntimeException("Unable to find a server to migrate the vm to");
+            }
+        } else {
+            dest = checkVmMigrationDestination(vm, srcHost, destinationHost);
+        }
 
-        if (destinationHost.getId() == srcHostId) {
+        // If no suitable destination found then throw exception
+        if (dest == null) {
+            throw new CloudRuntimeException("Unable to find suitable destination to migrate VM " + vm.getInstanceName());
+        }
+
+        UserVmVO uservm = _vmDao.findById(vmId);
+        if (uservm != null) {
+            collectVmDiskStatistics(uservm);
+            collectVmNetworkStatistics(uservm);
+        }
+        _itMgr.migrate(vm.getUuid(), srcHostId, dest);

Review comment:
       Done. thanks for review and suggestions @DaanHoogland 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-804146365


   @shwstppr @sureshanaparti added a parameter 'autoselect' to migratevm and migratesystemvm apis.
   
   it is not valid for api migratevmwithvolume


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-873021858


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + vmware-65u2) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-873020891


   @blueorangutan test centos7 vmware-65u2


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-864948905


   @shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-888580055


   @weizhouapache a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-888219710


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 669


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-864944437


   @shwstppr @rhtyd updated this pr.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache closed pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-800285573


   > Working for user vms but fails for systemvm because of condition mentioned in MigrateSysteVMCmd.java
   > 
   > ```
   > > migrate systemvm virtualmachineid=c3737243-93b2-4369-bf60-08ac217e3465
   > {
   >   "accountid": "2db730f5-8649-11eb-989e-1e00f1000222",
   >   "cmd": "org.apache.cloudstack.api.command.admin.systemvm.MigrateSystemVMCmd",
   >   "completed": "2021-03-16T12:39:17+0000",
   >   "created": "2021-03-16T12:39:17+0000",
   >   "jobid": "0e9fb1b6-00eb-4614-8f23-aca481eb6bd1",
   >   "jobprocstatus": 0,
   >   "jobresult": {
   >     "errorcode": 431,
   >     "errortext": "Either hostId or storageId must be specified"
   >   },
   >   "jobresultcode": 530,
   >   "jobresulttype": "object",
   >   "jobstatus": 2,
   >   "userid": "2db8127b-8649-11eb-989e-1e00f1000222"
   > }
   > ```
   
   @shwstppr my mistake. I removed wrong lines.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-871147868


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-884681285


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + vmware-65u2) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-895686395


   <b>Trillian test result (tid-1558)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 35862 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1558-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 89 look OK, 0 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-867168059


   <b>Trillian test result (tid-1058)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 47563 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1058-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 84 look OK, 4 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Failure` | 540.79 | test_internal_lb.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 103.02 | test_kubernetes_clusters.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 144.26 | test_routers_network_ops.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 470.25 | test_vpc_redundant.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-766730273


   @ustcweizhou needs changes in new UI


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-733853668


   @DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r502264412



##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -5627,6 +5629,50 @@ public VirtualMachine migrateVirtualMachine(Long vmId, Host destinationHost) thr
 
         // check if migrating to same host
         long srcHostId = vm.getHostId();
+
+        DeployDestination dest = null;
+        if (destinationHost == null) {
+            vm.setLastHostId(null); // Do not check last host
+            final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm);
+            final Host host = _hostDao.findById(srcHostId);
+            final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), null, null, null, null, null);

Review comment:
       @ustcweizhou finding a suitable host anywhere within the zone might result in returning a host in a different pod or cluster. This might also need storage migration. Will that be an issue? Especially when VM is having data disks attached. Also, cross-cluster at present fails on VMware when storage pools are cluster scoped. #4385 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-866606369


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-875869551


   @blueorangutan test centos7 vmware-65u2


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-870711656


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 426


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595769435



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java
##########
@@ -122,34 +122,34 @@ public String getEventDescription() {
 
     @Override
     public void execute() {
-        if (getHostId() == null && getStorageId() == null) {
-            throw new InvalidParameterValueException("Either hostId or storageId must be specified");
-        }
-
         if (getHostId() != null && getStorageId() != null) {
             throw new InvalidParameterValueException("Only one of hostId and storageId can be specified");
         }
+
         try {
             //FIXME : Should not be calling UserVmService to migrate all types of VMs - need a generic VM layer
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
-                Host destinationHost = _resourceService.getHost(getHostId());
-                if (destinationHost == null) {
-                    throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
-                }
-                if (destinationHost.getType() != Host.Type.Routing) {
-                    throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
-                }
-                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
-                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());
-            } else if (getStorageId() != null) {
+            if (getStorageId() != null) {
                 // OfflineMigration performed when this parameter is specified
                 StoragePool destStoragePool = _storageService.getStoragePool(getStorageId());
                 if (destStoragePool == null) {
                     throw new InvalidParameterValueException("Unable to find the storage pool to migrate the VM");
                 }
                 CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to storage pool Id: " + getStorageId());
                 migratedVm = _userVmService.vmStorageMigration(getVirtualMachineId(), destStoragePool);
+            } else {
+                Host destinationHost = null;
+                if (getHostId() != null) {
+                    destinationHost =_resourceService.getHost(getHostId());
+                    if (destinationHost == null) {
+                        throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
+                    }
+                    if (destinationHost.getType() != Host.Type.Routing) {
+                        throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
+                    }
+                }
+                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
+                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());

Review comment:
       @weizhouapache change added in UserVmManagerImpl to find suitable host is only for `migrateVirtualMachine` method while we call `migrateVirtualMachineWithVolume`
   Currently we will get NPE on https://github.com/apache/cloudstack/blob/f5aed32ebddef19b9b2f1bed724bf0b16c69f0b6/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java#L6289




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-870689095


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache closed pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-733853268


   @blueorangutan package


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-870798173


   @weizhouapache a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-870556507


   > Trillian test result (tid-1114)
   > Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   > Total time taken: 39823 seconds
   > Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1114-vmware-65u2.zip
   > Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   > Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   > Smoke tests completed. 87 look OK, 1 have error(s)
   > Only failed tests results shown below:
   > 
   > Test	Result	Time (s)	Test File
   > test_01_migrate_VM_and_root_volume	`Error`	106.91	test_vm_life_cycle.py
   > test_02_migrate_VM_with_two_data_disks	`Error`	63.30	test_vm_life_cycle.py
   
   @weizhouapache can you check these test failures, and retrigger the tests please ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-864948548






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-866605769






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-864964761


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 314


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-780334015


   @weizhouapache this will need some update with late changes in master


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-886006745


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-895095919


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 803


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-866248723


   <b>Trillian test result (tid-1031)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 41014 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1031-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_templates.py
   Intermittent failure detected: /marvin/tests/smoke/test_usage.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Smoke tests completed. 85 look OK, 3 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 1023.61 | test_privategw_acl.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 340.66 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 340.23 | test_routers_network_ops.py
   test_08_migrate_vm | `Error` | 1.07 | test_vm_life_cycle.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-888613565


   <b>Trillian Build Failed (tid-1398)<b/>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-886015600


   <b>Trillian test result (tid-1362)</b>
   Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 76717 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1362-vmware-65u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_pvlan.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 77 look OK, 12 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Failure` | 617.43 | test_internal_lb.py
   test_create_pvlan_network | `Error` | 0.03 | test_pvlan.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
   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=TestServiceOfferings>:setup | `Error` | 1519.86 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 610.22 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 591.76 | test_vpc_redundant.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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-890144964


   <b>Trillian test result (tid-1429)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 40649 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1429-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 88 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 515.61 | test_kubernetes_clusters.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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-884949132


   > > Sorry for the late reply, @ustcweizhou.
   > > > @GabrielBrascher @sureshanaparti
   > > > we have 3 options
   > > > (1) autoselect is 'true' by default
   > > > (2) autoselect is 'false' by default (current behavior)
   > > > (3) a global setting. if autoselect is not set in api request, then honor the global setting.
   > > > what's your opinion ?
   > > 
   > > 
   > > I like to give autonomy to Admins thus I would go with **(1)**, but I do understand why **(2)** is considered.
   > > Regarding **(3)**, I personally prefer to avoid adding new global settings, if possible. Keeping **(1)** or **(2)** would update the API documentation which would keep users/admins in the same page with the API changes.
   > 
   > @GabrielBrascher thanks for reply. both are ok to me.
   > @sureshanaparti your opinion ?
   
   I'm OK with **(1)**, so auto pick the host to migrate when host/storage are not specified, else ignore the autoselect flag.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-887926997


   @nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache closed pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-885886128


   <b>Trillian test result (tid-1361)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 34846 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1361-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 86 look OK, 3 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 339.14 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 337.73 | test_routers_network_ops.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 463.55 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Failure` | 353.62 | test_vpc_redundant.py
   test_01_vpc_site2site_vpn | `Failure` | 243.24 | test_vpc_vpn.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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-885524438


   @weizhouapache a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-885459319


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 636


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-884562313


   <b>Trillian Build Failed (tid-1346)<b/>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r658756913



##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -5895,8 +5897,46 @@ public VirtualMachine migrateVirtualMachine(Long vmId, Host destinationHost) thr
             throw new InvalidParameterValueException("Cannot migrate VM, host with id: " + srcHostId + " for VM not found");
         }
 
+        DeployDestination dest = null;
+        if (destinationHost == null) {
+            vm.setLastHostId(null); // Last host does not have higher priority in vm migration
+            final ServiceOfferingVO offering = _offeringDao.findById(vm.getId(), vm.getServiceOfferingId());
+            final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, null, offering, null, null);
+            final Host host = _hostDao.findById(srcHostId);
+            final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), host.getPodId(), host.getClusterId(), null, null, null);
+            ExcludeList excludes = new ExcludeList();
+            excludes.addHost(srcHostId);
+            try {
+                dest = _planningMgr.planDeployment(profile, plan, excludes, null);
+            } catch (final AffinityConflictException e2) {
+                s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2);
+                throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict");
+            } catch (final InsufficientServerCapacityException e3) {
+                throw new CloudRuntimeException("Unable to find a server to migrate the vm to");
+            }
+        } else {
+            dest = checkVmMigrationDestination(vm, srcHost, destinationHost);
+        }
 
-        if (destinationHost.getId() == srcHostId) {
+        // If no suitable destination found then throw exception
+        if (dest == null) {
+            throw new CloudRuntimeException("Unable to find suitable destination to migrate VM " + vm.getInstanceName());
+        }
+
+        UserVmVO uservm = _vmDao.findById(vmId);
+        if (uservm != null) {
+            collectVmDiskStatistics(uservm);
+            collectVmNetworkStatistics(uservm);
+        }
+        _itMgr.migrate(vm.getUuid(), srcHostId, dest);

Review comment:
       @DaanHoogland 
   which lines do you mean ?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-883956513


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 612


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] GabrielBrascher commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-875802574


   Sorry for the late reply, @ustcweizhou.
   
   > @GabrielBrascher @sureshanaparti
   > we have 3 options
   > (1) autoselect is 'true' by default
   > (2) autoselect is 'false' by default (current behavior)
   > (3) a global setting. if autoselect is not set in api request, then honor the global setting.
   > 
   > what's your opinion ?
   
   I like to give autonomy to Admins thus I would go with **(1)**,  but I do understand why **(2)** is considered.
   
   Regarding **(3)**, I personally prefer to avoid adding new global settings, if possible. Keeping **(1)** or **(2)** would update the API documentation which would keep users/admins in the same page with the API changes.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-868444598


   > changes look good, but these are made to a lt of suspect code.
   > Also my original functional doubt is unanswerred:
   > 
   > * one operator makes one host/cluster free or less occupied
   > * another one makes another host/cluster free
   > * they end out planning VM-tennis
   
   @DaanHoogland yeah, but it could happen even if users migrate vms manually.
   
   the choose of destination host in this pr, is same as what cloudstack does when start a vm (capacity check, affinity group check, tag check, ordering, etc).
   
   migrate vm without hostid -> simiar as start vm without hostid
   migrate vm with hostid -> simiar as start vm with hostid
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-824733446


   Self-assigned this to test/review new api, ui changes from last review


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-871127198


   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-783370783


   @shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-783840465


   <b>Trillian test result (tid-3593)</b>
   Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 37820 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t3593-vmware-65u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Smoke tests completed. 85 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 782.62 | test_kubernetes_clusters.py
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r658756669



##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -5895,8 +5897,46 @@ public VirtualMachine migrateVirtualMachine(Long vmId, Host destinationHost) thr
             throw new InvalidParameterValueException("Cannot migrate VM, host with id: " + srcHostId + " for VM not found");
         }
 
+        DeployDestination dest = null;
+        if (destinationHost == null) {
+            vm.setLastHostId(null); // Last host does not have higher priority in vm migration
+            final ServiceOfferingVO offering = _offeringDao.findById(vm.getId(), vm.getServiceOfferingId());
+            final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, null, offering, null, null);
+            final Host host = _hostDao.findById(srcHostId);
+            final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), host.getPodId(), host.getClusterId(), null, null, null);
+            ExcludeList excludes = new ExcludeList();
+            excludes.addHost(srcHostId);
+            try {
+                dest = _planningMgr.planDeployment(profile, plan, excludes, null);
+            } catch (final AffinityConflictException e2) {
+                s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2);
+                throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict");
+            } catch (final InsufficientServerCapacityException e3) {
+                throw new CloudRuntimeException("Unable to find a server to migrate the vm to");
+            }
+        } else {
+            dest = checkVmMigrationDestination(vm, srcHost, destinationHost);
+        }

Review comment:
       @DaanHoogland 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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-868960744


   <b>Trillian test result (tid-1114)</b>
   Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 39823 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4378-t1114-vmware-65u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Smoke tests completed. 87 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_migrate_VM_and_root_volume | `Error` | 106.91 | test_vm_life_cycle.py
   test_02_migrate_VM_with_two_data_disks | `Error` | 63.30 | test_vm_life_cycle.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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-885449272


   @GabrielBrascher @sureshanaparti changed default value of autoselect to 'true'.
   
   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r677864062



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java
##########
@@ -91,6 +97,10 @@ public Long getStorageId() {
         return storageId;
     }
 
+    public Boolean isAutoSelect() {
+        return autoSelect != null ? autoSelect : true;

Review comment:
       Minor suggestion: to use `BooleanUtils.toBoolean(autoSelect)` here

##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVMCmd.java
##########
@@ -93,6 +99,10 @@ public Long getStoragePoolId() {
         return storageId;
     }
 
+    public Boolean isAutoSelect() {
+        return autoSelect != null ? autoSelect : true;

Review comment:
       Same here




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-888205850


   @weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-889737377


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r655318680



##########
File path: ui/src/views/compute/MigrateWizard.vue
##########
@@ -168,6 +170,12 @@ export default {
         this.hosts.sort((a, b) => {
           return b.suitableformigration - a.suitableformigration
         })
+        for (const key in this.hosts) {
+          if (this.hosts[key].suitableformigration && !this.hosts[key].requiresstoragemigration) {
+            this.hosts.unshift({ id: -1, name: 'autoselect', suitableformigration: true, requiresstoragemigration: false })

Review comment:
       @shwstppr done.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-895055248


   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-800130563


   @shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. [S]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-783471922


   @blueorangutan test matrix


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on a change in pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#discussion_r595912257



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/MigrateSystemVMCmd.java
##########
@@ -122,34 +122,34 @@ public String getEventDescription() {
 
     @Override
     public void execute() {
-        if (getHostId() == null && getStorageId() == null) {
-            throw new InvalidParameterValueException("Either hostId or storageId must be specified");
-        }
-
         if (getHostId() != null && getStorageId() != null) {
             throw new InvalidParameterValueException("Only one of hostId and storageId can be specified");
         }
+
         try {
             //FIXME : Should not be calling UserVmService to migrate all types of VMs - need a generic VM layer
             VirtualMachine migratedVm = null;
-            if (getHostId() != null) {
-                Host destinationHost = _resourceService.getHost(getHostId());
-                if (destinationHost == null) {
-                    throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
-                }
-                if (destinationHost.getType() != Host.Type.Routing) {
-                    throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
-                }
-                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
-                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());
-            } else if (getStorageId() != null) {
+            if (getStorageId() != null) {
                 // OfflineMigration performed when this parameter is specified
                 StoragePool destStoragePool = _storageService.getStoragePool(getStorageId());
                 if (destStoragePool == null) {
                     throw new InvalidParameterValueException("Unable to find the storage pool to migrate the VM");
                 }
                 CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to storage pool Id: " + getStorageId());
                 migratedVm = _userVmService.vmStorageMigration(getVirtualMachineId(), destStoragePool);
+            } else {
+                Host destinationHost = null;
+                if (getHostId() != null) {
+                    destinationHost =_resourceService.getHost(getHostId());
+                    if (destinationHost == null) {
+                        throw new InvalidParameterValueException("Unable to find the host to migrate the VM, host id=" + getHostId());
+                    }
+                    if (destinationHost.getType() != Host.Type.Routing) {
+                        throw new InvalidParameterValueException("The specified host(" + destinationHost.getName() + ") is not suitable to migrate the VM, please specify another one");
+                    }
+                }
+                CallContext.current().setEventDetails("VM Id: " + getVirtualMachineId() + " to host Id: " + getHostId());
+                migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, new HashMap<String, String>());

Review comment:
       @weizhouapache yes, we do pass empty volume-pool mapping to the method but later in the code, we  generate volume-pool mapping when needed (such as inter-cluster migration with cluster-scoped pools)
   Moving back to 4.15 code will definitely allow systemvm migration with changes from this PR but inter-cluster migration of systemvms would not work.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4378: server: Optional destination host when migrate a vm

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4378:
URL: https://github.com/apache/cloudstack/pull/4378#issuecomment-866517897


   @weizhouapache can you please have a look at migration test failures


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org