[Qemu-devel] [PATCH 0/5] don't use NVDIMM for balooning

[Qemu-devel] [PATCH 0/5] don't use NVDIMM for balooning

[Vladimir Sementsov-Ogievskiy]

v6:
 add stubbed pc_dimm_build_list, fix compilation for
 !CONFIG_MEM_HOTPLUG targets - thx to Cornelia.

v5: do not use qapi
 0002-0004: new patches
 0005: white list instead of black list

v4:
 0001: Reviewed-by: Eric Blake <eblake@redhat.com>
 second patch is splitted to 0002 and 0003
 0002: Add 'type' field instead of 'balloonable' to PCDIMMDeviceInfo
 0003: chec 'type' instead of 'balloonable'

v3:
    - do not use additional class variable

NVDIMM for now is planned to use as a backing store for DAX filesystem
in the guest and thus this memory is excluded from guest memory
management and LRUs.

In this case libvirt running QEMU along with configured balloon almost
immediately inflates balloon and effectively kill the guest as
qemu counts nvdimm as part of the ram.

Vladimir Sementsov-Ogievskiy (5):
  move get_current_ram_size to virtio-balloon.c
  pc-dimm: rename pc_dimm_built_list()
  pc-dimm: add pc_dimm_build_list()
  virtio-balloon: rewrite get_current_ram_size()
  balloon: Use only 'pc-dimm' type dimm for ballooning

 hw/mem/pc-dimm.c                | 47 ++++++++++++++++-------------------------
 hw/virtio/virtio-balloon.c      | 19 +++++++++++++++++
 include/exec/cpu-common.h       |  1 -
 include/hw/mem/pc-dimm.h        |  3 +++
 stubs/Makefile.objs             |  2 +-
 stubs/pc_dimm.c                 | 12 +++++++++++
 stubs/qmp_pc_dimm_device_list.c | 12 -----------
 7 files changed, 53 insertions(+), 43 deletions(-)
 create mode 100644 stubs/pc_dimm.c
 delete mode 100644 stubs/qmp_pc_dimm_device_list.c

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/5] move get_current_ram_size to virtio-balloon.c

[Vladimir Sementsov-Ogievskiy]

get_current_ram_size() is used only in virtio-balloon.c
This patch moves it into virtio-balloon and make it static, to allow
some balloon-specific tuning.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 hw/mem/pc-dimm.c                | 26 --------------------------
 hw/virtio/virtio-balloon.c      | 26 ++++++++++++++++++++++++++
 include/exec/cpu-common.h       |  1 -
 stubs/qmp_pc_dimm_device_list.c |  5 -----
 4 files changed, 26 insertions(+), 32 deletions(-)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index d5cdab2..4f30950 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -191,32 +191,6 @@ 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->type) {
-            case MEMORY_DEVICE_INFO_KIND_DIMM:
-                size += value->u.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/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 9671635..6a4c4d2 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -294,6 +294,32 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
     memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
 }
 
+static 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->type) {
+            case MEMORY_DEVICE_INFO_KIND_DIMM:
+                size += value->u.dimm->size;
+                break;
+            default:
+                break;
+            }
+        }
+    }
+    qapi_free_MemoryDeviceInfoList(info_list);
+
+    return size;
+}
+
 static void virtio_balloon_set_config(VirtIODevice *vdev,
                                       const uint8_t *config_data)
 {
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 85aa403..a0ad2ac 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -54,7 +54,6 @@ typedef uintptr_t ram_addr_t;
 #endif
 
 extern ram_addr_t ram_size;
-ram_addr_t get_current_ram_size(void);
 
 /* memory API */
 
diff --git a/stubs/qmp_pc_dimm_device_list.c b/stubs/qmp_pc_dimm_device_list.c
index b584bd8..5cb220c 100644
--- a/stubs/qmp_pc_dimm_device_list.c
+++ b/stubs/qmp_pc_dimm_device_list.c
@@ -5,8 +5,3 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
 {
    return 0;
 }
-
-ram_addr_t get_current_ram_size(void)
-{
-    return ram_size;
-}
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/5] pc-dimm: rename pc_dimm_built_list()

[Vladimir Sementsov-Ogievskiy]

s/pc_dimm_built_list/pc_dimm_build_list_sorted

- need for add pc_dimm_build_list (not sorted) in the next patch
- fix typo (built -> build)

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 hw/mem/pc-dimm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 4f30950..4a681bc 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -251,7 +251,7 @@ static gint pc_dimm_addr_sort(gconstpointer a, gconstpointer b)
     return 0;
 }
 
-static int pc_dimm_built_list(Object *obj, void *opaque)
+static int pc_dimm_build_list_sorted(Object *obj, void *opaque)
 {
     GSList **list = opaque;
 
@@ -262,7 +262,7 @@ static int pc_dimm_built_list(Object *obj, void *opaque)
         }
     }
 
-    object_child_foreach(obj, pc_dimm_built_list, opaque);
+    object_child_foreach(obj, pc_dimm_build_list_sorted, opaque);
     return 0;
 }
 
@@ -296,7 +296,7 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
     }
 
     assert(address_space_end > address_space_start);
-    object_child_foreach(qdev_get_machine(), pc_dimm_built_list, &list);
+    object_child_foreach(qdev_get_machine(), pc_dimm_build_list_sorted, &list);
 
     if (hint) {
         new_addr = *hint;
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/5] pc-dimm: add pc_dimm_build_list()

[Vladimir Sementsov-Ogievskiy]

