Skip to content

Conversation

@palnabarun
Copy link
Contributor

@palnabarun palnabarun commented Oct 30, 2025

Description

Previously, the UUID was always set to a new value even if it was already present. This patch ensures that the disk UUID is now set only if it is
currently empty in the backing information.

How Has This Been Tested?

Made changes to govmomi pulled by vmware-tanzu/vm-operator#1258 and tested whether the UUID if set, remains same. With this patch, the expected behavior is validated.

Adds an unit test to validate the contract.

❯ go test -v -timeout 30s -run ^TestVirtualDiskUUIDAssignment$ github.com/vmware/govmomi/simulator
=== RUN   TestVirtualDiskUUIDAssignment
=== RUN   TestVirtualDiskUUIDAssignment/empty_UUID
=== RUN   TestVirtualDiskUUIDAssignment/generated_UUID
=== RUN   TestVirtualDiskUUIDAssignment/explicit_UUID
--- PASS: TestVirtualDiskUUIDAssignment (0.44s)
    --- PASS: TestVirtualDiskUUIDAssignment/empty_UUID (0.16s)
    --- PASS: TestVirtualDiskUUIDAssignment/generated_UUID (0.05s)
    --- PASS: TestVirtualDiskUUIDAssignment/explicit_UUID (0.05s)
PASS
ok      github.com/vmware/govmomi/simulator     1.304s

Guidelines

Please read and follow the CONTRIBUTION guidelines of this project.

@palnabarun
Copy link
Contributor Author

Failing test

=== RUN   ExampleHandlerREST
--- FAIL: ExampleHandlerREST (2.22s)
got:
session valid=true
session valid=true
session valid=true
session valid=true
session valid=false
session valid=false
want:
session valid=true
session valid=false
session valid=true
session valid=true
session valid=false
session valid=false
FAIL
FAIL	github.com/vmware/govmomi/session/keepalive	6.102s

@palnabarun
Copy link
Contributor Author

Most likely a flake. Locally, the test passes.

~/s/v/govmomi on 🌱 vcsim/check-disk-backing-if-uuid-exists (1f70772) [📦] [💣 1]
❯ go test -v github.com/vmware/govmomi/session/keepalive
=== RUN   TestHandlerSOAP
--- PASS: TestHandlerSOAP (0.49s)
=== RUN   TestHandlerREST
--- PASS: TestHandlerREST (0.49s)
=== RUN   ExampleHandlerSOAP
--- PASS: ExampleHandlerSOAP (1.90s)
=== RUN   ExampleHandlerREST
--- PASS: ExampleHandlerREST (1.92s)
PASS
ok      github.com/vmware/govmomi/session/keepalive     5.234s

@palnabarun palnabarun force-pushed the vcsim/check-disk-backing-if-uuid-exists branch 2 times, most recently from 7f435bd to f03b66d Compare October 31, 2025 11:49
@palnabarun
Copy link
Contributor Author

palnabarun commented Oct 31, 2025

This is failing locally as well. Digging into it. Fixed

not ok 313 vm.clone
# (from function `assert_equal' in file ./test_helper.bash, line 232,
#  in test file ./vm.bats, line 939)
#   `assert_equal "$nitems" "$uitems"' failed
# expected: 9
# actual:   5

…issues

Changes Made:

1. Previously, the UUID was always set to a new value even if it was already
present. This patch ensures that the disk UUID is now set only if it is
currently empty in the backing information.

2. Also, fixes vdmCreateVirtualDisk to return descriptor for UUID assignment.
Returns the created vmdk.Descriptor from vdmCreateVirtualDisk instead of nil.
This allows the calling code in virtual_machine.go to properly set the UUID
on newly created disks.

3. Clear disk UUID during VM cloning to generate unique UUIDs per clone
When cloning a VM, the disk backing info including UUID was being deep copied
from the source VM. However, the FileName was cleared to generate a new disk
but the UUID was not cleared, leading to duplicate UUIDs across cloned VMs.

4.  Add test cases to verify the behavior.

Signed-off-by: Nabarun Pal <[email protected]>
@palnabarun palnabarun force-pushed the vcsim/check-disk-backing-if-uuid-exists branch from f03b66d to 27d3b54 Compare October 31, 2025 16:44
@akutz akutz merged commit d74e7b6 into vmware:main Oct 31, 2025
11 checks passed
@palnabarun palnabarun deleted the vcsim/check-disk-backing-if-uuid-exists branch November 3, 2025 06:23
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.

2 participants