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

2014-11-20 Thread Michael S. Tsirkin 
On Wed, Nov 19, 2014 at 09:31:35AM -0700, Eric Blake wrote:
 On 11/19/2014 09:06 AM, Michael S. Tsirkin wrote:
 
  This affects QMP right?
 
  I think later patches will tell how. CC'ing Eric.
 
  As far as I can tell, this is just correcting a reporting issue; the
  existing QMP commands/events for tracking balloon size will now properly
  account for hotplugged memory.
 
  What I don't know is if this change in semantics will affect any users.
   Libvirt is not yet supporting memory hotplug, so ideally, fixing this
  bug before libvirt uses memory hotplug means libvirt will never have to
  worry about qemu versions that do incorrect reporting.
 
  The alternative is to declare that the existing QMP commands cannot
  change in semantics for the existing members that it reports, and must
  instead report additional dictionary members describing the amount of
  hot-plugged memory, and then require that the client add the numbers
  together itself.  That sounds mean to the client, so I'm hoping we don't
  have to go there.
  
  
  IOW you ack this patch for 2.2?
  
 
 Is memory hotplug one of the new features in 2.2?  If so, then yes, we
 should get its semantics right from the start (this is a bug fix to
 avoid a release with broken semantics).  On the other hand, if hotplug
 existed in 2.1, then we already have a release with odd semantics, so
 delaying this fix until 2.3 and leaving 2.2 with the same odd semantics
 would not hurt, and it then becomes a judgment call of whether we are
 rushing in a possibly incomplete solution by trying to get this into
 2.2. (Sorry I haven't been following the history of memory hotplug closer)

AFAIK it's there since 2.0.


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

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

2014-11-19 Thread Igor Mammedov 
On Mon, 17 Nov 2014 13:11:08 +0800
zhanghailiang zhang.zhanghaili...@huawei.com wrote:

 The global parameter 'ram_size' does not take into account
 the hotplugged memory.
 
 In some codes, we use 'ram_size' as current VM's real RAM size,
 which is not correct.
 
 Add function 'get_current_ram_size' to calculate VM's current RAM
 size, it will enumerate present memory devices and also plus ram_size.
 
 Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
 ---
  hw/mem/pc-dimm.c| 26 ++
  include/exec/cpu-common.h   |  1 +
  stubs/qmp_pc_dimm_device_list.c |  5 +
  3 files changed, 32 insertions(+)
 
 diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
 index a800ea7..38465d0 100644
 --- a/hw/mem/pc-dimm.c
 +++ b/hw/mem/pc-dimm.c
 @@ -62,6 +62,32 @@ int qmp_pc_dimm_device_list(Object *obj, void
 *opaque) return 0;
  }
  
 +ram_addr_t get_current_ram_size(void)
 +{
 +MemoryDeviceInfoList *info_list = NULL;
 +MemoryDeviceInfoList **prev = info_list;

 +MemoryDeviceInfoList *info;
 +ram_addr_t size = ram_size;
 +
 +qmp_pc_dimm_device_list(qdev_get_machine(), prev);
 +for (info = info_list; info; info = info-next) {
 +MemoryDeviceInfo *value = info-value;
 +
 +if (value) {
 +switch (value-kind) {
 +case MEMORY_DEVICE_INFO_KIND_DIMM:
 +size += value-dimm-size;
 +break;
 +default:
 +break;
 +}
 +}
 +}
 +qapi_free_MemoryDeviceInfoList(info_list);
 +
 +return size;
 +}
function is a generic one and it could handle not only pc-dimm device
in future, so it would be better to put it in some device neutral place.

