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

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

[Vladimir Sementsov-Ogievskiy]

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.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
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>

Vladimir Sementsov-Ogievskiy (3):
  move get_current_ram_size to virtio-balloon.c
  pcdimm: add 'type' field to PCDIMMDeviceInfo
  balloon: don't use NVDIMM for ballooning

 hw/mem/pc-dimm.c                | 27 +--------------------------
 hw/virtio/virtio-balloon.c      | 29 +++++++++++++++++++++++++++++
 include/exec/cpu-common.h       |  1 -
 qapi-schema.json                |  5 ++++-
 stubs/qmp_pc_dimm_device_list.c |  5 -----
 5 files changed, 34 insertions(+), 33 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/3] 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>

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/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/3] pcdimm: add 'type' field to PCDIMMDeviceInfo

[Vladimir Sementsov-Ogievskiy]

The field is needed to distinguish pc-dimm and nvdimm.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
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/mem/pc-dimm.c | 1 +
 qapi-schema.json | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 4f30950..7469bd4 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -178,6 +178,7 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
             di->size = object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP,
                                                NULL);
             di->memdev = object_get_canonical_path(OBJECT(dimm->hostmem));
+            di->type = g_strdup(object_get_typename(obj));
 
             info->u.dimm = di;
             elem->value = info;
diff --git a/qapi-schema.json b/qapi-schema.json
index 8d04897..3bcc957 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3924,6 +3924,8 @@
 #
 # @hotpluggable: true if device if could be added/removed while machine is running
 #
+# @type: device type: 'pc-dimm' or 'nvdimm' (since 2.6)
+#
 # Since: 2.1
 ##
 { 'struct': 'PCDIMMDeviceInfo',
@@ -3934,7 +3936,8 @@
             'node': 'int',
             'memdev': 'str',
             'hotplugged': 'bool',
-            'hotpluggable': 'bool'
+            'hotpluggable': 'bool',
+            'type': 'str'
           }
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/3] pcdimm: add 'type' field to PCDIMMDeviceInfo

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> The field is needed to distinguish pc-dimm and nvdimm.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> 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/mem/pc-dimm.c | 1 +
>  qapi-schema.json | 5 ++++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 4f30950..7469bd4 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -178,6 +178,7 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
       if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
           DeviceState *dev = DEVICE(obj);

           if (dev->realized) {
[...]
>              di->size = object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP,
>                                                 NULL);
>              di->memdev = object_get_canonical_path(OBJECT(dimm->hostmem));
> +            di->type = g_strdup(object_get_typename(obj));

@type is the type name of a subtype of TYPE_PC_DIMM.

Current subtypes are TYPE_PC_DIMM itself and TYPE_NVDIMM, i.e. "pc-dimm"
and "nvdimm".

>  
>              info->u.dimm = di;
>              elem->value = info;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 8d04897..3bcc957 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3924,6 +3924,8 @@
>  #
>  # @hotpluggable: true if device if could be added/removed while machine is running
>  #
> +# @type: device type: 'pc-dimm' or 'nvdimm' (since 2.6)
> +#

Not wrong, but brittle: it breaks whenever someone adds another subtype
and doesn't remember to update this spot.  Remembering it seems
unlikely, because it's somewhere else entirely.

Let's document it to be the type name of a subtype of pc-dimm, and that
you can query the possible values with QMP command

    { "execute": "qom-list-types", "arguments": { "implements": "pc-dimm", "abstract": false } }

>  # Since: 2.1
>  ##
>  { 'struct': 'PCDIMMDeviceInfo',
> @@ -3934,7 +3936,8 @@
>              'node': 'int',
>              'memdev': 'str',
>              'hotplugged': 'bool',
> -            'hotpluggable': 'bool'
> +            'hotpluggable': 'bool',
> +            'type': 'str'
>            }
>  }

Re: [Qemu-devel] [PATCH 2/3] pcdimm: add 'type' field to PCDIMMDeviceInfo

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1352 bytes --]

On 01/27/2016 11:51 PM, Vladimir Sementsov-Ogievskiy wrote:
> The field is needed to distinguish pc-dimm and nvdimm.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> 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>
> ---

