Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLOUDSTACK-8609. [VMware] VM is not accessible after a migration acro… #556

Closed
wants to merge 1 commit into from

Conversation

likitha
Copy link

@likitha likitha commented Jul 2, 2015

…ss clusters.

Once a VM is successfully started, don't delete the files associated with the unregistered VM, if the files are in a storage that is being used by the new VM.
Attempt to unregister a VM in another DC, only if there is a host associated with a VM.

…ss clusters.

Once a VM is successfully started, don't delete the files associated with the unregistered VM, if the files are in a storage that is being used by the new VM.
Attempt to unregister a VM in another DC, only if there is a host associated with a VM.
@remibergsma
Copy link
Contributor

Who wants to step in and finish this work? It seems the original author is not able to finish it. If no one steps in, we'll have to close the PR without merging it so please help :-).

remibergsma added a commit to remibergsma/cloudstack that referenced this pull request Aug 17, 2015
This closes apache#577
This closes apache#566
This closes apache#562
This closes apache#561
This closes apache#556
This closes apache#555
This closes apache#554
This closes apache#548
This closes apache#544
This closes apache#540
This closes apache#508
This closes apache#384
This closes apache#372
remibergsma added a commit to remibergsma/cloudstack that referenced this pull request Aug 17, 2015
This closes apache#577
This closes apache#566
This closes apache#562
This closes apache#561
This closes apache#556
This closes apache#555
This closes apache#554
This closes apache#548
This closes apache#544
This closes apache#540
This closes apache#384
This closes apache#372
remibergsma added a commit to remibergsma/cloudstack that referenced this pull request Aug 17, 2015
This closes apache#577
This closes apache#566
This closes apache#562
This closes apache#561
This closes apache#556
This closes apache#555
This closes apache#554
This closes apache#548
This closes apache#544
This closes apache#540
This closes apache#384
This closes apache#372
skipDatastores.add(existingDatastore.getName());
}
}
deleteUnregisteredVmFiles(existingVmFileLayout, dcMo, true, skipDatastores);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, likitha.
Could you extract the code between lines 1770 - 1775 and 1776 - 1780 to methods with a little test case and Javadoc explaining what the methods do, the params that they use and what they return?

Another thing, how about you replace the logic in 'if's at lines 2272, 2283 and 2291 for a method with a little test case and a Javadoc, that receives 2 params, (List skipDatastores, String name ) and returns skipDatastores == null || !skipDatastores.contains(name).

Ty.

@rodrigo93
Copy link
Contributor

Could someone put this PR to test again? If it fails, it could be closed since the author seems to be inactive on github since September of the last year and the PR is very old.
If someone shows up some interest in continuing it, then it could be opened again.

PS: This is just a suggestion.

List<ObjectContent> ocs = _context.getService().retrieveProperties(_context.getPropertyCollector(), pfSpecArr);

List<DatastoreMO> datastores = new ArrayList<DatastoreMO>();
if (ocs != null && ocs.size() > 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @likitha, you could use CollectionUtils.isEmpty for this conditional.
It returns true if the List is empty or null.
Thanks.
Documentation: https://commons.apache.org/proper/commons-collections/apidocs/org/apache/commons/collections4/CollectionUtils.html#isEmpty%28java.util.Collection%29

@rohityadavcloud
Copy link
Member

tag:vmware-pickup

@bvbharatk
Copy link
Contributor

ACS CI BVT Run

Sumarry:
Build Number 137
Hypervisor xenserver
NetworkType Advanced
Passed=73
Failed=0
Skipped=3

Link to logs Folder (search by build_no): https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0

Failed tests:

Skipped tests:
test_vm_nic_adapter_vmxnet3
test_static_role_account_acls
test_deploy_vgpu_enabled_vm

Passed test suits:
test_deploy_vm_with_userdata.py
test_affinity_groups_projects.py
test_portable_publicip.py
test_vpc_vpn.py
test_over_provisioning.py
test_global_settings.py
test_scale_vm.py
test_service_offerings.py
test_routers_iptables_default_policy.py
test_routers.py
test_reset_vm_on_reboot.py
test_snapshots.py
test_deploy_vms_with_varied_deploymentplanners.py
test_login.py
test_list_ids_parameter.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_regions.py
test_affinity_groups.py
test_network_acl.py
test_pvlan.py
test_volumes.py
test_nic.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_vm_life_cycle.py
test_disk_offerings.py

@sureshanaparti
Copy link
Contributor

Continued with the code changes here in a new PR #2091.

@DaanHoogland
Copy link
Contributor

closing as @sureshanaparti tookk this to #2091

rohityadavcloud pushed a commit that referenced this pull request Aug 21, 2018
… clusters (#2091)

[VMware] VM is not accessible after migration across clusters.

Once a VM is successfully started, don't delete the files associated with the unregistered VM, if the files are in a storage that is being used by the new VM.
Attempt to unregister a VM in another DC, only if there is a host associated with a VM.

This closes #556
rohityadavcloud pushed a commit that referenced this pull request Jan 20, 2021
Add and update translations for German

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants