Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+22 −3
Open

govc: Add datastore.cp disk format and adapter options #3758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c803ab8
api: Add optional spec param to DatastoreFileManager.Copy
dougm Apr 8, 2025
2c2d11c
govc: Add datastore.cp disk format and adapter options
dougm Apr 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion cli/datastore/cp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/vmware/govmomi/cli"
"github.com/vmware/govmomi/cli/flags"
"github.com/vmware/govmomi/object"
"github.com/vmware/govmomi/vim25/types"
)

type cp struct {
Expand All @@ -30,6 +31,7 @@ type target struct {

kind bool
force bool
spec types.VirtualDiskSpec
}

func (cmd *target) FileManager() (*object.DatastoreFileManager, error) {
Expand Down Expand Up @@ -71,6 +73,8 @@ func (cmd *target) Register(ctx context.Context, f *flag.FlagSet) {

f.BoolVar(&cmd.kind, "t", true, "Use file type to choose disk or file manager")
f.BoolVar(&cmd.force, "f", false, "If true, overwrite any identically named file at the destination")
f.StringVar(&cmd.spec.AdapterType, "a", string(types.VirtualDiskAdapterTypeLsiLogic), "Disk adapter")
f.StringVar(&cmd.spec.DiskType, "d", string(types.VirtualDiskTypeThin), "Disk format")
}

func (cmd *target) Process(ctx context.Context) error {
Expand Down Expand Up @@ -130,7 +134,9 @@ func (cmd *cp) Run(ctx context.Context, f *flag.FlagSet) error {

cp := m.CopyFile
if cmd.kind {
cp = m.Copy
cp = func(ctx context.Context, src string, dst string) error {
return m.Copy(ctx, src, dst, &cmd.spec)
}
}

logger := cmd.ProgressLogger(fmt.Sprintf("Copying %s to %s...", src, dst))
Expand Down
4 changes: 4 additions & 0 deletions govc/USAGE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1310,6 +1310,8 @@ Examples:
govc datastore.cp disks/disk1.vmdk -ds-target NFS-2 disks/disk2.vmdk

Options:
-a=lsiLogic Disk adapter
-d=thin Disk format
-dc-target= Datacenter destination (defaults to -dc)
-ds= Datastore [GOVC_DATASTORE]
-ds-target= Datastore destination (defaults to -ds)
Expand Down Expand Up @@ -1536,6 +1538,8 @@ Examples:
govc datastore.mv -f my.vmx foo/foo.vmx

Options:
-a=lsiLogic Disk adapter
-d=thin Disk format
-dc-target= Datacenter destination (defaults to -dc)
-ds= Datastore [GOVC_DATASTORE]
-ds-target= Datastore destination (defaults to -ds)
Expand Down
3 changes: 3 additions & 0 deletions govc/test/datastore.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,9 @@ upload_file() {
run govc datastore.rm -dc DC1 -ds LocalDS_1 "$clone"
assert_success
done

run govc datastore.cp -dc DC0 -ds LocalDS_0 -d preallocated "$vmdk" "$id/$id-thick.vmdk"
assert_success
}

@test "datastore.disk.info" {
Expand Down
10 changes: 8 additions & 2 deletions object/datastore_file_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

"github.com/vmware/govmomi/vim25/progress"
"github.com/vmware/govmomi/vim25/soap"
"github.com/vmware/govmomi/vim25/types"
)

// DatastoreFileManager combines FileManager and VirtualDiskManager to manage files on a Datastore
Expand Down Expand Up @@ -115,16 +116,21 @@ func (m *DatastoreFileManager) CopyFile(ctx context.Context, src string, dst str
}

// Copy dispatches to the appropriate FileManager or VirtualDiskManager Copy method based on file name extension
func (m *DatastoreFileManager) Copy(ctx context.Context, src string, dst string) error {
func (m *DatastoreFileManager) Copy(ctx context.Context, src string, dst string, spec ...types.BaseVirtualDiskSpec) error {
srcp := m.Path(src)
dstp := m.Path(dst)

f := m.FileManager.CopyDatastoreFile

if srcp.IsVMDK() {
var dstSpec types.BaseVirtualDiskSpec
if len(spec) != 0 {
dstSpec = spec[0]
}

// types.VirtualDiskSpec=nil as it is not implemented by vCenter
Copy link
Member Author

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

Copy link

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.

Copy link
Member Author

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.

Copy link

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.

Copy link
Member Author

@dougm dougm Apr 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is then uploaded to vsphere esxi host

How was it uploaded? scp or ?
Another option to try: https://github.com/vmware/govmomi/blob/main/govc/USAGE.md#importvmdk
This goes through the conversion process, from desktop (streamOptimized) format to thin. After that, you can use datastore.cp and target disks will be thin format.

Copy link

Choose a reason for hiding this comment

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

It was converted to vsphere workspace growable format and then I used the datastore.upload command.

Sadly the import.vmdk command needs direct access to the esxi hosts, which would result in some security valnurabilities in our system, so that is not an option.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could make import.vmdk work with Content Library, then upload would be to vCenter rather than ESX. When a vmdk is imported to CL, it will be converted, so datastore.cp target should also be thin.

Or if we added an option to import.{vmdk,ovf} to pullFromUrls , then vCenter would pull from the provided URL, rather than client uploading to ESX. Planning to do that for #3728

Copy link

Choose a reason for hiding this comment

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

That is a neat feature. Sadly it would complicate our workflow probably too much so I would prefer the first option.

f = func(ctx context.Context, src string, srcDC *Datacenter, dst string, dstDC *Datacenter, force bool) (*Task, error) {
return m.VirtualDiskManager.CopyVirtualDisk(ctx, src, srcDC, dst, dstDC, nil, force)
return m.VirtualDiskManager.CopyVirtualDisk(ctx, src, srcDC, dst, dstDC, dstSpec, force)
}
}

Expand Down
Loading