From 4697cf3c79a075911f5fc98eaf6ddd18da92b52a Mon Sep 17 00:00:00 2001 From: Jose Carlos Venegas Munoz Date: Sat, 29 Sep 2018 11:15:58 -0500 Subject: [PATCH 1/2] memory: update: Update state using the memory removed. If the memory is reduced , its cgroup in the VM was updated properly. But the runtime assumed that the memory was also removed from the VM. Then when it is added more memory again, more is added (but not needed). Fixes: #801 Signed-off-by: Jose Carlos Venegas Munoz --- virtcontainers/container.go | 35 ++++++++++++++++++++++++++++++----- virtcontainers/qemu.go | 26 ++++++++++++++------------ 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/virtcontainers/container.go b/virtcontainers/container.go index bf063c0fd6..96e853eb57 100644 --- a/virtcontainers/container.go +++ b/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 } diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index db8aa69a88..920ed9268b 100644 --- a/virtcontainers/qemu.go +++ b/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 { From 1f5792ecbb5c15be375beba3bcd47528d17dd80f Mon Sep 17 00:00:00 2001 From: Jose Carlos Venegas Munoz Date: Tue, 2 Oct 2018 14:30:24 -0500 Subject: [PATCH 2/2] test: fix unit test nil pointer. Add filesystem to qemu object. Fix mock_hypervisor Signed-off-by: Jose Carlos Venegas Munoz --- virtcontainers/mock_hypervisor.go | 3 +++ virtcontainers/qemu_test.go | 8 ++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/virtcontainers/mock_hypervisor.go b/virtcontainers/mock_hypervisor.go index c94096b12e..5952fb7f95 100644 --- a/virtcontainers/mock_hypervisor.go +++ b/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 } diff --git a/virtcontainers/qemu_test.go b/virtcontainers/qemu_test.go index f4b96ee722..e0b7159b4e 100644 --- a/virtcontainers/qemu_test.go +++ b/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)