[Qemu-devel] [PATCH 0/3] fix bug about balloon working incorrect when hotplug memeory

[Qemu-devel] [PATCH 0/3] fix bug about balloon working incorrect when hotplug memeory

[zhanghailiang]

Patch 1 and 2 mainly fix bug about balloon not working correctly when we do
hotplug memory. It takes 'ram_size' as VM's real RAM size which is wrong
after we hotplug memory.

This bug exists since we begin to support hotplug memory, and it is better
to fix it.

Patch 3 add some trace events, it helps debugging balloon. If it is unnecessary, 
pls feel free to remove it.

Thanks
zhanghailiang (3):
  pc-dimm: add a function to calculate VM's current RAM size
  virtio-balloon: Fix balloon not working correctly when hotplug memory
  virtio-balloon: Add some trace events

 hw/mem/pc-dimm.c           | 26 ++++++++++++++++++++++++++
 hw/virtio/virtio-balloon.c | 21 +++++++++++++++------
 include/exec/cpu-common.h  |  1 +
 trace-events               |  4 ++++
 4 files changed, 46 insertions(+), 6 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH 1/3] pc-dimm: add a function to calculate VM's current RAM size

[zhanghailiang]

The global parameter 'ram_size' does not take into account
the hotplugged memory.

In some codes, we use 'ram_size' as current VM's real RAM size,
which is not correct.

Add function 'get_current_ram_size' to calculate VM's current RAM size,
it will enumerate present memory devices and also plus ram_size.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 hw/mem/pc-dimm.c          | 26 ++++++++++++++++++++++++++
 include/exec/cpu-common.h |  1 +
 2 files changed, 27 insertions(+)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index a800ea7..38465d0 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -62,6 +62,32 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
     return 0;
 }
 