Like pc_dimm_build_list_sorted but not sorted - for cases where sorting
is not necessary. Add stubbed version too - for targets without
CONFIG_MEM_HOTPLUG.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 hw/mem/pc-dimm.c                | 15 +++++++++++++++
 include/hw/mem/pc-dimm.h        |  3 +++
 stubs/Makefile.objs             |  2 +-
 stubs/pc_dimm.c                 | 12 ++++++++++++
 stubs/qmp_pc_dimm_device_list.c |  7 -------
 5 files changed, 31 insertions(+), 8 deletions(-)
 create mode 100644 stubs/pc_dimm.c
 delete mode 100644 stubs/qmp_pc_dimm_device_list.c

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 4a681bc..0f102c0 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -266,6 +266,21 @@ static int pc_dimm_build_list_sorted(Object *obj, void *opaque)
     return 0;
 }
 
+int pc_dimm_build_list(Object *obj, void *opaque)
+{
+    GSList **list = opaque;
+
+    if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
+        DeviceState *dev = DEVICE(obj);
+        if (dev->realized) { /* only realized DIMMs matter */
+            *list = g_slist_prepend(*list, dev);
+        }
+    }
+
+    object_child_foreach(obj, pc_dimm_build_list, opaque);
+    return 0;
+}
+
 uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
                                uint64_t address_space_size,
                                uint64_t *hint, uint64_t align, uint64_t size,
diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index d83bf30..ab136a3 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -94,4 +94,7 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
                          MemoryRegion *mr, uint64_t align, Error **errp);
 void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms,
                            MemoryRegion *mr);
+
+int pc_dimm_build_list(Object *obj, void *opaque);
+
 #endif
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index d7898a0..781c002 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -34,7 +34,7 @@ stub-obj-y += vmstate.o
 stub-obj-$(CONFIG_WIN32) += fd-register.o
 stub-obj-y += cpus.o
 stub-obj-y += kvm.o
-stub-obj-y += qmp_pc_dimm_device_list.o
+stub-obj-y += pc_dimm.o
 stub-obj-y += target-monitor-defs.o
 stub-obj-y += target-get-monitor-def.o
 stub-obj-y += vhost.o
diff --git a/stubs/pc_dimm.c b/stubs/pc_dimm.c
new file mode 100644
index 0000000..5312f50
--- /dev/null
+++ b/stubs/pc_dimm.c
@@ -0,0 +1,12 @@
+#include "qom/object.h"
+#include "hw/mem/pc-dimm.h"
+
+int qmp_pc_dimm_device_list(Object *obj, void *opaque)
+{
+   return 0;
+}
+
+int pc_dimm_build_list(Object *obj, void *opaque)
+{
+   return 0;
+}
diff --git a/stubs/qmp_pc_dimm_device_list.c b/stubs/qmp_pc_dimm_device_list.c
deleted file mode 100644
index 5cb220c..0000000
--- a/stubs/qmp_pc_dimm_device_list.c
+++ /dev/null
@@ -1,7 +0,0 @@
-#include "qom/object.h"
-#include "hw/mem/pc-dimm.h"
-
-int qmp_pc_dimm_device_list(Object *obj, void *opaque)
-{
-   return 0;
-}
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/5] virtio-balloon: rewrite get_current_ram_size()

[Vladimir Sementsov-Ogievskiy]

Use pc_dimm_built_list() instead of qmp_pc_dimm_device_list()

Actually, Qapi is not related to this internal helper.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 hw/virtio/virtio-balloon.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 6a4c4d2..b9c1964 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -296,26 +296,15 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
 
 static ram_addr_t get_current_ram_size(void)
 {
-    MemoryDeviceInfoList *info_list = NULL;
-    MemoryDeviceInfoList **prev = &info_list;
-    MemoryDeviceInfoList *info;
+    GSList *list = NULL, *item;
     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->type) {
-            case MEMORY_DEVICE_INFO_KIND_DIMM:
-                size += value->u.dimm->size;
-                break;
-            default:
-                break;
-            }
-        }
+    pc_dimm_build_list(qdev_get_machine(), &list);
+    for (item = list; item; item = g_slist_next(item)) {
+        PCDIMMDevice *dimm = item->data;
+        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
     }
-    qapi_free_MemoryDeviceInfoList(info_list);
+    g_slist_free(list);
 
     return size;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/5] balloon: Use only 'pc-dimm' type dimm for ballooning

[Vladimir Sementsov-Ogievskiy]

For now there are only two dimm's: pc-dimm and nvdimm. This patch is
actually needed to disable ballooning on nvdimm. But, to avoid future
bugs, instead of disallowing nvdimm, we allow only pc-dimm. So, if
someone adds new dimm which should be balloon-able, then this ability
should be explicitly specified here.

Why ballooning for nvdimm should be disabled for now:

NVDIMM for now is planned to use as a backing store for DAX filesystem
in the guest and thus this memory is excluded from guest memory
management and LRUs.

In this case libvirt running QEMU along with configured balloon almost
immediately inflates balloon and effectively kill the guest as
qemu counts nvdimm as part of the ram.

Counting dimm devices as part of the ram for ballooning was started from
commit 463756d03:
 virtio-balloon: Fix balloon not working correctly when hotplug memory

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 hw/virtio/virtio-balloon.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index b9c1964..0415e07 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -18,6 +18,7 @@
 #include "qemu-common.h"
 #include "hw/virtio/virtio.h"
 #include "hw/i386/pc.h"
+#include "hw/mem/nvdimm.h"
 #include "cpu.h"
 #include "sysemu/balloon.h"
 #include "hw/virtio/virtio-balloon.h"
@@ -302,7 +303,10 @@ static ram_addr_t get_current_ram_size(void)
     pc_dimm_build_list(qdev_get_machine(), &list);
     for (item = list; item; item = g_slist_next(item)) {
         PCDIMMDevice *dimm = item->data;
-        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
+        if (!strcmp(object_get_typename(OBJECT(dimm)), TYPE_PC_DIMM)) {
+            size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP,
+                                            NULL);
+        }
     }
     g_slist_free(list);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v6 0/5] don't use NVDIMM for balooning

