Repository: vmware/govmomi (go)

Issue:
[BUG] VirtualDisk's capacityInBytes not works for vm.AddDevice
**Describe the bug**
VirtualDisk's capacityInKB attribute is deprecated in vSphere API 5.5 but without this vm.AddDevice doesn't work

Disk didn't created.

**To Reproduce**
create new disk for vm with funciton like in govc(https://github.com/vmware/govmomi/blob/master/govc/vm/disk/create.go#L166)
but change string 166	disk.CapacityInBytes = int64(cmd.Bytes)

**Expected behavior**
disk created and added to VM

**Affected version**
	github.com/vmware/govmomi v0.27.1


Pull request:
fix: DiskFileOperation must consider both capacity fields
## Description

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

## Type of change

Please mark options that are relevant:

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to
  not work as expected)
- [ ] This change requires a documentation update
- [ ] Build related change

## How Has This Been Tested?

Added `TestDiskFileOperation`

## Checklist:

- [x] My code follows the CONTRIBUTION [guidelines](./../CONTRIBUTING.md) of
  this project
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged

Patch:
diff --git a/object/virtual_device_list.go b/object/virtual_device_list.go
index 39eae0d5..30bd1705 100644
--- a/object/virtual_device_list.go
+++ b/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)
diff --git a/object/virtual_machine.go b/object/virtual_machine.go
index 8c9a85a9..eeffc19f 100644
--- a/object/virtual_machine.go
+++ b/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)


Test patch:
diff --git a/object/virtual_machine_test.go b/object/virtual_machine_test.go
index e3a860d1..b1879f96 100644
--- a/object/virtual_machine_test.go
+++ b/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)
+	}
+}