diff --git a/vsphere/internal/virtualdevice/virtual_machine_disk_subresource.go b/vsphere/internal/virtualdevice/virtual_machine_disk_subresource.go index 602a5d0bf..88c0a61d1 100644 --- a/vsphere/internal/virtualdevice/virtual_machine_disk_subresource.go +++ b/vsphere/internal/virtualdevice/virtual_machine_disk_subresource.go @@ -1094,7 +1094,7 @@ func ReadDiskAttrsForDataSource(l object.VirtualDeviceList, count int) ([]map[st if backing.ThinProvisioned != nil { thin = *backing.ThinProvisioned } - m["size"] = int(structure.ByteToGiB(disk.CapacityInBytes).(int64)) + m["size"] = diskCapacityInGiB(disk) m["eagerly_scrub"] = eager m["thin_provisioned"] = thin out = append(out, m) @@ -1175,9 +1175,13 @@ func (r *DiskSubresource) Read(l object.VirtualDeviceList) error { r.Set("disk_mode", b.DiskMode) r.Set("write_through", b.WriteThrough) - // Only use disk_sharing if we are on vSphere 6.0 and higher + // Only use disk_sharing if we are on vSphere 6.0 and higher. In addition, + // skip if the value is unset - this prevents spurious diffs during upgrade + // situations where the VM hardware version does not actually allow disk + // sharing. In this situation, the value will be blank, and setting it will + // actually result in an error. version := viapi.ParseVersionFromClient(r.client) - if version.Newer(viapi.VSphereVersion{Product: version.Product, Major: 6}) { + if version.Newer(viapi.VSphereVersion{Product: version.Product, Major: 6}) && b.Sharing != "" { r.Set("disk_sharing", b.Sharing) } @@ -1194,7 +1198,7 @@ func (r *DiskSubresource) Read(l object.VirtualDeviceList) error { return fmt.Errorf("could not parse path from filename: %s", b.FileName) } r.Set("path", dp.Path) - r.Set("size", structure.ByteToGiB(disk.CapacityInBytes)) + r.Set("size", diskCapacityInGiB(disk)) } if allocation := disk.StorageIOAllocation; allocation != nil { @@ -1876,3 +1880,18 @@ func diskUUIDMatch(device types.BaseVirtualDevice, uuid string) bool { } return true } + +// diskCapacityInGiB reports the supplied disk's capacity, by first checking +// CapacityInBytes, and then falling back to CapacityInKB if that value is +// unavailable. This helps correct some situations where the former value's +// data gets cleared, which seems to happen on upgrades. +func diskCapacityInGiB(disk *types.VirtualDisk) int { + if disk.CapacityInBytes > 0 { + return int(structure.ByteToGiB(disk.CapacityInBytes).(int64)) + } + log.Printf( + "[DEBUG] diskCapacityInGiB: capacityInBytes missing for for %s, falling back to capacityInKB", + object.VirtualDeviceList{}.Name(disk), + ) + return int(structure.ByteToGiB(disk.CapacityInKB * 1024).(int64)) +} diff --git a/vsphere/internal/virtualdevice/virtual_machine_disk_subresource_test.go b/vsphere/internal/virtualdevice/virtual_machine_disk_subresource_test.go new file mode 100644 index 000000000..d4661ef7c --- /dev/null +++ b/vsphere/internal/virtualdevice/virtual_machine_disk_subresource_test.go @@ -0,0 +1,39 @@ +package virtualdevice + +import ( + "testing" + + "github.com/vmware/govmomi/vim25/types" +) + +func TestDiskCapacityInGiB(t *testing.T) { + cases := []struct { + name string + subject *types.VirtualDisk + expected int + }{ + { + name: "capacityInBytes", + subject: &types.VirtualDisk{ + CapacityInBytes: 4294967296, + CapacityInKB: 4194304, + }, + expected: 4, + }, + { + name: "capacityInKB", + subject: &types.VirtualDisk{ + CapacityInKB: 4194304, + }, + expected: 4, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + actual := diskCapacityInGiB(tc.subject) + if tc.expected != actual { + t.Fatalf("expected %d, got %d", tc.expected, actual) + } + }) + } +}