[Vladimir Sementsov-Ogievskiy]

Sorry for subject, it should be
[PATCH v6 0/5] don't use NVDIMM for balooning

On 05.02.2016 11:18, Vladimir Sementsov-Ogievskiy wrote:
> v6:
>   add stubbed pc_dimm_build_list, fix compilation for
>   !CONFIG_MEM_HOTPLUG targets - thx to Cornelia.
>
> v5: do not use qapi
>   0002-0004: new patches
>   0005: white list instead of black list
>
> v4:
>   0001: Reviewed-by: Eric Blake <eblake@redhat.com>
>   second patch is splitted to 0002 and 0003
>   0002: Add 'type' field instead of 'balloonable' to PCDIMMDeviceInfo
>   0003: chec 'type' instead of 'balloonable'
>
> v3:
>      - do not use additional class variable
>
> NVDIMM for now is planned to use as a backing store for DAX filesystem
> in the guest and thus this memory is excluded from guest memory
> management and LRUs.
>
> In this case libvirt running QEMU along with configured balloon almost
> immediately inflates balloon and effectively kill the guest as
> qemu counts nvdimm as part of the ram.
>
> Vladimir Sementsov-Ogievskiy (5):
>    move get_current_ram_size to virtio-balloon.c
>    pc-dimm: rename pc_dimm_built_list()
>    pc-dimm: add pc_dimm_build_list()
>    virtio-balloon: rewrite get_current_ram_size()
>    balloon: Use only 'pc-dimm' type dimm for ballooning
>
>   hw/mem/pc-dimm.c                | 47 ++++++++++++++++-------------------------
>   hw/virtio/virtio-balloon.c      | 19 +++++++++++++++++
>   include/exec/cpu-common.h       |  1 -
>   include/hw/mem/pc-dimm.h        |  3 +++
>   stubs/Makefile.objs             |  2 +-
>   stubs/pc_dimm.c                 | 12 +++++++++++
>   stubs/qmp_pc_dimm_device_list.c | 12 -----------
>   7 files changed, 53 insertions(+), 43 deletions(-)
>   create mode 100644 stubs/pc_dimm.c
>   delete mode 100644 stubs/qmp_pc_dimm_device_list.c
>


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 2/5] pc-dimm: rename pc_dimm_built_list()

[Igor Mammedov]

On Fri,  5 Feb 2016 11:18:58 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:

> s/pc_dimm_built_list/pc_dimm_build_list_sorted
> 
> - need for add pc_dimm_build_list (not sorted) in the next patch
> - fix typo (built -> build)
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/mem/pc-dimm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 4f30950..4a681bc 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -251,7 +251,7 @@ static gint pc_dimm_addr_sort(gconstpointer a, gconstpointer b)
>      return 0;
>  }
>  
> -static int pc_dimm_built_list(Object *obj, void *opaque)
> +static int pc_dimm_build_list_sorted(Object *obj, void *opaque)
>  {
>      GSList **list = opaque;
>  
> @@ -262,7 +262,7 @@ static int pc_dimm_built_list(Object *obj, void *opaque)
>          }
>      }
>  
> -    object_child_foreach(obj, pc_dimm_built_list, opaque);
> +    object_child_foreach(obj, pc_dimm_build_list_sorted, opaque);
>      return 0;
>  }
>  
> @@ -296,7 +296,7 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
>      }
>  
>      assert(address_space_end > address_space_start);
> -    object_child_foreach(qdev_get_machine(), pc_dimm_built_list, &list);
> +    object_child_foreach(qdev_get_machine(), pc_dimm_build_list_sorted, &list);
>  
>      if (hint) {
>          new_addr = *hint;

Re: [Qemu-devel] [PATCH 3/5] pc-dimm: add pc_dimm_build_list()

[Igor Mammedov]

On Fri,  5 Feb 2016 11:18:59 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:

> Like pc_dimm_build_list_sorted but not sorted - for cases where sorting
> is not necessary. Add stubbed version too - for targets without
> CONFIG_MEM_HOTPLUG.
It's not performance critical path, is there a reason why sorted list
can't be used?

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  hw/mem/pc-dimm.c                | 15 +++++++++++++++
>  include/hw/mem/pc-dimm.h        |  3 +++
>  stubs/Makefile.objs             |  2 +-
>  stubs/pc_dimm.c                 | 12 ++++++++++++
>  stubs/qmp_pc_dimm_device_list.c |  7 -------
>  5 files changed, 31 insertions(+), 8 deletions(-)
>  create mode 100644 stubs/pc_dimm.c
>  delete mode 100644 stubs/qmp_pc_dimm_device_list.c
> 
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 4a681bc..0f102c0 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -266,6 +266,21 @@ static int pc_dimm_build_list_sorted(Object *obj, void *opaque)
>      return 0;
>  }
>  
> +int pc_dimm_build_list(Object *obj, void *opaque)
> +{
> +    GSList **list = opaque;
> +
> +    if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
> +        DeviceState *dev = DEVICE(obj);
> +        if (dev->realized) { /* only realized DIMMs matter */
> +            *list = g_slist_prepend(*list, dev);
> +        }
> +    }
> +
> +    object_child_foreach(obj, pc_dimm_build_list, opaque);
> +    return 0;
> +}
> +
>  uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
>                                 uint64_t address_space_size,
>                                 uint64_t *hint, uint64_t align, uint64_t size,
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index d83bf30..ab136a3 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -94,4 +94,7 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
>                           MemoryRegion *mr, uint64_t align, Error **errp);
>  void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms,
>                             MemoryRegion *mr);
> +
> +int pc_dimm_build_list(Object *obj, void *opaque);
> +
>  #endif
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index d7898a0..781c002 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -34,7 +34,7 @@ stub-obj-y += vmstate.o
>  stub-obj-$(CONFIG_WIN32) += fd-register.o
>  stub-obj-y += cpus.o
>  stub-obj-y += kvm.o
> -stub-obj-y += qmp_pc_dimm_device_list.o
> +stub-obj-y += pc_dimm.o
>  stub-obj-y += target-monitor-defs.o
>  stub-obj-y += target-get-monitor-def.o
>  stub-obj-y += vhost.o
> diff --git a/stubs/pc_dimm.c b/stubs/pc_dimm.c
> new file mode 100644
> index 0000000..5312f50
> --- /dev/null
> +++ b/stubs/pc_dimm.c
> @@ -0,0 +1,12 @@
> +#include "qom/object.h"
> +#include "hw/mem/pc-dimm.h"
> +
> +int qmp_pc_dimm_device_list(Object *obj, void *opaque)
> +{
> +   return 0;
> +}
> +
> +int pc_dimm_build_list(Object *obj, void *opaque)
> +{
> +   return 0;
> +}
> diff --git a/stubs/qmp_pc_dimm_device_list.c b/stubs/qmp_pc_dimm_device_list.c
> deleted file mode 100644
> index 5cb220c..0000000
> --- a/stubs/qmp_pc_dimm_device_list.c
> +++ /dev/null
> @@ -1,7 +0,0 @@
> -#include "qom/object.h"
> -#include "hw/mem/pc-dimm.h"
> -
> -int qmp_pc_dimm_device_list(Object *obj, void *opaque)
> -{
> -   return 0;
> -}

