-
Notifications
You must be signed in to change notification settings - Fork 920
govc: Add datastore.cp disk format and adapter options #3758
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dougm I added a comment for a minor nit, but otherwise LGTM.
Signed-off-by: Doug MacEachern <[email protected]>
Same '-a' and '-d' flags as datastore.disk.create Fixes vmware#3748 Signed-off-by: Doug MacEachern <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
if len(spec) != 0 { | ||
dstSpec = spec[0] | ||
} | ||
|
||
// types.VirtualDiskSpec=nil as it is not implemented by vCenter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticed my own comment here from ~7 years ago. And I'm not seeing a difference now with the spec in place, it seems to be ignored. The target disk is always the same format as the source, regardless of specifying a different -d
(type). For example copying from thin
to preallocated
results in thin
:
% govc datastore.mkdir disks
% govc datastore.disk.create -size 16G -d thin disks/thin-disk-src.vmdk
Creating [sharedVmfs-0] disks/thin-disk-src.vmdk...OK
% govc datastore.cp -d preallocated disks/thin-disk-src.vmdk disks/thick-disk-dst.vmdk
Copying [sharedVmfs-0] disks/thin-disk-src.vmdk to [sharedVmfs-0] disks/thick-disk-dst.vmdk...OK
% govc datastore.disk.info disks/thick-disk-dst.vmdk
Name: disks/thick-disk-dst.vmdk
Type: thin
I'll hold off merging for now, will take a closer look at some point too. cc @aklakina
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure that is the case?
For example in my use case, I upload a .vmdk to vsphere and then I use datastore.cp to copy it with auto type detection. The disk is created with starwind v2v as file type vsphere_ws_growable, which should be the thin provisioned. This is then uploaded to vsphere esxi host. (I know that esxi is not at all the same as a vsphere workstation, but because of security purposes, the machine running the starwind can not connect directly to esxi host, so it can not use the vsphere_esxi_growable output type.)
So the disk uploaded is thin provisioned (confirmed by size) but the result of cp is eager zeroed. Could the issue be somewhere else?
I haven't tried the changes yet. Will only have time at the start of next week.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've only tested source disk from datastore.disk.create
, not tried starwind. Ok, let us know how it goes when you are able to test. I'll still try to take a closer look too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've ran the test and can confirm that
govc datastore.cp <source> <dest>
creates an eager zeroed disk on this branch too.
Same '-a' and '-d' flags as datastore.disk.create
Fixes #3748
api: Add optional spec param to DatastoreFileManager.Copy