You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by likitha <gi...@git.apache.org> on 2015/07/02 12:29:15 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-8609. [VMware] VM is not acces...
GitHub user likitha opened a pull request:
https://github.com/apache/cloudstack/pull/556
CLOUDSTACK-8609. [VMware] VM is not accessible after a migration acro…
…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.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/likitha/cloudstack CLOUDSTACK-8609
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/556.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #556
----
commit a97a402f304303af78552777d5fde56abab902e5
Author: Likitha Shetty <li...@citrix.com>
Date: 2015-05-15T08:44:37Z
CLOUDSTACK-8609. [VMware] VM is not accessible after a 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.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #556: CLOUDSTACK-8609. [VMware] VM is not accessible after ...
Posted by bvbharatk <gi...@git.apache.org>.
Github user bvbharatk commented on the issue:
https://github.com/apache/cloudstack/pull/556
### 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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8609. [VMware] VM is not acces...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/556#issuecomment-216186028
tag:vmware-pickup
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8609. [VMware] VM is not acces...
Posted by GabrielBrascher <gi...@git.apache.org>.
Github user GabrielBrascher commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/556#discussion_r53570662
--- Diff: vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java ---
@@ -880,6 +880,38 @@ else if (prop.getName().startsWith("value[")) {
return networks;
}
+ public List<DatastoreMO> getAllDatastores() throws Exception {
+ PropertySpec pSpec = new PropertySpec();
+ pSpec.setType("Datastore");
+ pSpec.getPathSet().add("name");
+
+ TraversalSpec vmDatastoreTraversal = new TraversalSpec();
+ vmDatastoreTraversal.setType("VirtualMachine");
+ vmDatastoreTraversal.setPath("datastore");
+ vmDatastoreTraversal.setName("vmDatastoreTraversal");
+
+ ObjectSpec oSpec = new ObjectSpec();
+ oSpec.setObj(_mor);
+ oSpec.setSkip(Boolean.TRUE);
+ oSpec.getSelectSet().add(vmDatastoreTraversal);
+
+ PropertyFilterSpec pfSpec = new PropertyFilterSpec();
+ pfSpec.getPropSet().add(pSpec);
+ pfSpec.getObjectSet().add(oSpec);
+ List<PropertyFilterSpec> pfSpecArr = new ArrayList<PropertyFilterSpec>();
+ pfSpecArr.add(pfSpec);
+
+ List<ObjectContent> ocs = _context.getService().retrieveProperties(_context.getPropertyCollector(), pfSpecArr);
+
+ List<DatastoreMO> datastores = new ArrayList<DatastoreMO>();
+ if (ocs != null && ocs.size() > 0) {
--- End diff --
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8609. [VMware] VM is not acces...
Posted by rodrigo93 <gi...@git.apache.org>.
Github user rodrigo93 commented on the pull request:
https://github.com/apache/cloudstack/pull/556#issuecomment-183909796
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._
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8609. [VMware] VM is not acces...
Posted by pedro-martins <gi...@git.apache.org>.
Github user pedro-martins commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/556#discussion_r51372149
--- Diff: plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java ---
@@ -1765,7 +1767,18 @@ protected StartAnswer execute(StartCommand cmd) {
// Since VM was successfully powered-on, if there was an existing VM in a different cluster that was unregistered, delete all the files associated with it.
if (existingVmName != null && existingVmFileLayout != null) {
- deleteUnregisteredVmFiles(existingVmFileLayout, dcMo, true);
+ List<String> skipDatastores = new ArrayList<String>();
+ List<String> vmDatastoreNames = new ArrayList<String>();
+ for (DatastoreMO vmDatastore : vmMo.getAllDatastores()) {
+ vmDatastoreNames.add(vmDatastore.getName());
+ }
+ // Don't delete files that are in a datastore that is being used by the new VM as well (zone-wide datastore).
+ for (DatastoreMO existingDatastore : existingDatastores) {
+ if (vmDatastoreNames.contains(existingDatastore.getName())) {
+ skipDatastores.add(existingDatastore.getName());
+ }
+ }
+ deleteUnregisteredVmFiles(existingVmFileLayout, dcMo, true, skipDatastores);
}
--- End diff --
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<String> skipDatastores, String name ) and returns skipDatastores == null || !skipDatastores.contains(name).
Ty.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8609. [VMware] VM is not acces...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/556#issuecomment-129227002
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 :-).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---