Re: [Qemu-devel] [PATCH 4/5] virtio-balloon: rewrite get_current_ram_size()

[Igor Mammedov]

On Fri,  5 Feb 2016 11:19:00 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:

> Use pc_dimm_built_list() instead of qmp_pc_dimm_device_list()
> 
> Actually, Qapi is not related to this internal helper.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  hw/virtio/virtio-balloon.c | 23 ++++++-----------------
>  1 file changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 6a4c4d2..b9c1964 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -296,26 +296,15 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
>  
>  static ram_addr_t get_current_ram_size(void)
>  {
> -    MemoryDeviceInfoList *info_list = NULL;
> -    MemoryDeviceInfoList **prev = &info_list;
> -    MemoryDeviceInfoList *info;
> +    GSList *list = NULL, *item;
>      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->type) {
> -            case MEMORY_DEVICE_INFO_KIND_DIMM:
> -                size += value->u.dimm->size;
> -                break;
> -            default:
> -                break;
> -            }
> -        }
> +    pc_dimm_build_list(qdev_get_machine(), &list);
> +    for (item = list; item; item = g_slist_next(item)) {
> +        PCDIMMDevice *dimm = item->data;
dimm is accessed via properties so you can use Object* instead of PCDIMMDevice

Object *obj = OBJECT(item->data)

> +        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
maybe
s/NULL/error_abort/

>      }
> -    qapi_free_MemoryDeviceInfoList(info_list);
> +    g_slist_free(list);
>  
>      return size;
>  }

Re: [Qemu-devel] [PATCH 5/5] balloon: Use only 'pc-dimm' type dimm for ballooning

[Igor Mammedov]

On Fri,  5 Feb 2016 11:19:01 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:

> For now there are only two dimm's: pc-dimm and nvdimm. This patch is
> actually needed to disable ballooning on nvdimm. But, to avoid future
> bugs, instead of disallowing nvdimm, we allow only pc-dimm. So, if
> someone adds new dimm which should be balloon-able, then this ability
> should be explicitly specified here.
> 
> Why ballooning for nvdimm should be disabled for now:
> 
> NVDIMM for now is planned to use as a backing store for DAX filesystem
> in the guest and thus this memory is excluded from guest memory
> management and LRUs.
> 
> In this case libvirt running QEMU along with configured balloon almost
> immediately inflates balloon and effectively kill the guest as
> qemu counts nvdimm as part of the ram.
> 
> Counting dimm devices as part of the ram for ballooning was started from
> commit 463756d03:
>  virtio-balloon: Fix balloon not working correctly when hotplug memory
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>  hw/virtio/virtio-balloon.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index b9c1964..0415e07 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -18,6 +18,7 @@
>  #include "qemu-common.h"
>  #include "hw/virtio/virtio.h"
>  #include "hw/i386/pc.h"
> +#include "hw/mem/nvdimm.h"
Is this include still needed?

>  #include "cpu.h"
>  #include "sysemu/balloon.h"
>  #include "hw/virtio/virtio-balloon.h"
> @@ -302,7 +303,10 @@ static ram_addr_t get_current_ram_size(void)
>      pc_dimm_build_list(qdev_get_machine(), &list);
>      for (item = list; item; item = g_slist_next(item)) {
>          PCDIMMDevice *dimm = item->data;
> -        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
> +        if (!strcmp(object_get_typename(OBJECT(dimm)), TYPE_PC_DIMM)) {
> +            size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP,
> +                                            NULL);
> +        }
>      }
>      g_slist_free(list);
>  

Re: [Qemu-devel] [PATCH 3/5] pc-dimm: add pc_dimm_build_list()

[Vladimir Sementsov-Ogievskiy]

On 05.02.2016 15:51, Igor Mammedov wrote:
> On Fri,  5 Feb 2016 11:18:59 +0300
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
>
>> Like pc_dimm_build_list_sorted but not sorted - for cases where sorting
>> is not necessary. Add stubbed version too - for targets without
>> CONFIG_MEM_HOTPLUG.
> It's not performance critical path, is there a reason why sorted list
> can't be used?