> +++ b/qapi-schema.json
> @@ -3924,6 +3924,8 @@
>  #
>  # @hotpluggable: true if device if could be added/removed while machine is running
>  #
> +# @type: device type: 'pc-dimm' or 'nvdimm' (since 2.6)
> +#
>  # Since: 2.1
>  ##
>  { 'struct': 'PCDIMMDeviceInfo',
> @@ -3934,7 +3936,8 @@
>              'node': 'int',
>              'memdev': 'str',
>              'hotplugged': 'bool',
> -            'hotpluggable': 'bool'
> +            'hotpluggable': 'bool',
> +            'type': 'str'

No. Since it is a finite set of values (just two possible), you should
be using an enum here rather than open-coded 'str'. Something like:

{ 'enum': 'DIMMType', 'data': [ 'pc-dimm', 'nvdimm' ] }

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 2/3] pcdimm: add 'type' field to PCDIMMDeviceInfo

[Vladimir Sementsov-Ogievskiy]

On 03.02.2016 01:12, Eric Blake wrote:
> On 01/27/2016 11:51 PM, Vladimir Sementsov-Ogievskiy wrote:
>> The field is needed to distinguish pc-dimm and nvdimm.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> 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>
>> ---
>> +++ b/qapi-schema.json
>> @@ -3924,6 +3924,8 @@
>>   #
>>   # @hotpluggable: true if device if could be added/removed while machine is running
>>   #
>> +# @type: device type: 'pc-dimm' or 'nvdimm' (since 2.6)
>> +#
>>   # Since: 2.1
>>   ##
>>   { 'struct': 'PCDIMMDeviceInfo',
>> @@ -3934,7 +3936,8 @@
>>               'node': 'int',
>>               'memdev': 'str',
>>               'hotplugged': 'bool',
>> -            'hotpluggable': 'bool'
>> +            'hotpluggable': 'bool',
>> +            'type': 'str'
> No. Since it is a finite set of values (just two possible), you should
> be using an enum here rather than open-coded 'str'. Something like:
>
> { 'enum': 'DIMMType', 'data': [ 'pc-dimm', 'nvdimm' ] }
>

Are you sure? This is only output Info, so user will never "set" this 
field. Also, qemu type system (as I understand) is based on string 
names. object_dynamic_cast and other functions uses "const char 
*typename". This enum will be out of qemu type system and we will have 
to sync it.. Is there already some practice of translating string 
typenames to enum values?


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 2/3] pcdimm: add 'type' field to PCDIMMDeviceInfo

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1536 bytes --]

On 02/03/2016 05:00 AM, Vladimir Sementsov-Ogievskiy wrote:

>>> +# @type: device type: 'pc-dimm' or 'nvdimm' (since 2.6)
>>> +#
>>>   # Since: 2.1
>>>   ##
>>>   { 'struct': 'PCDIMMDeviceInfo',
>>> @@ -3934,7 +3936,8 @@
>>>               'node': 'int',
>>>               'memdev': 'str',
>>>               'hotplugged': 'bool',
>>> -            'hotpluggable': 'bool'
>>> +            'hotpluggable': 'bool',
>>> +            'type': 'str'
>> No. Since it is a finite set of values (just two possible), you should
>> be using an enum here rather than open-coded 'str'. Something like:
>>
>> { 'enum': 'DIMMType', 'data': [ 'pc-dimm', 'nvdimm' ] }
>>
> 
> Are you sure? This is only output Info, so user will never "set" this
> field. Also, qemu type system (as I understand) is based on string
> names. object_dynamic_cast and other functions uses "const char
> *typename". This enum will be out of qemu type system and we will have
> to sync it.. Is there already some practice of translating string
> typenames to enum values?

Yes, exposing a finite set of strings as an enum is ideal for the user
interface, even if we carry string values instead of enum values in
other places in the code.  QAPI already includes convenience methods for
translating between strings and enum values (EnumName_lookup[] to go
from int to string, and qapi_enum_parse() to reverse from string back to
enum).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 2/3] pcdimm: add 'type' field to PCDIMMDeviceInfo

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 02/03/2016 05:00 AM, Vladimir Sementsov-Ogievskiy wrote:
>
>>>> +# @type: device type: 'pc-dimm' or 'nvdimm' (since 2.6)
>>>> +#
>>>>   # Since: 2.1
>>>>   ##
>>>>   { 'struct': 'PCDIMMDeviceInfo',
>>>> @@ -3934,7 +3936,8 @@
>>>>               'node': 'int',
>>>>               'memdev': 'str',
>>>>               'hotplugged': 'bool',
>>>> -            'hotpluggable': 'bool'
>>>> +            'hotpluggable': 'bool',
>>>> +            'type': 'str'
>>> No. Since it is a finite set of values (just two possible), you should
>>> be using an enum here rather than open-coded 'str'. Something like:
>>>
>>> { 'enum': 'DIMMType', 'data': [ 'pc-dimm', 'nvdimm' ] }
>>>
>> 
>> Are you sure? This is only output Info, so user will never "set" this
>> field. Also, qemu type system (as I understand) is based on string
>> names. object_dynamic_cast and other functions uses "const char
>> *typename". This enum will be out of qemu type system and we will have
>> to sync it.. Is there already some practice of translating string
>> typenames to enum values?
>
> Yes, exposing a finite set of strings as an enum is ideal for the user
> interface, even if we carry string values instead of enum values in
> other places in the code.  QAPI already includes convenience methods for
> translating between strings and enum values (EnumName_lookup[] to go
> from int to string, and qapi_enum_parse() to reverse from string back to
> enum).

Well, it's not an arbitrary set of strings, it the set of names of
concrete subtypes of "pc-dimm".  See also my reply to this patch
(Message-ID: <8737tbjfp9.fsf@blackfin.pond.sub.org>), which also shows
how to introspect this set.

Doesn't mean we must not create an enum for this set.  If we do, we get
to map from type name to enum value in qmp_pc_dimm_device_list().  Begs
the question what to do when we run into a type name we don't know to
map.

The deeper question is whether we want to duplicate sets of QOM type
names as QAPI enums in general.  I'm leaning towards "we don't", but I'm
happy to hear arguments for doing it.

[Qemu-devel] [PATCH 3/3] balloon: don't use NVDIMM for ballooning

[Vladimir Sementsov-Ogievskiy]

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>
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 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 6a4c4d2..749be25 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -26,6 +26,7 @@
 #include "qapi/visitor.h"
 #include "qapi-event.h"
 #include "trace.h"
+#include "hw/mem/nvdimm.h"
 
 #if defined(__linux__)
 #include <sys/mman.h>
@@ -308,7 +309,9 @@ static ram_addr_t get_current_ram_size(void)
         if (value) {
             switch (value->type) {
             case MEMORY_DEVICE_INFO_KIND_DIMM:
-                size += value->u.dimm->size;
+                if (strcmp(value->u.dimm->type, TYPE_NVDIMM)) {
+                    size += value->u.dimm->size;
+                }
                 break;
             default:
                 break;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 3/3] balloon: don't use NVDIMM for ballooning

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> 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>
> 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 | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 6a4c4d2..749be25 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -26,6 +26,7 @@
>  #include "qapi/visitor.h"
>  #include "qapi-event.h"
>  #include "trace.h"
> +#include "hw/mem/nvdimm.h"
>  
>  #if defined(__linux__)
>  #include <sys/mman.h>
> @@ -308,7 +309,9 @@ static ram_addr_t get_current_ram_size(void)
>          if (value) {
>              switch (value->type) {
>              case MEMORY_DEVICE_INFO_KIND_DIMM:
> -                size += value->u.dimm->size;
> +                if (strcmp(value->u.dimm->type, TYPE_NVDIMM)) {
> +                    size += value->u.dimm->size;
> +                }
>                  break;
>              default:
>                  break;

Should this be a blacklist ("don't count TYPE_NVDIMM") or a whitelist
("count TYPE_PC_DIMM")?  I guess that depends on whether we think future
types are more likely to need counting or not counting.

Re: [Qemu-devel] [PATCH 3/3] balloon: don't use NVDIMM for ballooning

[Vladimir Sementsov-Ogievskiy]

On 02.02.2016 18:30, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
>
>> 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>
>> 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 | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index 6a4c4d2..749be25 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -26,6 +26,7 @@
>>   #include "qapi/visitor.h"
>>   #include "qapi-event.h"
>>   #include "trace.h"
>> +#include "hw/mem/nvdimm.h"
>>   
>>   #if defined(__linux__)
>>   #include <sys/mman.h>
>> @@ -308,7 +309,9 @@ static ram_addr_t get_current_ram_size(void)
>>           if (value) {
>>               switch (value->type) {
>>               case MEMORY_DEVICE_INFO_KIND_DIMM:
>> -                size += value->u.dimm->size;
>> +                if (strcmp(value->u.dimm->type, TYPE_NVDIMM)) {
>> +                    size += value->u.dimm->size;
>> +                }
>>                   break;
>>               default:
>>                   break;
> Should this be a blacklist ("don't count TYPE_NVDIMM") or a whitelist
> ("count TYPE_PC_DIMM")?  I guess that depends on whether we think future
> types are more likely to need counting or not counting.

May be, to avoid such bugs in future, it would be better to make like a 
whitelist.



-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 3/3] balloon: don't use NVDIMM for ballooning

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]

On 01/27/2016 11:51 PM, Vladimir Sementsov-Ogievskiy wrote:
> 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>
> 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>
> ---

> @@ -308,7 +309,9 @@ static ram_addr_t get_current_ram_size(void)
>          if (value) {
>              switch (value->type) {
>              case MEMORY_DEVICE_INFO_KIND_DIMM:
> -                size += value->u.dimm->size;
> +                if (strcmp(value->u.dimm->type, TYPE_NVDIMM)) {

If you fix 2/3 to use a QAPI enum, then this will be an integer compare
instead of a strcmp().

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 3/3] balloon: don't use NVDIMM for ballooning

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 01/27/2016 11:51 PM, Vladimir Sementsov-Ogievskiy wrote:
>> 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>
>> 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>
>> ---
>
>> @@ -308,7 +309,9 @@ static ram_addr_t get_current_ram_size(void)
>>          if (value) {
>>              switch (value->type) {
>>              case MEMORY_DEVICE_INFO_KIND_DIMM:
>> -                size += value->u.dimm->size;
>> +                if (strcmp(value->u.dimm->type, TYPE_NVDIMM)) {
>
> If you fix 2/3 to use a QAPI enum, then this will be an integer compare
> instead of a strcmp().

Another option is to add a flag to the subtypes of TYPE_PC_DIMM, set it
for the subtypes that should be counted here, and accumulate the sizes
of devices where the flag is set.  Requires iterating directly over the
devices here (like qmp_pc_dimm_device_list() does under the hood) rather
than the MemoryDeviceInfoList returned by qmp_pc_dimm_device_list(),

Re: [Qemu-devel] [PATCH 3/3] balloon: don't use NVDIMM for ballooning

[Igor Mammedov]

On Wed, 03 Feb 2016 16:42:21 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Eric Blake <eblake@redhat.com> writes:
> 
> > On 01/27/2016 11:51 PM, Vladimir Sementsov-Ogievskiy wrote:  
> >> 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>
> >> 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>
> >> ---  
> >  
> >> @@ -308,7 +309,9 @@ static ram_addr_t get_current_ram_size(void)
> >>          if (value) {
> >>              switch (value->type) {
> >>              case MEMORY_DEVICE_INFO_KIND_DIMM:
> >> -                size += value->u.dimm->size;
> >> +                if (strcmp(value->u.dimm->type, TYPE_NVDIMM)) {  
> >
> > If you fix 2/3 to use a QAPI enum, then this will be an integer compare
> > instead of a strcmp().  
> 
> Another option is to add a flag to the subtypes of TYPE_PC_DIMM, set it
> for the subtypes that should be counted here, and accumulate the sizes
> of devices where the flag is set.  Requires iterating directly over the
> devices here (like qmp_pc_dimm_device_list() does under the hood) rather
> than the MemoryDeviceInfoList returned by qmp_pc_dimm_device_list(),
I'd rather not add flag to TYPE_PC_DIMM state/class as its purely balloon
qemu/guest driver limit and not related to [pc-|nv]in general.
Lets whitelist balloon supported type here.

Re: [Qemu-devel] [PATCH 3/3] balloon: don't use NVDIMM for ballooning

[Vladimir Sementsov-Ogievskiy]

On 03.02.2016 18:42, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> On 01/27/2016 11:51 PM, Vladimir Sementsov-Ogievskiy wrote:
>>> 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>
>>> 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>
>>> ---
>>> @@ -308,7 +309,9 @@ static ram_addr_t get_current_ram_size(void)
>>>           if (value) {
>>>               switch (value->type) {
>>>               case MEMORY_DEVICE_INFO_KIND_DIMM:
>>> -                size += value->u.dimm->size;
>>> +                if (strcmp(value->u.dimm->type, TYPE_NVDIMM)) {
>> If you fix 2/3 to use a QAPI enum, then this will be an integer compare
>> instead of a strcmp().
> Another option is to add a flag to the subtypes of TYPE_PC_DIMM, set it
> for the subtypes that should be counted here, and accumulate the sizes
> of devices where the flag is set.  Requires iterating directly over the
> devices here (like qmp_pc_dimm_device_list() does under the hood) rather
> than the MemoryDeviceInfoList returned by qmp_pc_dimm_device_list(),
It was my first approach but it was rejected)

As another option I can make a function iterating over the devices and 
return list of them, and then use it instead of 
qmp_pc_dimm_device_list.. Then, I'll have pointers to devices and can 
use object_dynamic_cast.

-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 3/3] balloon: don't use NVDIMM for ballooning

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> On 03.02.2016 18:42, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>>
>>> On 01/27/2016 11:51 PM, Vladimir Sementsov-Ogievskiy wrote:
>>>> 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>
>>>> 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>
>>>> ---
>>>> @@ -308,7 +309,9 @@ static ram_addr_t get_current_ram_size(void)
>>>>           if (value) {
>>>>               switch (value->type) {
>>>>               case MEMORY_DEVICE_INFO_KIND_DIMM:
>>>> -                size += value->u.dimm->size;
>>>> +                if (strcmp(value->u.dimm->type, TYPE_NVDIMM)) {
>>> If you fix 2/3 to use a QAPI enum, then this will be an integer compare
>>> instead of a strcmp().
>> Another option is to add a flag to the subtypes of TYPE_PC_DIMM, set it
>> for the subtypes that should be counted here, and accumulate the sizes
>> of devices where the flag is set.  Requires iterating directly over the
>> devices here (like qmp_pc_dimm_device_list() does under the hood) rather
>> than the MemoryDeviceInfoList returned by qmp_pc_dimm_device_list(),
> It was my first approach but it was rejected)
>
> As another option I can make a function iterating over the devices and
> return list of them, and then use it instead of
> qmp_pc_dimm_device_list.. Then, I'll have pointers to devices and can
> use object_dynamic_cast.

I fail to see how splitting a tree walk doing stuff into a tree walk
creating a list and a list walk doing stuff makes things better :)

Anyway, you guys figure it out.  The only part where I get involved is
QAPI design.

Re: [Qemu-devel] [PATCH 3/3] balloon: don't use NVDIMM for ballooning

[Vladimir Sementsov-Ogievskiy]

On 04.02.2016 09:20, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
>
>> On 03.02.2016 18:42, Markus Armbruster wrote:
>>> Eric Blake <eblake@redhat.com> writes:
>>>
>>>> On 01/27/2016 11:51 PM, Vladimir Sementsov-Ogievskiy wrote:
>>>>> 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>
>>>>> 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>
>>>>> ---
>>>>> @@ -308,7 +309,9 @@ static ram_addr_t get_current_ram_size(void)
>>>>>            if (value) {
>>>>>                switch (value->type) {
>>>>>                case MEMORY_DEVICE_INFO_KIND_DIMM:
>>>>> -                size += value->u.dimm->size;
>>>>> +                if (strcmp(value->u.dimm->type, TYPE_NVDIMM)) {
>>>> If you fix 2/3 to use a QAPI enum, then this will be an integer compare
>>>> instead of a strcmp().
>>> Another option is to add a flag to the subtypes of TYPE_PC_DIMM, set it
>>> for the subtypes that should be counted here, and accumulate the sizes
>>> of devices where the flag is set.  Requires iterating directly over the
>>> devices here (like qmp_pc_dimm_device_list() does under the hood) rather
>>> than the MemoryDeviceInfoList returned by qmp_pc_dimm_device_list(),
>> It was my first approach but it was rejected)
>>
>> As another option I can make a function iterating over the devices and
>> return list of them, and then use it instead of
>> qmp_pc_dimm_device_list.. Then, I'll have pointers to devices and can
>> use object_dynamic_cast.
> I fail to see how splitting a tree walk doing stuff into a tree walk
> creating a list and a list walk doing stuff makes things better :)

It will allow me not touch qapi)

>
> Anyway, you guys figure it out.  The only part where I get involved is
> QAPI design.



-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 3/3] balloon: don't use NVDIMM for ballooning

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> On 04.02.2016 09:20, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
>>
>>> On 03.02.2016 18:42, Markus Armbruster wrote:
>>>> Eric Blake <eblake@redhat.com> writes:
>>>>
>>>>> On 01/27/2016 11:51 PM, Vladimir Sementsov-Ogievskiy wrote:
>>>>>> 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>
>>>>>> 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>
>>>>>> ---
>>>>>> @@ -308,7 +309,9 @@ static ram_addr_t get_current_ram_size(void)
>>>>>>            if (value) {
>>>>>>                switch (value->type) {
>>>>>>                case MEMORY_DEVICE_INFO_KIND_DIMM:
>>>>>> -                size += value->u.dimm->size;
>>>>>> +                if (strcmp(value->u.dimm->type, TYPE_NVDIMM)) {
>>>>> If you fix 2/3 to use a QAPI enum, then this will be an integer compare
>>>>> instead of a strcmp().
>>>> Another option is to add a flag to the subtypes of TYPE_PC_DIMM, set it
>>>> for the subtypes that should be counted here, and accumulate the sizes
>>>> of devices where the flag is set.  Requires iterating directly over the
>>>> devices here (like qmp_pc_dimm_device_list() does under the hood) rather
>>>> than the MemoryDeviceInfoList returned by qmp_pc_dimm_device_list(),
>>> It was my first approach but it was rejected)

Sounds like your first approach was the right approach.

>>> As another option I can make a function iterating over the devices and
>>> return list of them, and then use it instead of
>>> qmp_pc_dimm_device_list.. Then, I'll have pointers to devices and can
>>> use object_dynamic_cast.
>> I fail to see how splitting a tree walk doing stuff into a tree walk
>> creating a list and a list walk doing stuff makes things better :)
>
> It will allow me not touch qapi)

Whatever it takes to get this fix past the gaggle of maintainers (I can
relate to that).

[...]

Re: [Qemu-devel] [PATCH 3/3] balloon: don't use NVDIMM for ballooning

[Vladimir Sementsov-Ogievskiy]

On 05.02.2016 12:53, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
>
>> On 04.02.2016 09:20, Markus Armbruster wrote:
>>> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
>>>
>>>> On 03.02.2016 18:42, Markus Armbruster wrote:
>>>>> Eric Blake <eblake@redhat.com> writes:
>>>>>
>>>>>> On 01/27/2016 11:51 PM, Vladimir Sementsov-Ogievskiy wrote:
>>>>>>> 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>
>>>>>>> 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>
>>>>>>> ---
>>>>>>> @@ -308,7 +309,9 @@ static ram_addr_t get_current_ram_size(void)
>>>>>>>             if (value) {
>>>>>>>                 switch (value->type) {
>>>>>>>                 case MEMORY_DEVICE_INFO_KIND_DIMM:
>>>>>>> -                size += value->u.dimm->size;
>>>>>>> +                if (strcmp(value->u.dimm->type, TYPE_NVDIMM)) {
>>>>>> If you fix 2/3 to use a QAPI enum, then this will be an integer compare
>>>>>> instead of a strcmp().
>>>>> Another option is to add a flag to the subtypes of TYPE_PC_DIMM, set it
>>>>> for the subtypes that should be counted here, and accumulate the sizes
>>>>> of devices where the flag is set.  Requires iterating directly over the
>>>>> devices here (like qmp_pc_dimm_device_list() does under the hood) rather
>>>>> than the MemoryDeviceInfoList returned by qmp_pc_dimm_device_list(),
>>>> It was my first approach but it was rejected)
> Sounds like your first approach was the right approach.
>
>>>> As another option I can make a function iterating over the devices and
>>>> return list of them, and then use it instead of
>>>> qmp_pc_dimm_device_list.. Then, I'll have pointers to devices and can
>>>> use object_dynamic_cast.
>>> I fail to see how splitting a tree walk doing stuff into a tree walk
>>> creating a list and a list walk doing stuff makes things better :)
>> It will allow me not touch qapi)
> Whatever it takes to get this fix past the gaggle of maintainers (I can
> relate to that).
>
> [...]

Everything is in your hands). I'm not mind if someone take any of this 
series versions, tune it as he wants and merge. It is a bug fix, but the 
discussion looks like I'm trying to add a new feature)


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v4 0/3] don't use NVDIMM for balooning

[Denis V. Lunev]

On 01/28/2016 09:51 AM, Vladimir Sementsov-Ogievskiy wrote:
> 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.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> 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>
>
> Vladimir Sementsov-Ogievskiy (3):
>    move get_current_ram_size to virtio-balloon.c
>    pcdimm: add 'type' field to PCDIMMDeviceInfo
>    balloon: don't use NVDIMM for ballooning
>
>   hw/mem/pc-dimm.c                | 27 +--------------------------
>   hw/virtio/virtio-balloon.c      | 29 +++++++++++++++++++++++++++++
>   include/exec/cpu-common.h       |  1 -
>   qapi-schema.json                |  5 ++++-
>   stubs/qmp_pc_dimm_device_list.c |  5 -----
>   5 files changed, 34 insertions(+), 33 deletions(-)
>
ping