Merge pull request #2895 from Syuparn/issue-2894

vcsim: Fix datastore freespace changed by ReconfigVM_Task
vmware · Jul 13, 2022 · e0c2c7b · e0c2c7b
2 parents 67e845e + b44828a
commit e0c2c7b
Show file tree

Hide file tree

Showing 2 changed files with 130 additions and 5 deletions.
diff --git a/simulator/virtual_machine.go b/simulator/virtual_machine.go
@@ -1106,7 +1106,12 @@ func (vm *VirtualMachine) validateSwitchMembers(id string) types.BaseMethodFault
 	return nil
 }
 
-func (vm *VirtualMachine) configureDevice(ctx *Context, devices object.VirtualDeviceList, spec *types.VirtualDeviceConfigSpec) types.BaseMethodFault {
+func (vm *VirtualMachine) configureDevice(
+	ctx *Context,
+	devices object.VirtualDeviceList,
+	spec *types.VirtualDeviceConfigSpec,
+	oldDevice types.BaseVirtualDevice) types.BaseMethodFault {
+
 	device := spec.Device
 	d := device.GetVirtualDevice()
 	var controller types.BaseVirtualController
@@ -1220,6 +1225,17 @@ func (vm *VirtualMachine) configureDevice(ctx *Context, devices object.VirtualDe
 			ds := vm.findDatastore(p.Datastore)
 			info.Datastore = &ds.Self
 
+			if oldDevice != nil {
+				if oldDisk, ok := oldDevice.(*types.VirtualDisk); ok {
+					// add previous capacity to datastore freespace
+					ctx.WithLock(ds, func() {
+						ds.Summary.FreeSpace += getDiskSize(oldDisk)
+						ds.Info.GetDatastoreInfo().FreeSpace = ds.Summary.FreeSpace
+					})
+				}
+			}
+
+			// then subtract new capacity from datastore freespace
 			// XXX: compare disk size and free space until windows stat is supported
 			ctx.WithLock(ds, func() {
 				ds.Summary.FreeSpace -= getDiskSize(x)
@@ -1453,7 +1469,7 @@ func (vm *VirtualMachine) configureDevices(ctx *Context, spec *types.VirtualMach
 			}
 
 			key := device.Key
-			err := vm.configureDevice(ctx, devices, dspec)
+			err := vm.configureDevice(ctx, devices, dspec, nil)
 			if err != nil {
 				return err
 			}
@@ -1470,16 +1486,17 @@ func (vm *VirtualMachine) configureDevices(ctx *Context, spec *types.VirtualMach
 			}
 		case types.VirtualDeviceConfigSpecOperationEdit:
 			rspec := *dspec
-			rspec.Device = devices.FindByKey(device.Key)
-			if rspec.Device == nil {
+			oldDevice := devices.FindByKey(device.Key)
+			if oldDevice == nil {
 				return invalid
 			}
+			rspec.Device = oldDevice
 			devices = vm.removeDevice(ctx, devices, &rspec)
 			if device.DeviceInfo != nil {
 				device.DeviceInfo.GetDescription().Summary = "" // regenerate summary
 			}
 
-			err := vm.configureDevice(ctx, devices, dspec)
+			err := vm.configureDevice(ctx, devices, dspec, oldDevice)
 			if err != nil {
 				return err
 			}

diff --git a/simulator/virtual_machine_test.go b/simulator/virtual_machine_test.go
@@ -1302,6 +1302,114 @@ func TestReconfigureDevicesCapacity(t *testing.T) {
 	}
 }
 
+func TestReconfigureDevicesDatastoreFreespace(t *testing.T) {
+	tests := []struct {
+		name          string
+		reconfigure   func(context.Context, *object.VirtualMachine, *Datastore, object.VirtualDeviceList) error
+		freespaceDiff int64
+	}{
+		{
+			"create a new disk",
+			func(ctx context.Context, vm *object.VirtualMachine, ds *Datastore, l object.VirtualDeviceList) error {
+				controller, err := l.FindDiskController("")
+				if err != nil {
+					return err
+				}
+
+				disk := l.CreateDisk(controller, ds.Reference(), "")
+				disk.CapacityInBytes = 10 * 1024 * 1024 * 1024 // 10GB
+
+				if err := vm.AddDevice(ctx, disk); err != nil {
+					return err
+				}
+				return nil
+			},
+			-10 * 1024 * 1024 * 1024, // -10GB
+		},
+		{
+			"edit disk size",
+			func(ctx context.Context, vm *object.VirtualMachine, ds *Datastore, l object.VirtualDeviceList) error {
+				disks := l.SelectByType((*types.VirtualDisk)(nil))
+				if len(disks) == 0 {
+					return fmt.Errorf("disk not found")
+				}
+				disk := disks[len(disks)-1].(*types.VirtualDisk)
+
+				// specify same disk capacity
+				if err := vm.EditDevice(ctx, disk); err != nil {
+					return err
+				}
+				return nil
+			},
+			0,
+		},
+		{
+			"remove a disk and its files",
+			func(ctx context.Context, vm *object.VirtualMachine, ds *Datastore, l object.VirtualDeviceList) error {
+				disks := l.SelectByType((*types.VirtualDisk)(nil))
+				if len(disks) == 0 {
+					return fmt.Errorf("disk not found")
+				}
+				disk := disks[len(disks)-1].(*types.VirtualDisk)
+
+				if err := vm.RemoveDevice(ctx, false, disk); err != nil {
+					return err
+				}
+				return nil
+			},
+			10 * 1024 * 1024 * 1024, // 10GB
+		},
+		{
+			"remove a disk but keep its files",
+			func(ctx context.Context, vm *object.VirtualMachine, ds *Datastore, l object.VirtualDeviceList) error {
+				disks := l.SelectByType((*types.VirtualDisk)(nil))
+				if len(disks) == 0 {
+					return fmt.Errorf("disk not found")
+				}
+				disk := disks[len(disks)-1].(*types.VirtualDisk)
+
+				if err := vm.RemoveDevice(ctx, true, disk); err != nil {
+					return err
+				}
+				return nil
+			},
+			0,
+		},
+	}
+
+	for _, test := range tests {
+		test := test // assign to local var since loop var is reused
+		t.Run(test.name, func(t *testing.T) {
+			m := ESX()
+
+			Test(func(ctx context.Context, c *vim25.Client) {
+				vmm := Map.Any("VirtualMachine").(*VirtualMachine)
+				vm := object.NewVirtualMachine(c, vmm.Reference())
+
+				ds := Map.Any("Datastore").(*Datastore)
+				freespaceBefore := ds.Datastore.Summary.FreeSpace
+
+				devices, err := vm.Device(ctx)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				err = test.reconfigure(ctx, vm, ds, devices)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				freespaceAfter := ds.Datastore.Summary.FreeSpace
+
+				if freespaceAfter-freespaceBefore != test.freespaceDiff {
+					t.Errorf("difference of freespace expected %d, got %d",
+						test.freespaceDiff, freespaceAfter-freespaceBefore)
+				}
+			}, m)
+		})
+	}
+}
+
 func TestShutdownGuest(t *testing.T) {
 	// use the default vm for testing
 	ctx := context.Background()