Only not doing superfluous sorting, no other reasons. I'm ok with sorted 
version too.

>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   hw/mem/pc-dimm.c                | 15 +++++++++++++++
>>   include/hw/mem/pc-dimm.h        |  3 +++
>>   stubs/Makefile.objs             |  2 +-
>>   stubs/pc_dimm.c                 | 12 ++++++++++++
>>   stubs/qmp_pc_dimm_device_list.c |  7 -------
>>   5 files changed, 31 insertions(+), 8 deletions(-)
>>   create mode 100644 stubs/pc_dimm.c
>>   delete mode 100644 stubs/qmp_pc_dimm_device_list.c
>>
>> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
>> index 4a681bc..0f102c0 100644
>> --- a/hw/mem/pc-dimm.c
>> +++ b/hw/mem/pc-dimm.c
>> @@ -266,6 +266,21 @@ static int pc_dimm_build_list_sorted(Object *obj, void *opaque)
>>       return 0;
>>   }
>>   
>> +int pc_dimm_build_list(Object *obj, void *opaque)
>> +{
>> +    GSList **list = opaque;
>> +
>> +    if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
>> +        DeviceState *dev = DEVICE(obj);
>> +        if (dev->realized) { /* only realized DIMMs matter */
>> +            *list = g_slist_prepend(*list, dev);
>> +        }
>> +    }
>> +
>> +    object_child_foreach(obj, pc_dimm_build_list, opaque);
>> +    return 0;
>> +}
>> +
>>   uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
>>                                  uint64_t address_space_size,
>>                                  uint64_t *hint, uint64_t align, uint64_t size,
>> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
>> index d83bf30..ab136a3 100644
>> --- a/include/hw/mem/pc-dimm.h
>> +++ b/include/hw/mem/pc-dimm.h
>> @@ -94,4 +94,7 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
>>                            MemoryRegion *mr, uint64_t align, Error **errp);
>>   void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms,
>>                              MemoryRegion *mr);
>> +
>> +int pc_dimm_build_list(Object *obj, void *opaque);
>> +
>>   #endif
>> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
>> index d7898a0..781c002 100644
>> --- a/stubs/Makefile.objs
>> +++ b/stubs/Makefile.objs
>> @@ -34,7 +34,7 @@ stub-obj-y += vmstate.o
>>   stub-obj-$(CONFIG_WIN32) += fd-register.o
>>   stub-obj-y += cpus.o
>>   stub-obj-y += kvm.o
>> -stub-obj-y += qmp_pc_dimm_device_list.o
>> +stub-obj-y += pc_dimm.o
>>   stub-obj-y += target-monitor-defs.o
>>   stub-obj-y += target-get-monitor-def.o
>>   stub-obj-y += vhost.o
>> diff --git a/stubs/pc_dimm.c b/stubs/pc_dimm.c
>> new file mode 100644
>> index 0000000..5312f50
>> --- /dev/null
>> +++ b/stubs/pc_dimm.c
>> @@ -0,0 +1,12 @@
>> +#include "qom/object.h"
>> +#include "hw/mem/pc-dimm.h"
>> +
>> +int qmp_pc_dimm_device_list(Object *obj, void *opaque)
>> +{
>> +   return 0;
>> +}
>> +
>> +int pc_dimm_build_list(Object *obj, void *opaque)
>> +{
>> +   return 0;
>> +}
>> diff --git a/stubs/qmp_pc_dimm_device_list.c b/stubs/qmp_pc_dimm_device_list.c
>> deleted file mode 100644
>> index 5cb220c..0000000
>> --- a/stubs/qmp_pc_dimm_device_list.c
>> +++ /dev/null
>> @@ -1,7 +0,0 @@
>> -#include "qom/object.h"
>> -#include "hw/mem/pc-dimm.h"
>> -
>> -int qmp_pc_dimm_device_list(Object *obj, void *opaque)
>> -{
>> -   return 0;
>> -}


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 5/5] balloon: Use only 'pc-dimm' type dimm for ballooning

[Vladimir Sementsov-Ogievskiy]

On 05.02.2016 15:58, Igor Mammedov wrote:
> On Fri,  5 Feb 2016 11:19:01 +0300
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
>
>> For now there are only two dimm's: pc-dimm and nvdimm. This patch is
>> actually needed to disable ballooning on nvdimm. But, to avoid future
>> bugs, instead of disallowing nvdimm, we allow only pc-dimm. So, if
>> someone adds new dimm which should be balloon-able, then this ability
>> should be explicitly specified here.
>>
>> Why ballooning for nvdimm should be disabled for now:
>>
>> NVDIMM for now is planned to use as a backing store for DAX filesystem
>> in the guest and thus this memory is excluded from guest memory
>> management and LRUs.
>>
>> In this case libvirt running QEMU along with configured balloon almost
>> immediately inflates balloon and effectively kill the guest as
>> qemu counts nvdimm as part of the ram.
>>
>> Counting dimm devices as part of the ram for ballooning was started from
>> commit 463756d03:
>>   virtio-balloon: Fix balloon not working correctly when hotplug memory
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> ---
>>   hw/virtio/virtio-balloon.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index b9c1964..0415e07 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -18,6 +18,7 @@
>>   #include "qemu-common.h"
>>   #include "hw/virtio/virtio.h"
>>   #include "hw/i386/pc.h"
>> +#include "hw/mem/nvdimm.h"
> Is this include still needed?

Yes, for TYPE_PC_DIMM

>
>>   #include "cpu.h"
>>   #include "sysemu/balloon.h"
>>   #include "hw/virtio/virtio-balloon.h"
>> @@ -302,7 +303,10 @@ static ram_addr_t get_current_ram_size(void)
>>       pc_dimm_build_list(qdev_get_machine(), &list);
>>       for (item = list; item; item = g_slist_next(item)) {
>>           PCDIMMDevice *dimm = item->data;
>> -        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
>> +        if (!strcmp(object_get_typename(OBJECT(dimm)), TYPE_PC_DIMM)) {
>> +            size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP,
>> +                                            NULL);
>> +        }
>>       }
>>       g_slist_free(list);
>>   


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 4/5] virtio-balloon: rewrite get_current_ram_size()

[Vladimir Sementsov-Ogievskiy]

On 05.02.2016 15:57, Igor Mammedov wrote:
> On Fri,  5 Feb 2016 11:19:00 +0300
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
>
>> Use pc_dimm_built_list() instead of qmp_pc_dimm_device_list()
>>
>> Actually, Qapi is not related to this internal helper.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   hw/virtio/virtio-balloon.c | 23 ++++++-----------------
>>   1 file changed, 6 insertions(+), 17 deletions(-)
>>
>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index 6a4c4d2..b9c1964 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -296,26 +296,15 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
>>   
>>   static ram_addr_t get_current_ram_size(void)
>>   {
>> -    MemoryDeviceInfoList *info_list = NULL;
>> -    MemoryDeviceInfoList **prev = &info_list;
>> -    MemoryDeviceInfoList *info;
>> +    GSList *list = NULL, *item;
>>       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->type) {
>> -            case MEMORY_DEVICE_INFO_KIND_DIMM:
>> -                size += value->u.dimm->size;
>> -                break;
>> -            default:
>> -                break;
>> -            }
>> -        }
>> +    pc_dimm_build_list(qdev_get_machine(), &list);
>> +    for (item = list; item; item = g_slist_next(item)) {
>> +        PCDIMMDevice *dimm = item->data;
> dimm is accessed via properties so you can use Object* instead of PCDIMMDevice
>
> Object *obj = OBJECT(item->data)

ok

>
>> +        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
> maybe
> s/NULL/error_abort/

Not sure, but agree, absence of size for memory device is very strange.

>
>>       }
>> -    qapi_free_MemoryDeviceInfoList(info_list);
>> +    g_slist_free(list);
>>   
>>       return size;
>>   }


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 4/5] virtio-balloon: rewrite get_current_ram_size()

[Igor Mammedov]

On Fri, 5 Feb 2016 17:12:55 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:

> On 05.02.2016 15:57, Igor Mammedov wrote:
> > On Fri,  5 Feb 2016 11:19:00 +0300
> > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
> >  
> >> Use pc_dimm_built_list() instead of qmp_pc_dimm_device_list()
> >>
> >> Actually, Qapi is not related to this internal helper.
> >>
> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> >> ---
> >>   hw/virtio/virtio-balloon.c | 23 ++++++-----------------
> >>   1 file changed, 6 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> >> index 6a4c4d2..b9c1964 100644
> >> --- a/hw/virtio/virtio-balloon.c
> >> +++ b/hw/virtio/virtio-balloon.c
> >> @@ -296,26 +296,15 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
> >>   
> >>   static ram_addr_t get_current_ram_size(void)
> >>   {
> >> -    MemoryDeviceInfoList *info_list = NULL;
> >> -    MemoryDeviceInfoList **prev = &info_list;
> >> -    MemoryDeviceInfoList *info;
> >> +    GSList *list = NULL, *item;
> >>       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->type) {
> >> -            case MEMORY_DEVICE_INFO_KIND_DIMM:
> >> -                size += value->u.dimm->size;
> >> -                break;
> >> -            default:
> >> -                break;
> >> -            }
> >> -        }
> >> +    pc_dimm_build_list(qdev_get_machine(), &list);
> >> +    for (item = list; item; item = g_slist_next(item)) {
> >> +        PCDIMMDevice *dimm = item->data;  
> > dimm is accessed via properties so you can use Object* instead of PCDIMMDevice
> >
> > Object *obj = OBJECT(item->data)  
> 
> ok
> 
> >  
> >> +        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);  
> > maybe
> > s/NULL/error_abort/  
> 
> Not sure, but agree, absence of size for memory device is very strange.
it shouldn't happen ever, if error happens it's programming error
and we should fix it then but not hide it.

> 
> >  
> >>       }
> >> -    qapi_free_MemoryDeviceInfoList(info_list);
> >> +    g_slist_free(list);
> >>   
> >>       return size;
> >>   }  
> 
> 

Re: [Qemu-devel] [PATCH 5/5] balloon: Use only 'pc-dimm' type dimm for ballooning

[Igor Mammedov]

On Fri, 5 Feb 2016 17:09:58 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:

> On 05.02.2016 15:58, Igor Mammedov wrote:
> > On Fri,  5 Feb 2016 11:19:01 +0300
> > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
> >  
> >> For now there are only two dimm's: pc-dimm and nvdimm. This patch is
> >> actually needed to disable ballooning on nvdimm. But, to avoid future
> >> bugs, instead of disallowing nvdimm, we allow only pc-dimm. So, if
> >> someone adds new dimm which should be balloon-able, then this ability
> >> should be explicitly specified here.
> >>
> >> Why ballooning for nvdimm should be disabled for now:
> >>
> >> NVDIMM for now is planned to use as a backing store for DAX filesystem
> >> in the guest and thus this memory is excluded from guest memory
> >> management and LRUs.
> >>
> >> In this case libvirt running QEMU along with configured balloon almost
> >> immediately inflates balloon and effectively kill the guest as
> >> qemu counts nvdimm as part of the ram.
> >>
> >> Counting dimm devices as part of the ram for ballooning was started from
> >> commit 463756d03:
> >>   virtio-balloon: Fix balloon not working correctly when hotplug memory
> >>
> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> >> Signed-off-by: Denis V. Lunev <den@openvz.org>
> >> ---
> >>   hw/virtio/virtio-balloon.c | 6 +++++-
> >>   1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> >> index b9c1964..0415e07 100644
> >> --- a/hw/virtio/virtio-balloon.c
> >> +++ b/hw/virtio/virtio-balloon.c
> >> @@ -18,6 +18,7 @@
> >>   #include "qemu-common.h"
> >>   #include "hw/virtio/virtio.h"
> >>   #include "hw/i386/pc.h"
> >> +#include "hw/mem/nvdimm.h"  
> > Is this include still needed?  
> 
> Yes, for TYPE_PC_DIMM
then should it be pc-dimm.h

> 
> >  
> >>   #include "cpu.h"
> >>   #include "sysemu/balloon.h"
> >>   #include "hw/virtio/virtio-balloon.h"
> >> @@ -302,7 +303,10 @@ static ram_addr_t get_current_ram_size(void)
> >>       pc_dimm_build_list(qdev_get_machine(), &list);
> >>       for (item = list; item; item = g_slist_next(item)) {
> >>           PCDIMMDevice *dimm = item->data;
> >> -        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
> >> +        if (!strcmp(object_get_typename(OBJECT(dimm)), TYPE_PC_DIMM)) {
> >> +            size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP,
> >> +                                            NULL);
> >> +        }
> >>       }
> >>       g_slist_free(list);
> >>     
> 
> 

Re: [Qemu-devel] [PATCH 5/5] balloon: Use only 'pc-dimm' type dimm for ballooning

[Vladimir Sementsov-Ogievskiy]

On 05.02.2016 17:55, Igor Mammedov wrote:
> On Fri, 5 Feb 2016 17:09:58 +0300
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
>
>> On 05.02.2016 15:58, Igor Mammedov wrote:
>>> On Fri,  5 Feb 2016 11:19:01 +0300
>>> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
>>>   
>>>> For now there are only two dimm's: pc-dimm and nvdimm. This patch is
>>>> actually needed to disable ballooning on nvdimm. But, to avoid future
>>>> bugs, instead of disallowing nvdimm, we allow only pc-dimm. So, if
>>>> someone adds new dimm which should be balloon-able, then this ability
>>>> should be explicitly specified here.
>>>>
>>>> Why ballooning for nvdimm should be disabled for now:
>>>>
>>>> NVDIMM for now is planned to use as a backing store for DAX filesystem
>>>> in the guest and thus this memory is excluded from guest memory
>>>> management and LRUs.
>>>>
>>>> In this case libvirt running QEMU along with configured balloon almost
>>>> immediately inflates balloon and effectively kill the guest as
>>>> qemu counts nvdimm as part of the ram.
>>>>
>>>> Counting dimm devices as part of the ram for ballooning was started from
>>>> commit 463756d03:
>>>>    virtio-balloon: Fix balloon not working correctly when hotplug memory
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>>> ---
>>>>    hw/virtio/virtio-balloon.c | 6 +++++-
>>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>>>> index b9c1964..0415e07 100644
>>>> --- a/hw/virtio/virtio-balloon.c
>>>> +++ b/hw/virtio/virtio-balloon.c
>>>> @@ -18,6 +18,7 @@
>>>>    #include "qemu-common.h"
>>>>    #include "hw/virtio/virtio.h"
>>>>    #include "hw/i386/pc.h"
>>>> +#include "hw/mem/nvdimm.h"
>>> Is this include still needed?
>> Yes, for TYPE_PC_DIMM
> then should it be pc-dimm.h

oh, yes, I'm wrong.

>
>>>   
>>>>    #include "cpu.h"
>>>>    #include "sysemu/balloon.h"
>>>>    #include "hw/virtio/virtio-balloon.h"
>>>> @@ -302,7 +303,10 @@ static ram_addr_t get_current_ram_size(void)
>>>>        pc_dimm_build_list(qdev_get_machine(), &list);
>>>>        for (item = list; item; item = g_slist_next(item)) {
>>>>            PCDIMMDevice *dimm = item->data;
>>>> -        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
>>>> +        if (!strcmp(object_get_typename(OBJECT(dimm)), TYPE_PC_DIMM)) {
>>>> +            size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP,
>>>> +                                            NULL);
>>>> +        }
>>>>        }
>>>>        g_slist_free(list);
>>>>      
>>


-- 
Best regards,
Vladimir

[Qemu-devel] [PATCH 4/5] virtio-balloon: rewrite get_current_ram_size()

[Vladimir Sementsov-Ogievskiy]

Use pc_dimm_built_list() instead of qmp_pc_dimm_device_list()

Actually, Qapi is not related to this internal helper.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 hw/virtio/virtio-balloon.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 6a4c4d2..14aab77 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -296,26 +296,15 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
 
 static ram_addr_t get_current_ram_size(void)
 {
-    MemoryDeviceInfoList *info_list = NULL;
-    MemoryDeviceInfoList **prev = &info_list;
-    MemoryDeviceInfoList *info;
+    GSList *list = NULL, *item;
     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->type) {
-            case MEMORY_DEVICE_INFO_KIND_DIMM:
-                size += value->u.dimm->size;
-                break;
-            default:
-                break;
-            }
-        }
+    pc_dimm_build_list(qdev_get_machine(), &list);
+    for (item = list; item; item = g_slist_next(item)) {
+        Object *obj = OBJECT(item->data);
+        size += object_property_get_int(obj, PC_DIMM_SIZE_PROP, &error_abort);
     }
-    qapi_free_MemoryDeviceInfoList(info_list);
+    g_slist_free(list);
 
     return size;
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 4/5] virtio-balloon: rewrite get_current_ram_size()

[Cornelia Huck]

On Thu, 4 Feb 2016 15:36:05 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:

> On 04.02.2016 15:23, Cornelia Huck wrote:
> > On Thu,  4 Feb 2016 14:37:22 +0300
> > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:

> >> +    pc_dimm_build_list(qdev_get_machine(), &list);
> > This will break the build for !CONFIG_MEM_HOTPLUG, as you didn't
> > provide a stubbed-out version of this function.
> 
> Ok, thanks, will add.
> 
> Also: it is already broken because of pc_dimm_memory_plug and 
> pc_dimm_memory_unplug

These are only used by targets that set CONFIG_MEM_HOTPLUG afaics.

Re: [Qemu-devel] [PATCH 4/5] virtio-balloon: rewrite get_current_ram_size()

[Vladimir Sementsov-Ogievskiy]

On 04.02.2016 15:23, Cornelia Huck wrote:
> On Thu,  4 Feb 2016 14:37:22 +0300
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
>
>> Use pc_dimm_built_list() instead of qmp_pc_dimm_device_list()
>>
>> Actually, Qapi is not related to this internal helper.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>
>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>> CC: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>> CC: "Michael S. Tsirkin" <mst@redhat.com>
>> CC: Igor Mammedov <imammedo@redhat.com>
>> CC: Eric Blake <eblake@redhat.com>
>> CC: Markus Armbruster <armbru@redhat.com>
>> ---
>>   hw/virtio/virtio-balloon.c | 23 ++++++-----------------
>>   1 file changed, 6 insertions(+), 17 deletions(-)
>>
>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index 6a4c4d2..b9c1964 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -296,26 +296,15 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
>>
>>   static ram_addr_t get_current_ram_size(void)
>>   {
>> -    MemoryDeviceInfoList *info_list = NULL;
>> -    MemoryDeviceInfoList **prev = &info_list;
>> -    MemoryDeviceInfoList *info;
>> +    GSList *list = NULL, *item;
>>       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->type) {
>> -            case MEMORY_DEVICE_INFO_KIND_DIMM:
>> -                size += value->u.dimm->size;
>> -                break;
>> -            default:
>> -                break;
>> -            }
>> -        }
>> +    pc_dimm_build_list(qdev_get_machine(), &list);
> This will break the build for !CONFIG_MEM_HOTPLUG, as you didn't
> provide a stubbed-out version of this function.

Ok, thanks, will add.

Also: it is already broken because of pc_dimm_memory_plug and 
pc_dimm_memory_unplug


>
>> +    for (item = list; item; item = g_slist_next(item)) {
>> +        PCDIMMDevice *dimm = item->data;
>> +        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
>>       }
>> -    qapi_free_MemoryDeviceInfoList(info_list);
>> +    g_slist_free(list);
>>
>>       return size;
>>   }


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 4/5] virtio-balloon: rewrite get_current_ram_size()

[Cornelia Huck]

On Thu,  4 Feb 2016 14:37:22 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:

> Use pc_dimm_built_list() instead of qmp_pc_dimm_device_list()
> 
> Actually, Qapi is not related to this internal helper.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> CC: "Michael S. Tsirkin" <mst@redhat.com>
> CC: Igor Mammedov <imammedo@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/virtio/virtio-balloon.c | 23 ++++++-----------------
>  1 file changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 6a4c4d2..b9c1964 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -296,26 +296,15 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
> 
>  static ram_addr_t get_current_ram_size(void)
>  {
> -    MemoryDeviceInfoList *info_list = NULL;
> -    MemoryDeviceInfoList **prev = &info_list;
> -    MemoryDeviceInfoList *info;
> +    GSList *list = NULL, *item;
>      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->type) {
> -            case MEMORY_DEVICE_INFO_KIND_DIMM:
> -                size += value->u.dimm->size;
> -                break;
> -            default:
> -                break;
> -            }
> -        }
> +    pc_dimm_build_list(qdev_get_machine(), &list);

This will break the build for !CONFIG_MEM_HOTPLUG, as you didn't
provide a stubbed-out version of this function.

> +    for (item = list; item; item = g_slist_next(item)) {
> +        PCDIMMDevice *dimm = item->data;
> +        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
>      }
> -    qapi_free_MemoryDeviceInfoList(info_list);
> +    g_slist_free(list);
> 
>      return size;
>  }

[Qemu-devel] [PATCH 4/5] virtio-balloon: rewrite get_current_ram_size()

[Vladimir Sementsov-Ogievskiy]

Use pc_dimm_built_list() instead of qmp_pc_dimm_device_list()

Actually, Qapi is not related to this internal helper.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Xiao Guangrong <guangrong.xiao@linux.intel.com>
CC: "Michael S. Tsirkin" <mst@redhat.com>
CC: Igor Mammedov <imammedo@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
---
 hw/virtio/virtio-balloon.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 6a4c4d2..b9c1964 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -296,26 +296,15 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
 
 static ram_addr_t get_current_ram_size(void)
 {
-    MemoryDeviceInfoList *info_list = NULL;
-    MemoryDeviceInfoList **prev = &info_list;
-    MemoryDeviceInfoList *info;
+    GSList *list = NULL, *item;
     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->type) {
-            case MEMORY_DEVICE_INFO_KIND_DIMM:
-                size += value->u.dimm->size;
-                break;
-            default:
-                break;
-            }
-        }
+    pc_dimm_build_list(qdev_get_machine(), &list);
+    for (item = list; item; item = g_slist_next(item)) {
+        PCDIMMDevice *dimm = item->data;
+        size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
     }
-    qapi_free_MemoryDeviceInfoList(info_list);
+    g_slist_free(list);
 
     return size;
 }
-- 
1.8.3.1