fix: DiskFileOperation must consider both capacity fields

Config spec helpers have logic to clear the DiskFileOperation for existing disks,
when 'CapacityInKB' is '0'. But we also need to check 'Capacity'.

Refactor some duplicate code into a separate function.

Closes #2805

Author: dougm

object/virtual_device_list.go

@@ -939,25 +939,9 @@ func (l VirtualDeviceList) ConfigSpec(op types.VirtualDeviceConfigSpecOperation)
 	var res []types.BaseVirtualDeviceConfigSpec
 	for _, device := range l {
 		config := &types.VirtualDeviceConfigSpec{
-			Device:    device,
-			Operation: op,
-		}
-
-		if disk, ok := device.(*types.VirtualDisk); ok {
-			config.FileOperation = fop
-
-			// Special case to attach an existing disk
-			if op == types.VirtualDeviceConfigSpecOperationAdd && disk.CapacityInKB == 0 {
-				childDisk := false
-				if b, ok := disk.Backing.(*types.VirtualDiskFlatVer2BackingInfo); ok {
-					childDisk = b.Parent != nil
-				}
-
-				if !childDisk {
-					// Existing disk, clear file operation
-					config.FileOperation = ""
-				}
-			}
+			Device:        device,
+			Operation:     op,
+			FileOperation: diskFileOperation(op, fop, device),
 		}
 
 		res = append(res, config)

object/virtual_machine.go

@@ -461,29 +461,33 @@ func (v VirtualMachine) ResourcePool(ctx context.Context) (*ResourcePool, error)
 	return NewResourcePool(v.c, *rp), nil
 }
 
-func (v VirtualMachine) configureDevice(ctx context.Context, op types.VirtualDeviceConfigSpecOperation, fop types.VirtualDeviceConfigSpecFileOperation, devices ...types.BaseVirtualDevice) error {
-	spec := types.VirtualMachineConfigSpec{}
+func diskFileOperation(op types.VirtualDeviceConfigSpecOperation, fop types.VirtualDeviceConfigSpecFileOperation, device types.BaseVirtualDevice) types.VirtualDeviceConfigSpecFileOperation {
+	if disk, ok := device.(*types.VirtualDisk); ok {
+		// Special case to attach an existing disk
+		if op == types.VirtualDeviceConfigSpecOperationAdd && disk.CapacityInKB == 0 && disk.CapacityInBytes == 0 {
+			childDisk := false
+			if b, ok := disk.Backing.(*types.VirtualDiskFlatVer2BackingInfo); ok {
+				childDisk = b.Parent != nil
+			}
 
-	for _, device := range devices {
-		config := &types.VirtualDeviceConfigSpec{
-			Device:    device,
-			Operation: op,
+			if !childDisk {
+				fop = "" // existing disk
+			}
 		}
+		return fop
+	}
 
-		if disk, ok := device.(*types.VirtualDisk); ok {
-			config.FileOperation = fop
+	return ""
+}
 
-			// Special case to attach an existing disk
-			if op == types.VirtualDeviceConfigSpecOperationAdd && disk.CapacityInKB == 0 {
-				childDisk := false
-				if b, ok := disk.Backing.(*types.VirtualDiskFlatVer2BackingInfo); ok {
-					childDisk = b.Parent != nil
-				}
+func (v VirtualMachine) configureDevice(ctx context.Context, op types.VirtualDeviceConfigSpecOperation, fop types.VirtualDeviceConfigSpecFileOperation, devices ...types.BaseVirtualDevice) error {
+	spec := types.VirtualMachineConfigSpec{}
 
-				if !childDisk {
-					config.FileOperation = "" // existing disk
-				}
-			}
+	for _, device := range devices {
+		config := &types.VirtualDeviceConfigSpec{
+			Device:        device,
+			Operation:     op,
+			FileOperation: diskFileOperation(op, fop, device),
 		}
 
 		spec.DeviceChange = append(spec.DeviceChange, config)

object/virtual_machine_test.go

@@ -204,3 +204,52 @@ func TestVirtualMachineSnapshotMap(t *testing.T) {
 		}
 	}
 }
+
+func TestDiskFileOperation(t *testing.T) {
+	backing := &types.VirtualDiskFlatVer2BackingInfo{
+		VirtualDeviceFileBackingInfo: types.VirtualDeviceFileBackingInfo{
+			FileName: "[datastore1] data/disk1.vmdk",
+		},
+		Parent: nil,
+	}
+
+	parent := &types.VirtualDiskFlatVer2BackingInfo{
+		VirtualDeviceFileBackingInfo: types.VirtualDeviceFileBackingInfo{
+			FileName: "[datastore1] data/parent.vmdk",
+		},
+	}
+
+	disk := &types.VirtualDisk{
+		VirtualDevice: types.VirtualDevice{
+			Backing: backing,
+		},
+	}
+
+	op := types.VirtualDeviceConfigSpecOperationAdd
+	fop := types.VirtualDeviceConfigSpecFileOperationCreate
+
+	res := diskFileOperation(op, fop, disk)
+	if res != "" {
+		t.Errorf("res=%s", res)
+	}
+
+	disk.CapacityInKB = 1
+	res = diskFileOperation(op, fop, disk)
+	if res != types.VirtualDeviceConfigSpecFileOperationCreate {
+		t.Errorf("res=%s", res)
+	}
+
+	disk.CapacityInKB = 0
+	disk.CapacityInBytes = 1
+	res = diskFileOperation(op, fop, disk)
+	if res != types.VirtualDeviceConfigSpecFileOperationCreate {
+		t.Errorf("res=%s", res)
+	}
+
+	disk.CapacityInBytes = 0
+	backing.Parent = parent
+	res = diskFileOperation(op, fop, disk)
+	if res != types.VirtualDeviceConfigSpecFileOperationCreate {
+		t.Errorf("res=%s", res)
+	}
+}