Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

r/virtual_machine: Address vCenter upgrade issues #405

Merged
merged 3 commits into from
Feb 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 23 additions & 4 deletions vsphere/internal/virtualdevice/virtual_machine_disk_subresource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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))
}
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}