That would allow to drop following stub as well.

 +
  static int pc_dimm_slot2bitmap(Object *obj, void *opaque)
  {
  unsigned long *bitmap = opaque;
 diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
 index 427b851..fcc3162 100644
 --- a/include/exec/cpu-common.h
 +++ b/include/exec/cpu-common.h
 @@ -52,6 +52,7 @@ typedef uintptr_t ram_addr_t;
  #endif
  
  extern ram_addr_t ram_size;
 +ram_addr_t get_current_ram_size(void);
  
  /* memory API */
  
 diff --git a/stubs/qmp_pc_dimm_device_list.c
 b/stubs/qmp_pc_dimm_device_list.c index 5cb220c..b584bd8 100644
 --- a/stubs/qmp_pc_dimm_device_list.c
 +++ b/stubs/qmp_pc_dimm_device_list.c
 @@ -5,3 +5,8 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
  {
 return 0;
  }
 +
 +ram_addr_t get_current_ram_size(void)
 +{
 +return ram_size;
 +}

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

2014-11-19 Thread Michael S. Tsirkin 
On Mon, Nov 17, 2014 at 01:11:08PM +0800, zhanghailiang wrote:
 The global parameter 'ram_size' does not take into account
 the hotplugged memory.
 
 In some codes, we use 'ram_size' as current VM's real RAM size,
 which is not correct.
 
 Add function 'get_current_ram_size' to calculate VM's current RAM size,
 it will enumerate present memory devices and also plus ram_size.
 
 Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com


This affects QMP right?
Cc Luiz.


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

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

2014-11-19 Thread Luiz Capitulino 
On Wed, 19 Nov 2014 12:32:46 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Mon, Nov 17, 2014 at 01:11:08PM +0800, zhanghailiang wrote:
  The global parameter 'ram_size' does not take into account
  the hotplugged memory.
  
  In some codes, we use 'ram_size' as current VM's real RAM size,
  which is not correct.
  
  Add function 'get_current_ram_size' to calculate VM's current RAM size,
  it will enumerate present memory devices and also plus ram_size.
  
  Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
 
 
 This affects QMP right?

I think later patches will tell how. CC'ing Eric.

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

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

2014-11-19 Thread Eric Blake 
On 11/19/2014 08:13 AM, Luiz Capitulino wrote:
 On Wed, 19 Nov 2014 12:32:46 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
 On Mon, Nov 17, 2014 at 01:11:08PM +0800, zhanghailiang wrote:
 The global parameter 'ram_size' does not take into account
 the hotplugged memory.

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

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

 Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com


 This affects QMP right?
 
 I think later patches will tell how. CC'ing Eric.

As far as I can tell, this is just correcting a reporting issue; the
existing QMP commands/events for tracking balloon size will now properly
account for hotplugged memory.

What I don't know is if this change in semantics will affect any users.
 Libvirt is not yet supporting memory hotplug, so ideally, fixing this
bug before libvirt uses memory hotplug means libvirt will never have to
worry about qemu versions that do incorrect reporting.

The alternative is to declare that the existing QMP commands cannot
change in semantics for the existing members that it reports, and must
instead report additional dictionary members describing the amount of
hot-plugged memory, and then require that the client add the numbers
together itself.  That sounds mean to the client, so I'm hoping we don't
have to go there.

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



signature.asc
Description: OpenPGP digital signature

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

2014-11-19 Thread Michael S. Tsirkin 
On Wed, Nov 19, 2014 at 08:52:19AM -0700, Eric Blake wrote:
 On 11/19/2014 08:13 AM, Luiz Capitulino wrote:
  On Wed, 19 Nov 2014 12:32:46 +0200
  Michael S. Tsirkin m...@redhat.com wrote:
  
  On Mon, Nov 17, 2014 at 01:11:08PM +0800, zhanghailiang wrote:
  The global parameter 'ram_size' does not take into account
  the hotplugged memory.
 
  In some codes, we use 'ram_size' as current VM's real RAM size,
  which is not correct.
 
  Add function 'get_current_ram_size' to calculate VM's current RAM size,
  it will enumerate present memory devices and also plus ram_size.
 
  Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
 
 
  This affects QMP right?
  
  I think later patches will tell how. CC'ing Eric.
 
 As far as I can tell, this is just correcting a reporting issue; the
 existing QMP commands/events for tracking balloon size will now properly
 account for hotplugged memory.
 
 What I don't know is if this change in semantics will affect any users.
  Libvirt is not yet supporting memory hotplug, so ideally, fixing this
 bug before libvirt uses memory hotplug means libvirt will never have to
 worry about qemu versions that do incorrect reporting.
 
 The alternative is to declare that the existing QMP commands cannot
 change in semantics for the existing members that it reports, and must
 instead report additional dictionary members describing the amount of
 hot-plugged memory, and then require that the client add the numbers
 together itself.  That sounds mean to the client, so I'm hoping we don't
 have to go there.


IOW you ack this patch for 2.2?

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

2014-11-19 Thread Eric Blake 
On 11/19/2014 09:06 AM, Michael S. Tsirkin wrote:

 This affects QMP right?

 I think later patches will tell how. CC'ing Eric.

 As far as I can tell, this is just correcting a reporting issue; the
 existing QMP commands/events for tracking balloon size will now properly
 account for hotplugged memory.

 What I don't know is if this change in semantics will affect any users.
  Libvirt is not yet supporting memory hotplug, so ideally, fixing this
 bug before libvirt uses memory hotplug means libvirt will never have to
 worry about qemu versions that do incorrect reporting.

 The alternative is to declare that the existing QMP commands cannot
 change in semantics for the existing members that it reports, and must
 instead report additional dictionary members describing the amount of
 hot-plugged memory, and then require that the client add the numbers
 together itself.  That sounds mean to the client, so I'm hoping we don't
 have to go there.
 
 
 IOW you ack this patch for 2.2?
 

Is memory hotplug one of the new features in 2.2?  If so, then yes, we
should get its semantics right from the start (this is a bug fix to
avoid a release with broken semantics).  On the other hand, if hotplug
existed in 2.1, then we already have a release with odd semantics, so
delaying this fix until 2.3 and leaving 2.2 with the same odd semantics
would not hurt, and it then becomes a judgment call of whether we are
rushing in a possibly incomplete solution by trying to get this into
2.2. (Sorry I haven't been following the history of memory hotplug closer)


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



signature.asc
Description: OpenPGP digital signature

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

2014-11-19 Thread zhanghailiang 

On 2014/11/20 0:31, Eric Blake wrote:

On 11/19/2014 09:06 AM, Michael S. Tsirkin wrote:


This affects QMP right?


I think later patches will tell how. CC'ing Eric.


As far as I can tell, this is just correcting a reporting issue; the
existing QMP commands/events for tracking balloon size will now properly
account for hotplugged memory.

What I don't know is if this change in semantics will affect any users.
  Libvirt is not yet supporting memory hotplug, so ideally, fixing this
bug before libvirt uses memory hotplug means libvirt will never have to
worry about qemu versions that do incorrect reporting.

The alternative is to declare that the existing QMP commands cannot
change in semantics for the existing members that it reports, and must
instead report additional dictionary members describing the amount of
hot-plugged memory, and then require that the client add the numbers
together itself.  That sounds mean to the client, so I'm hoping we don't
have to go there.



IOW you ack this patch for 2.2?



Is memory hotplug one of the new features in 2.2?  If so, then yes, we


No, after searching this feature in ChangeLogs, i found pc-dimm memory hotplug
was supported since 2.1. It only supports for x86 target.

And one more thing, i found in 2.2's Changelog it begin to support memory 
hotplug
for s390 target, I'm not sure whether this problem also exists for s390.


should get its semantics right from the start (this is a bug fix to
avoid a release with broken semantics).  On the other hand, if hotplug
existed in 2.1, then we already have a release with odd semantics, so
delaying this fix until 2.3 and leaving 2.2 with the same odd semantics
would not hurt, and it then becomes a judgment call of whether we are
rushing in a possibly incomplete solution by trying to get this into
2.2. (Sorry I haven't been following the history of memory hotplug closer)

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

2014-11-19 Thread zhanghailiang 

On 2014/11/19 18:32, Michael S. Tsirkin wrote:

On Mon, Nov 17, 2014 at 01:11:08PM +0800, zhanghailiang wrote:

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

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

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

Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com



This affects QMP right?


Yes, will affect the results of balloon commands ;)


Cc Luiz.



---
  hw/mem/pc-dimm.c| 26 ++
  include/exec/cpu-common.h   |  1 +
  stubs/qmp_pc_dimm_device_list.c |  5 +
  3 files changed, 32 insertions(+)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index a800ea7..38465d0 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -62,6 +62,32 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
  return 0;
  }

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

  extern ram_addr_t ram_size;
+ram_addr_t get_current_ram_size(void);

  /* memory API */

diff --git a/stubs/qmp_pc_dimm_device_list.c b/stubs/qmp_pc_dimm_device_list.c
index 5cb220c..b584bd8 100644
--- a/stubs/qmp_pc_dimm_device_list.c
+++ b/stubs/qmp_pc_dimm_device_list.c
@@ -5,3 +5,8 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
  {
 return 0;
  }
+
+ram_addr_t get_current_ram_size(void)
+{
+return ram_size;
+}
--
1.7.12.4



.

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

2014-11-16 Thread zhanghailiang 
The global parameter 'ram_size' does not take into account
the hotplugged memory.

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

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

Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
---
 hw/mem/pc-dimm.c| 26 ++
 include/exec/cpu-common.h   |  1 +
 stubs/qmp_pc_dimm_device_list.c |  5 +
 3 files changed, 32 insertions(+)

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