Skip to content
This repository has been archived by the owner on Nov 9, 2020. It is now read-only.

Supplying a fix for "#836: vm creation is failing on vsanDatastore" #839

Conversation

shuklanirdesh82
Copy link
Contributor

@shuklanirdesh82 shuklanirdesh82 commented Dec 22, 2016

Vm creation was not complaining for vmfs as it doesn't use osfs code path and fails only for vsan as it is objectBased FS and vm-name was appending twice hence the issue.

Testing Done: Yes (both on vsan and vmfs)

vmdk_ops_test.py tests are passing locally and able to create vm successfully.

Running unit tests in /tmp/vmdk_ops_unittest22238/vmdk_ops_test.py...
.................
----------------------------------------------------------------------
Ran 17 tests in 272.076s

OK

//CC @ashahi1 @shaominchen

# bare minimum VM shell, no disks. Feel free to edit
vmx_file = vim.vm.FileInfo(logDirectory=None,
snapshotDirectory=None,
suspendDirectory=None,
vmPathName=datastore_path)
vmPathName='[' + datastore + '] ')
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename create_vm param from datastore to vm_path and it will be good to go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, Thanks!

@shuklanirdesh82 shuklanirdesh82 force-pushed the vmCreationOnVsanFix.shuklanirdesh82 branch from 59381b4 to 697becf Compare December 22, 2016 22:28
…anDatastore"

Addressing Mark's comment (renaming method parameter to more meaningful name)

Correcting the lint errors
@shuklanirdesh82 shuklanirdesh82 force-pushed the vmCreationOnVsanFix.shuklanirdesh82 branch from 697becf to 8ef06a5 Compare December 22, 2016 22:51
Copy link
Contributor

@msterin msterin left a comment

Choose a reason for hiding this comment

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

please see comments

@@ -479,7 +476,7 @@ def setUp(self):
si = vmdk_ops.get_si()
error, self.vm = create_vm(si=si,
vm_name=self.vm_name,
datastore=self.datastore_name)
vm_path=self.datastore_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is still weird... why self.datastore_name actually keep the vm_path ?
I'll approve to unbock , but please investigate as a separate item

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually create_vm is internally prepares vm_path (vmHome) i.e. vm_path = datastore_name + vmDisplayName.

So I am going to rename it with 'datastore_name' with more meaningful name to reduce the confusion.

@shuklanirdesh82 shuklanirdesh82 merged commit 0e3b065 into vmware-archive:master Dec 23, 2016
@shuklanirdesh82 shuklanirdesh82 deleted the vmCreationOnVsanFix.shuklanirdesh82 branch December 23, 2016 18:08
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants