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/03/05 08:58:26 UTC
[GitHub] [cloudstack] shwstppr opened a new pull request #4758: Fix vm volmigrate bug
shwstppr opened a new pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758
### Description
Fixes #4674
TODO: Fix for VMs with deploy-as-is
<!--- ********************************************************************************* -->
<!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ********************************************************************************* -->
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [ ] Minor
#### Bug Severity
- [x] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
<!-- 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] blueorangutan commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-826082608
<b>Trillian test result (tid-514)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33934 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4758-t514-vmware-67u3.zip
Smoke tests completed. 87 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-818704348
@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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-793500848
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2887
----------------------------------------------------------------
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-793475046
@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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-794808420
<b>[S] Trillian test result (tid-64)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37730 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4758-t64-vmware-67u3.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` | 786.77 | 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] blueorangutan commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-808444128
<b>Trillian test result (tid-271)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38541 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4758-t271-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Smoke tests completed. 84 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 777.03 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 79.97 | test_kubernetes_clusters.py
ContextSuite context=TestVAppsVM>:setup | `Error` | 44.31 | 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] nvazquez commented on a change in pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r603218609
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -1898,10 +1900,14 @@ protected StartAnswer execute(StartCommand cmd) {
}
}
tearDownVm(vmMo);
- } else if (!hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
- vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
- controllerInfo, systemVm)) {
- throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ } else {
+ boolean blankVmCreated = hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
+ vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
+ controllerInfo, systemVm);
+ if (!blankVmCreated) {
+ throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ }
+ deployAsIs = false;
Review comment:
Yes - as long a new system VM template is not registered it uses the non-deploy-as-is behaviour
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-820195722
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 415
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r613802095
##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -2294,43 +2294,6 @@ private void markVolumesInPool(VMInstanceVO vm, StoragePool destPool, Answer[] h
}
}
- private Pair<Long, Long> findClusterAndHostIdForVm(VMInstanceVO vm) {
- Long hostId = vm.getHostId();
- Long clusterId = null;
- // OfflineVmwareMigration: probably this is null when vm is stopped
- if(hostId == null) {
- hostId = vm.getLastHostId();
- if (s_logger.isDebugEnabled()) {
- s_logger.debug(String.format("host id is null, using last host id %d", hostId) );
- }
- }
- if (hostId == null) {
- List<VolumeVO> volumes = _volsDao.findByInstanceAndType(vm.getId(), Type.ROOT);
- if (CollectionUtils.isNotEmpty(volumes)) {
- for (VolumeVO rootVolume : volumes) {
- if (rootVolume.getPoolId() != null) {
- StoragePoolVO pool = _storagePoolDao.findById(rootVolume.getPoolId());
- if (pool != null && pool.getClusterId() != null) {
- clusterId = pool.getClusterId();
- List<HostVO> hosts = _hostDao.findHypervisorHostInCluster(pool.getClusterId());
- if (CollectionUtils.isNotEmpty(hosts)) {
- hostId = hosts.get(0).getId();
- break;
- }
- }
- }
- }
- }
- }
- if (clusterId == null && hostId != null) {
- HostVO host = _hostDao.findById(hostId);
- if (host != null) {
- clusterId = host.getClusterId();
- }
- }
- return new Pair<>(clusterId, hostId);
- }
-
private void migrateThroughHypervisorOrStorage(StoragePool destPool, VMInstanceVO vm) throws StorageUnavailableException, InsufficientCapacityException {
final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm);
Pair<Long, Long> vmClusterAndHost = findClusterAndHostIdForVm(vm);
Review comment:
Uses private method
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-820169461
@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] nvazquez commented on a change in pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r598406184
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -1898,10 +1900,14 @@ protected StartAnswer execute(StartCommand cmd) {
}
}
tearDownVm(vmMo);
- } else if (!hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
- vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
- controllerInfo, systemVm)) {
- throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ } else {
+ boolean blankVmCreated = hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
+ vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
+ controllerInfo, systemVm);
+ if (!blankVmCreated) {
+ throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ }
+ deployAsIs = false;
Review comment:
Can you please include a log line explaining why not deploying-as-is in this case? Otherwise will look confusing as the StartCommand may receive deployAsIs = true and not honouring 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-794865923
<b>Trillian test result (tid-3683)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39053 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4758-t3683-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Smoke tests completed. 84 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 774.83 | test_kubernetes_clusters.py
test_03_migrate_detached_volume | `Error` | 74.32 | test_vm_life_cycle.py
ContextSuite context=TestVAppsVM>:setup | `Error` | 44.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] shwstppr commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-817347414
@blueorangutan test centos7 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-825732981
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 455
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-809753096
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r603931838
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -1898,10 +1900,14 @@ protected StartAnswer execute(StartCommand cmd) {
}
}
tearDownVm(vmMo);
- } else if (!hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
- vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
- controllerInfo, systemVm)) {
- throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ } else {
+ boolean blankVmCreated = hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
+ vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
+ controllerInfo, systemVm);
+ if (!blankVmCreated) {
+ throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ }
+ deployAsIs = false;
Review comment:
@shwstppr possible to consider user input through some strictness flag for deployAsIs, and then fallback to not deploying-as-is case if strictness flag is false? (so that user will be aware of it already)
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-818962229
@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] nvazquez commented on a change in pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r612876821
##########
File path: plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java
##########
@@ -210,7 +173,7 @@ private VirtualMachine getVolumeVm(DataObject srcData) {
private Pair<Long, String> getHostIdForVmAndHostGuidInTargetClusterForAttachedVm(VirtualMachine vm,
StoragePool targetPool,
ScopeType targetScopeType) {
- Pair<Long, Long> clusterAndHostId = findClusterAndHostIdForVm(vm);
+ Pair<Long, Long> clusterAndHostId = vmManager.findClusterAndHostIdForVm(vm);
Review comment:
With the above comment, this should be passing vm.getId() instead
##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -2294,43 +2294,6 @@ private void markVolumesInPool(VMInstanceVO vm, StoragePool destPool, Answer[] h
}
}
- private Pair<Long, Long> findClusterAndHostIdForVm(VMInstanceVO vm) {
- Long hostId = vm.getHostId();
- Long clusterId = null;
- // OfflineVmwareMigration: probably this is null when vm is stopped
- if(hostId == null) {
- hostId = vm.getLastHostId();
- if (s_logger.isDebugEnabled()) {
- s_logger.debug(String.format("host id is null, using last host id %d", hostId) );
- }
- }
- if (hostId == null) {
- List<VolumeVO> volumes = _volsDao.findByInstanceAndType(vm.getId(), Type.ROOT);
- if (CollectionUtils.isNotEmpty(volumes)) {
- for (VolumeVO rootVolume : volumes) {
- if (rootVolume.getPoolId() != null) {
- StoragePoolVO pool = _storagePoolDao.findById(rootVolume.getPoolId());
- if (pool != null && pool.getClusterId() != null) {
- clusterId = pool.getClusterId();
- List<HostVO> hosts = _hostDao.findHypervisorHostInCluster(pool.getClusterId());
- if (CollectionUtils.isNotEmpty(hosts)) {
- hostId = hosts.get(0).getId();
- break;
- }
- }
- }
- }
- }
- }
- if (clusterId == null && hostId != null) {
- HostVO host = _hostDao.findById(hostId);
- if (host != null) {
- clusterId = host.getClusterId();
- }
- }
- return new Pair<>(clusterId, hostId);
- }
-
private void migrateThroughHypervisorOrStorage(StoragePool destPool, VMInstanceVO vm) throws StorageUnavailableException, InsufficientCapacityException {
final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm);
Pair<Long, Long> vmClusterAndHost = findClusterAndHostIdForVm(vm);
Review comment:
With the above comment, this should be receiving vm.getId() instead
##########
File path: engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java
##########
@@ -255,4 +256,10 @@ static String getHypervisorHostname(String name) {
boolean unmanage(String vmUuid);
UserVm restoreVirtualMachine(long vmId, Long newTemplateId) throws ResourceUnavailableException, InsufficientCapacityException;
+
+ Pair<Long, Long> findClusterAndHostIdForVmFromVolumes(long vmId);
+
+ Pair<Long, Long> findClusterAndHostIdForVm(VirtualMachine vm);
+
+ Pair<Long, Long> findClusterAndHostIdForVm(long vmId);
Review comment:
As methods are related, why not leaving only this one and make the first two methods as private on the implementation?
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-793983683
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 60
----------------------------------------------------------------
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-818338895
<b>Trillian test result (tid-415)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43541 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4758-t415-vmware-67u3.zip
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_vpc_vpn.py
Smoke tests completed. 85 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
ContextSuite context=TestVAppsVM>:setup | `Error` | 79.82 | test_vm_life_cycle.py
test_01_vpc_site2site_vpn_multiple_options | `Failure` | 348.03 | 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-794189964
@blueorangutan test centos7 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-817347169
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 383
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-815124356
<b>Trillian test result (tid-372)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 23167 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4758-t372-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.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. 79 look OK, 8 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_deploy_kubernetes_cluster | `Failure` | 112.68 | test_kubernetes_clusters.py
test_02_invalid_upgrade_kubernetes_cluster | `Failure` | 120.00 | test_kubernetes_clusters.py
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 67.70 | test_kubernetes_clusters.py
test_04_deploy_and_scale_kubernetes_cluster | `Failure` | 68.72 | test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster | `Failure` | 69.76 | test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster | `Failure` | 68.61 | test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 63.64 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 66.66 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 105.93 | test_kubernetes_clusters.py
ContextSuite context=TestDeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py
test_01_migrate_VM_and_root_volume | `Error` | 1.21 | test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks | `Error` | 1.13 | test_vm_life_cycle.py
test_03_migrate_detached_volume | `Error` | 1.12 | test_vm_life_cycle.py
test_01_unmanage_vm_cycle | `Error` | 1.16 | test_vm_life_cycle.py
test_01_unmanage_vm_cycle | `Error` | 2.23 | test_vm_life_cycle.py
ContextSuite context=TestUnmanageVM>:teardown | `Error` | 3.31 | test_vm_life_cycle.py
ContextSuite context=TestVAppsVM>:setup | `Error` | 3.71 | test_vm_life_cycle.py
ContextSuite context=TestVMLifeCycle>:setup | `Error` | 5.41 | test_vm_life_cycle.py
test_change_service_offering_for_vm_with_snapshots | `Error` | 1.18 | test_vm_snapshots.py
ContextSuite context=TestVmSnapshot>:setup | `Error` | 6.98 | test_vm_snapshots.py
ContextSuite context=TestCreateVolume>:setup | `Error` | 0.00 | test_volumes.py
ContextSuite context=TestVolumes>:setup | `Error` | 0.00 | test_volumes.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 7.68 | test_vpc_redundant.py
test_02_redundant_VPC_default_routes | `Error` | 8.70 | test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Error` | 7.71 | test_vpc_redundant.py
test_04_rvpc_network_garbage_collector_nics | `Error` | 7.69 | test_vpc_redundant.py
test_05_rvpc_multi_tiers | `Error` | 7.67 | test_vpc_redundant.py
test_01_VPC_nics_after_destroy | `Error` | 4.62 | test_vpc_router_nics.py
test_02_VPC_default_routes | `Error` | 5.65 | test_vpc_router_nics.py
test_01_redundant_vpc_site2site_vpn | `Failure` | 6.32 | test_vpc_vpn.py
test_01_vpc_site2site_vpn_multiple_options | `Failure` | 5.30 | test_vpc_vpn.py
test_01_vpc_remote_access_vpn | `Failure` | 3.15 | test_vpc_vpn.py
test_01_vpc_site2site_vpn | `Failure` | 4.27 | test_vpc_vpn.py
test_01_cancel_host_maintenace_with_no_migration_jobs | `Error` | 0.04 | test_host_maintenance.py
test_02_cancel_host_maintenace_with_migration_jobs | `Error` | 0.04 | test_host_maintenance.py
test_03_cancel_host_maintenace_with_migration_jobs_failure | `Error` | 0.04 | test_host_maintenance.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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-815456005
<b>Trillian test result (tid-377)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35474 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4758-t377-vmware-67u3.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_vm_life_cycle.py
Smoke tests completed. 84 look OK, 3 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_10_traceroute_in_vr | `Failure` | 61.18 | test_diagnostics.py
test_01_deploy_kubernetes_cluster | `Failure` | 84.24 | test_kubernetes_clusters.py
test_02_invalid_upgrade_kubernetes_cluster | `Failure` | 58.46 | test_kubernetes_clusters.py
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 96.17 | test_kubernetes_clusters.py
test_04_deploy_and_scale_kubernetes_cluster | `Failure` | 60.34 | test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster | `Failure` | 57.32 | test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster | `Failure` | 66.41 | test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 64.45 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 63.41 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 102.75 | test_kubernetes_clusters.py
ContextSuite context=TestVAppsVM>:setup | `Error` | 44.49 | 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-814835711
@shwstppr a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-810747055
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-817758123
@shwstppr a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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] shwstppr commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-795046005
`test_03_migrate_detached_volume` failure related to https://github.com/apache/cloudstack/pull/4785
----------------------------------------------------------------
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-814697564
@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] shwstppr commented on a change in pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r611222986
##########
File path: plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java
##########
@@ -138,11 +144,83 @@ private boolean isOnVmware(DataObject srcData, DataObject destData) {
&& HypervisorType.VMware.equals(destData.getTO().getHypervisorType());
}
- private Pair<Long, String> getHostIdForVmAndHostGuidInTargetCluster(DataObject srcData, DataObject destData) {
- StoragePool sourcePool = (StoragePool) srcData.getDataStore();
- ScopeType sourceScopeType = srcData.getDataStore().getScope().getScopeType();
- StoragePool targetPool = (StoragePool) destData.getDataStore();
- ScopeType targetScopeType = destData.getDataStore().getScope().getScopeType();
+ private Pair<Long, Long> findClusterAndHostIdForVm(VirtualMachine vm) {
+ Long hostId = vm.getHostId();
+ Long clusterId = null;
+ // OfflineVmwareMigration: probably this is null when vm is stopped
+ if(hostId == null) {
+ hostId = vm.getLastHostId();
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug(String.format("host id is null, using last host id %d", hostId) );
+ }
+ }
+ if (hostId == null) {
+ List<VolumeVO> volumes = volDao.findByInstanceAndType(vm.getId(), Volume.Type.ROOT);
+ if (CollectionUtils.isNotEmpty(volumes)) {
+ for (VolumeVO rootVolume : volumes) {
+ if (rootVolume.getPoolId() != null) {
+ StoragePoolVO pool = storagePoolDao.findById(rootVolume.getPoolId());
+ if (pool != null && pool.getClusterId() != null) {
+ clusterId = pool.getClusterId();
+ List<HostVO> hosts = hostDao.findHypervisorHostInCluster(pool.getClusterId());
+ if (CollectionUtils.isNotEmpty(hosts)) {
+ hostId = hosts.get(0).getId();
+ break;
+ }
+ }
+ }
+ }
+ }
+ }
+ if (clusterId == null && hostId != null) {
+ HostVO host = hostDao.findById(hostId);
+ if (host != null) {
+ clusterId = host.getClusterId();
+ }
+ }
+ return new Pair<>(clusterId, hostId);
+ }
Review comment:
@rhtyd @DaanHoogland @Pearl1594 @harikrishna-patnala I want to move this method to some common class where it can be accessed by:
1. `engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java`
2. `plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java`
3. `plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java`
Any suggestions to which class?
--
This is an automated message from the 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 a change in pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r598406184
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -1898,10 +1900,14 @@ protected StartAnswer execute(StartCommand cmd) {
}
}
tearDownVm(vmMo);
- } else if (!hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
- vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
- controllerInfo, systemVm)) {
- throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ } else {
+ boolean blankVmCreated = hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
+ vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
+ controllerInfo, systemVm);
+ if (!blankVmCreated) {
+ throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ }
+ deployAsIs = false;
Review comment:
Can you please include a log line explaining why not deploying-as-is in this case? Otherwise will look confusion as the StartCommand may receive deployAsIs = true and not honouring 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] rhtyd commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-818704143
@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] nvazquez commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-809752762
@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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r611396156
##########
File path: plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java
##########
@@ -138,11 +144,83 @@ private boolean isOnVmware(DataObject srcData, DataObject destData) {
&& HypervisorType.VMware.equals(destData.getTO().getHypervisorType());
}
- private Pair<Long, String> getHostIdForVmAndHostGuidInTargetCluster(DataObject srcData, DataObject destData) {
- StoragePool sourcePool = (StoragePool) srcData.getDataStore();
- ScopeType sourceScopeType = srcData.getDataStore().getScope().getScopeType();
- StoragePool targetPool = (StoragePool) destData.getDataStore();
- ScopeType targetScopeType = destData.getDataStore().getScope().getScopeType();
+ private Pair<Long, Long> findClusterAndHostIdForVm(VirtualMachine vm) {
+ Long hostId = vm.getHostId();
+ Long clusterId = null;
+ // OfflineVmwareMigration: probably this is null when vm is stopped
+ if(hostId == null) {
+ hostId = vm.getLastHostId();
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug(String.format("host id is null, using last host id %d", hostId) );
+ }
+ }
+ if (hostId == null) {
+ List<VolumeVO> volumes = volDao.findByInstanceAndType(vm.getId(), Volume.Type.ROOT);
+ if (CollectionUtils.isNotEmpty(volumes)) {
+ for (VolumeVO rootVolume : volumes) {
+ if (rootVolume.getPoolId() != null) {
+ StoragePoolVO pool = storagePoolDao.findById(rootVolume.getPoolId());
+ if (pool != null && pool.getClusterId() != null) {
+ clusterId = pool.getClusterId();
+ List<HostVO> hosts = hostDao.findHypervisorHostInCluster(pool.getClusterId());
+ if (CollectionUtils.isNotEmpty(hosts)) {
+ hostId = hosts.get(0).getId();
+ break;
+ }
+ }
+ }
+ }
+ }
+ }
+ if (clusterId == null && hostId != null) {
+ HostVO host = hostDao.findById(hostId);
+ if (host != null) {
+ clusterId = host.getClusterId();
+ }
+ }
+ return new Pair<>(clusterId, hostId);
+ }
Review comment:
thanks @harikrishna-patnala. 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] shwstppr commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-825715702
@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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r590338661
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
##########
@@ -2683,15 +2683,14 @@ public Answer deleteVolume(DeleteCommand cmd) {
List<VirtualDisk> virtualDisks = vmMo.getVirtualDisks();
List<String> managedDatastoreNames = getManagedDatastoreNamesFromVirtualDisks(virtualDisks);
+ // Preserve other disks of the VM
Review comment:
@harikrishna-patnala I've verified the destroy case and have seen any issue.
Will try to share API responses if needed
----------------------------------------------------------------
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-814835349
@blueorangutan test centos7 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] rhtyd commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-823837274
@borisstoyanov @nvazquez are you lgtm on this?
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-823983397
Yes, LGTM after various rounds of review and manual 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] blueorangutan commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-814715268
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 344
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-821061560
@blueorangutan test centos7 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] harikrishna-patnala commented on a change in pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r611354479
##########
File path: plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java
##########
@@ -138,11 +144,83 @@ private boolean isOnVmware(DataObject srcData, DataObject destData) {
&& HypervisorType.VMware.equals(destData.getTO().getHypervisorType());
}
- private Pair<Long, String> getHostIdForVmAndHostGuidInTargetCluster(DataObject srcData, DataObject destData) {
- StoragePool sourcePool = (StoragePool) srcData.getDataStore();
- ScopeType sourceScopeType = srcData.getDataStore().getScope().getScopeType();
- StoragePool targetPool = (StoragePool) destData.getDataStore();
- ScopeType targetScopeType = destData.getDataStore().getScope().getScopeType();
+ private Pair<Long, Long> findClusterAndHostIdForVm(VirtualMachine vm) {
+ Long hostId = vm.getHostId();
+ Long clusterId = null;
+ // OfflineVmwareMigration: probably this is null when vm is stopped
+ if(hostId == null) {
+ hostId = vm.getLastHostId();
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug(String.format("host id is null, using last host id %d", hostId) );
+ }
+ }
+ if (hostId == null) {
+ List<VolumeVO> volumes = volDao.findByInstanceAndType(vm.getId(), Volume.Type.ROOT);
+ if (CollectionUtils.isNotEmpty(volumes)) {
+ for (VolumeVO rootVolume : volumes) {
+ if (rootVolume.getPoolId() != null) {
+ StoragePoolVO pool = storagePoolDao.findById(rootVolume.getPoolId());
+ if (pool != null && pool.getClusterId() != null) {
+ clusterId = pool.getClusterId();
+ List<HostVO> hosts = hostDao.findHypervisorHostInCluster(pool.getClusterId());
+ if (CollectionUtils.isNotEmpty(hosts)) {
+ hostId = hosts.get(0).getId();
+ break;
+ }
+ }
+ }
+ }
+ }
+ }
+ if (clusterId == null && hostId != null) {
+ HostVO host = hostDao.findById(hostId);
+ if (host != null) {
+ clusterId = host.getClusterId();
+ }
+ }
+ return new Pair<>(clusterId, hostId);
+ }
Review comment:
@shwstppr I think virtualMachineManagerImpl.java is the better place since I don't see any hypervisor specific code here and this can be useful later for other usecases too.
--
This is an automated message from the 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 a change in pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r598406184
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -1898,10 +1900,14 @@ protected StartAnswer execute(StartCommand cmd) {
}
}
tearDownVm(vmMo);
- } else if (!hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
- vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
- controllerInfo, systemVm)) {
- throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ } else {
+ boolean blankVmCreated = hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
+ vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
+ controllerInfo, systemVm);
+ if (!blankVmCreated) {
+ throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ }
+ deployAsIs = false;
Review comment:
Can you please include a log line explaining why not deploying-as-is in this case? Otherwise will look confusing as the StartCommand may receive `deployAsIs = true` and not honouring 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] harikrishna-patnala commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-815664756
In my understanding the concern is about inter cluster migration.
So if we use migrateVolumeAPI and if there is a possibility that disks of a stopped VM can land on different storage pools which are different clusters then we should not allow that operation.
I agree with @shwstppr on this to disallow the API for inter-cluster migration.
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-814697258
@blueorangutan package
@nvazquez @harikrishna-patnala I've made changes to use relocateVM API for migrating attached volumes of a stopped VM. Please review.
Inter-cluster migration, in this case, won't be allowed (part overlaps with #4895)
In the case of inter-cluster migration we shouldn't allow migration using migrateVolume API when VM has more than one disk (rather ask user to use migrateVirtualMachine API?)
cc @rhtyd
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-796554387
@nvazquez @harikrishna-patnala can you review this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd merged pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-815590558
@shwstppr looks like in UI we do support migration of stopped VM (https://github.com/apache/cloudstack/blob/master/ui/src/config/section/compute.js#L298), so I think we should allow migration of stopped VM, i.e. all disks. The migrateVolume API will do this for individual disk, so it's fundamentally different from migrating the entire VM with all its disks. If the issue is merge conflict, you may meld both your PRs together. Let's see what others have to say - @harikrishna-patnala @nvazquez @andrijapanicsb
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-817568936
@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] shwstppr commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-817344035
@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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-817588052
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 392
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-826019266
@shwstppr a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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] shwstppr closed pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr closed pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-808014269
@blueorangutan test centos7 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-818991060
@shwstppr a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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] shwstppr commented on a change in pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r602076034
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -1898,10 +1900,14 @@ protected StartAnswer execute(StartCommand cmd) {
}
}
tearDownVm(vmMo);
- } else if (!hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
- vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
- controllerInfo, systemVm)) {
- throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ } else {
+ boolean blankVmCreated = hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
+ vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
+ controllerInfo, systemVm);
+ if (!blankVmCreated) {
+ throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ }
+ deployAsIs = false;
Review comment:
@nvazquez I've added a log message. Please see if there needs to be any change in the text.
Also, I've found that systemvms use the same flow with the default systemvm template, i.e., register a blank VM and then start it. Though deployAsIs was already `false` in that case.
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-826019172
@blueorangutan test centos7 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] shwstppr commented on a change in pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r613801929
##########
File path: plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java
##########
@@ -210,7 +173,7 @@ private VirtualMachine getVolumeVm(DataObject srcData) {
private Pair<Long, String> getHostIdForVmAndHostGuidInTargetClusterForAttachedVm(VirtualMachine vm,
StoragePool targetPool,
ScopeType targetScopeType) {
- Pair<Long, Long> clusterAndHostId = findClusterAndHostIdForVm(vm);
+ Pair<Long, Long> clusterAndHostId = vmManager.findClusterAndHostIdForVm(vm);
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-815159498
@blueorangutan test centos7 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-826082608
<b>Trillian test result (tid-514)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33934 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4758-t514-vmware-67u3.zip
Smoke tests completed. 87 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-811771029
Moved to draft for rework
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-809560989
@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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-821538026
<b>Trillian test result (tid-469)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36360 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4758-t469-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Smoke tests completed. 86 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_ping_in_vr_success | `Failure` | 15.33 | test_diagnostics.py
test_03_ping_in_ssvm_success | `Failure` | 15.34 | test_diagnostics.py
test_05_ping_in_cpvm_success | `Failure` | 15.34 | test_diagnostics.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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-818990626
@blueorangutan test centos7 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] shwstppr commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-817757776
@blueorangutan test centos7 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-809561964
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on a change in pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r605388641
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -1898,10 +1900,14 @@ protected StartAnswer execute(StartCommand cmd) {
}
}
tearDownVm(vmMo);
- } else if (!hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
- vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
- controllerInfo, systemVm)) {
- throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ } else {
+ boolean blankVmCreated = hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
+ vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
+ controllerInfo, systemVm);
+ if (!blankVmCreated) {
+ throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ }
+ deployAsIs = false;
Review comment:
Thanks for pointing it out @harikrishna-patnala . There seem an issue vApp appliances. Re-working on 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] harikrishna-patnala commented on a change in pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r605386577
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -1898,10 +1900,14 @@ protected StartAnswer execute(StartCommand cmd) {
}
}
tearDownVm(vmMo);
- } else if (!hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
- vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
- controllerInfo, systemVm)) {
- throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ } else {
+ boolean blankVmCreated = hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
+ vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
+ controllerInfo, systemVm);
+ if (!blankVmCreated) {
+ throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ }
+ deployAsIs = false;
Review comment:
@shwstppr, a small concern here. Since we are starting the VM as not deployAsIs for a VM which is actually a deployAsIs, will there be any problem while applying vApp properties as it is. Can you please test that.
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-807970158
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 264
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-817464925
<b>Trillian test result (tid-405)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35379 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4758-t405-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_root_resize.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Smoke tests completed. 86 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
ContextSuite context=TestVAppsVM>:setup | `Error` | 81.64 | 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-818718928
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 406
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-808014729
@shwstppr a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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] shwstppr commented on a change in pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r613804608
##########
File path: engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java
##########
@@ -255,4 +256,10 @@ static String getHypervisorHostname(String name) {
boolean unmanage(String vmUuid);
UserVm restoreVirtualMachine(long vmId, Long newTemplateId) throws ResourceUnavailableException, InsufficientCapacityException;
+
+ Pair<Long, Long> findClusterAndHostIdForVmFromVolumes(long vmId);
+
+ Pair<Long, Long> findClusterAndHostIdForVm(VirtualMachine vm);
+
+ Pair<Long, Long> findClusterAndHostIdForVm(long vmId);
Review comment:
Made other methods private
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r588150811
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
##########
@@ -2683,15 +2683,14 @@ public Answer deleteVolume(DeleteCommand cmd) {
List<VirtualDisk> virtualDisks = vmMo.getVirtualDisks();
List<String> managedDatastoreNames = getManagedDatastoreNamesFromVirtualDisks(virtualDisks);
+ // Preserve other disks of the VM
Review comment:
Thanks for these changes @shwstppr.
Can you please test destroy VM and expunge VM cases with VM having disks. Because this particular block is removed to address disk detachments in case of destroy VM. If those operations are not effected, this looks good to me.
----------------------------------------------------------------
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#discussion_r603931838
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -1898,10 +1900,14 @@ protected StartAnswer execute(StartCommand cmd) {
}
}
tearDownVm(vmMo);
- } else if (!hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
- vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
- controllerInfo, systemVm)) {
- throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ } else {
+ boolean blankVmCreated = hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec),
+ vmSpec.getLimitCpuUse(), (int) (vmSpec.getMaxRam() / ResourceType.bytesToMiB), getReservedMemoryMb(vmSpec), guestOsId, rootDiskDataStoreDetails.first(), false,
+ controllerInfo, systemVm);
+ if (!blankVmCreated) {
+ throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
+ }
+ deployAsIs = false;
Review comment:
@shwstppr possible to consider user input through some strictness flag for deployAsIs, and then fallback to not deploying-as-is case if strictness flag is false?
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-815160115
@shwstppr a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-819230772
<b>Trillian test result (tid-435)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34275 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4758-t435-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
Smoke tests completed. 86 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_10_traceroute_in_vr | `Failure` | 61.41 | test_diagnostics.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 merged pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-817347522
@shwstppr a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-818975759
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 408
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-793475618
@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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-817344160
@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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-794191329
@shwstppr a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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] shwstppr removed a comment on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr removed a comment on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-825715702
@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 pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-820168999
@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 pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-818961906
@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 pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-817568699
@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 closed pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr closed pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758
--
This is an automated message from the 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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-825718936
@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 #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-821061987
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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] shwstppr commented on pull request #4758: vmware: fix stopped VM volume migration
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4758:
URL: https://github.com/apache/cloudstack/pull/4758#issuecomment-825718409
@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