Merge pull request #802 from jcvenegas/fix-memory-update

memory: update: Update state using the memory removed

virtcontainers/container.go

@@ -1301,9 +1301,13 @@ func (c *Container) memHotplugValid(mem uint32) (uint32, error) {
 	return uint32(math.Ceil(float64(mem)/float64(memorySectionSizeMB))) * memorySectionSizeMB, nil
 }
 
-func (c *Container) updateMemoryResources(oldResources, newResources ContainerResources) error {
+func (c *Container) updateMemoryResources(oldResources ContainerResources, newResources *ContainerResources) error {
 	oldMemMB := oldResources.MemMB
 	newMemMB := newResources.MemMB
+	c.Logger().WithFields(logrus.Fields{
+		"old-mem": fmt.Sprintf("%dMB", oldMemMB),
+		"new-mem": fmt.Sprintf("%dMB", newMemMB),
+	}).Debug("Request update memory")
 
 	if oldMemMB == newMemMB {
 		c.Logger().WithFields(logrus.Fields{
@@ -1325,16 +1329,37 @@ func (c *Container) updateMemoryResources(oldResources, newResources ContainerRe
 		addMemDevice := &memoryDevice{
 			sizeMB: int(memHotplugMB),
 		}
-		_, err = c.sandbox.hypervisor.hotplugAddDevice(addMemDevice, memoryDev)
+		data, err := c.sandbox.hypervisor.hotplugAddDevice(addMemDevice, memoryDev)
 		if err != nil {
 			return err
 		}
-		newResources.MemMB = newMemMB
+		memoryAdded, ok := data.(int)
+		if !ok {
+			return fmt.Errorf("Could not get the memory added, got %+v", data)
+		}
+		newResources.MemMB = oldMemMB + uint32(memoryAdded)
 		if err := c.sandbox.agent.onlineCPUMem(0, false); err != nil {
 			return err
 		}
 	}
-	// hot remove memory unsupported
+	if oldMemMB > newMemMB {
+		// Try to remove a memory device with the difference
+		// from new memory and old memory
+		removeMem := &memoryDevice{
+			sizeMB: int(oldMemMB - newMemMB),
+		}
+
+		data, err := c.sandbox.hypervisor.hotplugRemoveDevice(removeMem, memoryDev)
+		if err != nil {
+			return err
+		}
+		memoryRemoved, ok := data.(int)
+		if !ok {
+			return fmt.Errorf("Could not get the memory added, got %+v", data)
+		}
+		newResources.MemMB = oldMemMB - uint32(memoryRemoved)
+		newResources.MemMB = oldResources.MemMB
+	}
 	return nil
 }
 
@@ -1358,7 +1383,7 @@ func (c *Container) updateResources(oldResources, newResources ContainerResource
 
 	// Memory is not updated if memory limit not set
 	if newResources.MemMB != 0 {
-		if err := c.updateMemoryResources(oldResources, newResources); err != nil {
+		if err := c.updateMemoryResources(oldResources, &newResources); err != nil {
 			return err
 		}
 

virtcontainers/mock_hypervisor.go

@@ -64,6 +64,9 @@ func (m *mockHypervisor) hotplugAddDevice(devInfo interface{}, devType deviceTyp
 	switch devType {
 	case cpuDev:
 		return m.vCPUs, nil
+	case memoryDev:
+		memdev := devInfo.(*memoryDevice)
+		return memdev.sizeMB, nil
 	}
 	return nil, nil
 }

virtcontainers/qemu.go

@@ -7,7 +7,6 @@ package virtcontainers
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -913,7 +912,7 @@ func (q *qemu) hotplugDevice(devInfo interface{}, devType deviceType, op operati
 		return nil, q.hotplugVFIODevice(device, op)
 	case memoryDev:
 		memdev := devInfo.(*memoryDevice)
-		return nil, q.hotplugMemory(memdev, op)
+		return q.hotplugMemory(memdev, op)
 	case netDev:
 		device := devInfo.(*VirtualEndpoint)
 		return nil, q.hotplugNetDevice(device, op)
@@ -1047,24 +1046,27 @@ func (q *qemu) hotplugRemoveCPUs(amount uint32) (uint32, error) {
 	return amount, q.storage.storeHypervisorState(q.id, q.state)
 }
 
-func (q *qemu) hotplugMemory(memDev *memoryDevice, op operation) error {
+func (q *qemu) hotplugMemory(memDev *memoryDevice, op operation) (int, error) {
 	if memDev.sizeMB < 0 {
-		return fmt.Errorf("cannot hotplug negative size (%d) memory", memDev.sizeMB)
+		return 0, fmt.Errorf("cannot hotplug negative size (%d) memory", memDev.sizeMB)
 	}
 
 	// We do not support memory hot unplug.
 	if op == removeDevice {
-		return errors.New("cannot hot unplug memory device")
+		// Dont fail for now, until we fix it.
+		// We return that we only unplugged 0
+		q.Logger().Warn("hot-remove VM memory not supported")
+		return 0, nil
 	}
 
 	err := q.qmpSetup()
 	if err != nil {
-		return err
+		return 0, err
 	}
 
 	maxMem, err := q.hostMemMB()
 	if err != nil {
-		return err
+		return 0, err
 	}
 
 	// calculate current memory
@@ -1072,13 +1074,13 @@ func (q *qemu) hotplugMemory(memDev *memoryDevice, op operation) error {
 
 	// Don't exceed the maximum amount of memory
 	if currentMemory+memDev.sizeMB > int(maxMem) {
-		return fmt.Errorf("Unable to hotplug %d MiB memory, the SB has %d MiB and the maximum amount is %d MiB",
+		return 0, fmt.Errorf("Unable to hotplug %d MiB memory, the SB has %d MiB and the maximum amount is %d MiB",
 			memDev.sizeMB, currentMemory, q.config.MemorySize)
 	}
 
 	memoryDevices, err := q.qmpMonitorCh.qmp.ExecQueryMemoryDevices(q.qmpMonitorCh.ctx)
 	if err != nil {
-		return fmt.Errorf("failed to query memory devices: %v", err)
+		return 0, fmt.Errorf("failed to query memory devices: %v", err)
 	}
 
 	if len(memoryDevices) != 0 {
@@ -1088,15 +1090,15 @@ func (q *qemu) hotplugMemory(memDev *memoryDevice, op operation) error {
 	return q.hotplugAddMemory(memDev)
 }
 
-func (q *qemu) hotplugAddMemory(memDev *memoryDevice) error {
+func (q *qemu) hotplugAddMemory(memDev *memoryDevice) (int, error) {
 	err := q.qmpMonitorCh.qmp.ExecHotplugMemory(q.qmpMonitorCh.ctx, "memory-backend-ram", "mem"+strconv.Itoa(memDev.slot), "", memDev.sizeMB)
 	if err != nil {
 		q.Logger().WithError(err).Error("hotplug memory")
-		return err
+		return 0, err
 	}
 
 	q.state.HotpluggedMemory += memDev.sizeMB
-	return q.storage.storeHypervisorState(q.id, q.state)
+	return memDev.sizeMB, q.storage.storeHypervisorState(q.id, q.state)
 }
 
 func (q *qemu) pauseSandbox() error {

virtcontainers/qemu_test.go

@@ -348,8 +348,10 @@ func TestHotplugRemoveMemory(t *testing.T) {
 	assert := assert.New(t)
 
 	qemuConfig := newQemuConfig()
+	fs := &filesystem{}
 	q := &qemu{
-		config: qemuConfig,
+		config:  qemuConfig,
+		storage: fs,
 	}
 
 	_, err := q.hotplugRemoveDevice(&memoryDevice{0, 128}, memoryDev)
@@ -360,8 +362,10 @@ func TestHotplugUnsupportedDeviceType(t *testing.T) {
 	assert := assert.New(t)
 
 	qemuConfig := newQemuConfig()
+	fs := &filesystem{}
 	q := &qemu{
-		config: qemuConfig,
+		config:  qemuConfig,
+		storage: fs,
 	}
 
 	_, err := q.hotplugAddDevice(&memoryDevice{0, 128}, fsDev)