+ram_addr_t get_current_ram_size(void)
+{
+    MemoryDeviceInfoList *info_list = NULL;
+    MemoryDeviceInfoList **prev = &info_list;
+    MemoryDeviceInfoList *info;
+    ram_addr_t size = ram_size;
+
+    qmp_pc_dimm_device_list(qdev_get_machine(), &prev);
+    for (info = info_list; info; info = info->next) {
+        MemoryDeviceInfo *value = info->value;
+
+        if (value) {
+            switch (value->kind) {
+            case MEMORY_DEVICE_INFO_KIND_DIMM:
+                size += value->dimm->size;
+                break;
+            default:
+                break;
+            }
+        }
+    }
+    qapi_free_MemoryDeviceInfoList(info_list);
+
+    return size;
+}
+
 static int pc_dimm_slot2bitmap(Object *obj, void *opaque)
 {
     unsigned long *bitmap = opaque;
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 427b851..fcc3162 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -52,6 +52,7 @@ typedef uintptr_t ram_addr_t;
 #endif
 
 extern ram_addr_t ram_size;
+ram_addr_t get_current_ram_size(void);
 
 /* memory API */
 
-- 
1.7.12.4

[Qemu-devel] [PATCH 2/3] virtio-balloon: Fix balloon not working correctly when hotplug memory

[zhanghailiang]

When do memory balloon, it takes the 'ram_size' as the VM's current ram size,
But 'ram_size' is the startup configured ram size, it does not take into
account the hotplugged memory.

As a result, the balloon result will be confused.
Steps to reproduce:
(1)Start VM: qemu -m size=1024,slots=4,maxmem=8G
(2)In VM: #free -m : 1024M
(3)qmp balloon 512M
(4)In VM: #free -m : 512M
(5)hotplug pc-dimm 1G
(6)In VM: #free -m : 1512M
(7)qmp balloon 256M
(8)In VM: #free -m :1256M
We expect the VM's available ram size to be 256M after 'qmp balloon 256M'
command, but VM's real available ram size is 1256M.

For "qmp balloon" is not performance critical code, we use function
'get_current_ram_size' to get VM's current ram size.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 hw/virtio/virtio-balloon.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 7bfbb75..41b24c9 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -294,10 +294,12 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
     VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
     struct virtio_balloon_config config;
     uint32_t oldactual = dev->actual;
+    ram_addr_t vm_ram_size = get_current_ram_size();
+
     memcpy(&config, config_data, sizeof(struct virtio_balloon_config));
     dev->actual = le32_to_cpu(config.actual);
     if (dev->actual != oldactual) {
-        qapi_event_send_balloon_change(ram_size -
+        qapi_event_send_balloon_change(vm_ram_size -
                         ((ram_addr_t) dev->actual << VIRTIO_BALLOON_PFN_SHIFT),
                         &error_abort);
     }
@@ -312,20 +314,21 @@ static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
 static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
 {
     VirtIOBalloon *dev = opaque;
-    info->actual = ram_size - ((uint64_t) dev->actual <<
-                               VIRTIO_BALLOON_PFN_SHIFT);
+    info->actual = get_current_ram_size() - ((uint64_t) dev->actual <<
+                                             VIRTIO_BALLOON_PFN_SHIFT);
 }
 
 static void virtio_balloon_to_target(void *opaque, ram_addr_t target)
 {
     VirtIOBalloon *dev = VIRTIO_BALLOON(opaque);
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    ram_addr_t vm_ram_size = get_current_ram_size();
 
-    if (target > ram_size) {
-        target = ram_size;
+    if (target > vm_ram_size) {
+        target = vm_ram_size;
     }
     if (target) {
-        dev->num_pages = (ram_size - target) >> VIRTIO_BALLOON_PFN_SHIFT;
+        dev->num_pages = (vm_ram_size - target) >> VIRTIO_BALLOON_PFN_SHIFT;
         virtio_notify_config(vdev);
     }
 }
-- 
1.7.12.4

[Qemu-devel] [PATCH 3/3] virtio-balloon: Add some trace events

[zhanghailiang]

Add some trace events for easier debugging

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 hw/virtio/virtio-balloon.c | 6 ++++++
 trace-events               | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 41b24c9..8a48d2a 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -25,6 +25,7 @@
 #include "exec/address-spaces.h"
 #include "qapi/visitor.h"
 #include "qapi-event.h"
+#include "trace.h"
 
 #if defined(__linux__)
 #include <sys/mman.h>
@@ -222,6 +223,8 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
             if (!int128_nz(section.size) || !memory_region_is_ram(section.mr))
                 continue;
 
+            trace_virtio_balloon_handle_output(memory_region_name(section.mr),
+                                               pa);
             /* Using memory_region_get_ram_ptr is bending the rules a bit, but
                should be OK because we only want a single page.  */
             addr = section.offset_within_region;
@@ -285,6 +288,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
     config.num_pages = cpu_to_le32(dev->num_pages);
     config.actual = cpu_to_le32(dev->actual);
 
+    trace_virtio_balloon_get_config(config.num_pages, config.actual);
     memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
 }
 
@@ -303,6 +307,7 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
                         ((ram_addr_t) dev->actual << VIRTIO_BALLOON_PFN_SHIFT),
                         &error_abort);
     }
+    trace_virtio_balloon_set_config(dev->actual, oldactual);
 }
 
 static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
@@ -331,6 +336,7 @@ static void virtio_balloon_to_target(void *opaque, ram_addr_t target)
         dev->num_pages = (vm_ram_size - target) >> VIRTIO_BALLOON_PFN_SHIFT;
         virtio_notify_config(vdev);
     }
+    trace_virtio_balloon_to_target(target, dev->num_pages);
 }
 
 static void virtio_balloon_save(QEMUFile *f, void *opaque)
diff --git a/trace-events b/trace-events
index b5722ea..bba5be8 100644
--- a/trace-events
+++ b/trace-events
@@ -142,6 +142,10 @@ cpu_out(unsigned int addr, unsigned int val) "addr %#x value %u"
 # balloon.c
 # Since requests are raised via monitor, not many tracepoints are needed.
 balloon_event(void *opaque, unsigned long addr) "opaque %p addr %lu"
+virtio_balloon_handle_output(const char *name, uint64_t gpa) "setion name: %s gpa: %"PRIx64""
+virtio_balloon_get_config(uint32_t num_pages, uint32_t acutal) "num_pages: %d acutal: %d"
+virtio_balloon_set_config(uint32_t acutal, uint32_t oldacutal) "acutal: %d oldacutal: %d"
+virtio_balloon_to_target(uint64_t target, uint32_t num_pages) "balloon target: %"PRIx64" num_pages: %d"
 
 # hw/intc/apic_common.c
 cpu_set_apic_base(uint64_t val) "%016"PRIx64
-- 
1.7.12.4