Merge pull request #2814 from dougm/issue-2805

fix: DiskFileOperation must consider both capacity fields
vmware · Apr 25, 2022 · 919f21a · 919f21a
2 parents ccb9729 + 5ef4aaa
commit 919f21a
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 37 deletions.
diff --git 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
@@ -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)

diff --git 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)
+	}
+}