* [Qemu-devel] [PATCH v1 1/3] pc: Fix DIMMs capacity calculation
2015-01-23 3:37 [Qemu-devel] [PATCH v1 0/3] Fix pc DIMMs capacity calculation Bharata B Rao
@ 2015-01-23 3:37 ` Bharata B Rao
2015-01-23 3:37 ` [Qemu-devel] [PATCH v1 2/3] pc-dimm: Make pc_existing_dimms_capacity global Bharata B Rao
2015-01-23 3:37 ` [Qemu-devel] [PATCH 3/3] pc-dimm: Add Error argument to pc_existing_dimms_capacity Bharata B Rao
2 siblings, 0 replies; 6+ messages in thread
From: Bharata B Rao @ 2015-01-23 3:37 UTC (permalink / raw)
To: qemu-devel; +Cc: imammedo, Bharata B Rao
pc_existing_dimms_capacity() is returning DIMMs count rather than capacity.
Fix this to return the capacity. Also consider only realized devices for
capacity calculation.
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/pc.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e07f1fa..125cf0a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1552,25 +1552,18 @@ void qemu_register_pc_machine(QEMUMachine *m)
g_free(name);
}
-static int pc_dimm_count(Object *obj, void *opaque)
-{
- int *count = opaque;
-
- if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
- (*count)++;
- }
-
- object_child_foreach(obj, pc_dimm_count, opaque);
- return 0;
-}
-
static int pc_existing_dimms_capacity(Object *obj, void *opaque)
{
Error *local_err = NULL;
uint64_t *size = opaque;
if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
- (*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP, &local_err);
+ DeviceState *dev = DEVICE(obj);
+
+ if (dev->realized) {
+ (*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
+ &local_err);
+ }
if (local_err) {
qerror_report_err(local_err);
@@ -1579,7 +1572,7 @@ static int pc_existing_dimms_capacity(Object *obj, void *opaque)
}
}
- object_child_foreach(obj, pc_dimm_count, opaque);
+ object_child_foreach(obj, pc_existing_dimms_capacity, opaque);
return 0;
}
@@ -1623,8 +1616,9 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
if (existing_dimms_capacity + memory_region_size(mr) >
machine->maxram_size - machine->ram_size) {
error_setg(&local_err, "not enough space, currently 0x%" PRIx64
- " in use of total 0x" RAM_ADDR_FMT,
- existing_dimms_capacity, machine->maxram_size);
+ " in use of total hot pluggable 0x" RAM_ADDR_FMT,
+ existing_dimms_capacity,
+ machine->maxram_size - machine->ram_size);
goto out;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v1 2/3] pc-dimm: Make pc_existing_dimms_capacity global
2015-01-23 3:37 [Qemu-devel] [PATCH v1 0/3] Fix pc DIMMs capacity calculation Bharata B Rao
2015-01-23 3:37 ` [Qemu-devel] [PATCH v1 1/3] pc: Fix " Bharata B Rao
@ 2015-01-23 3:37 ` Bharata B Rao
2015-01-23 12:17 ` Igor Mammedov
2015-01-23 3:37 ` [Qemu-devel] [PATCH 3/3] pc-dimm: Add Error argument to pc_existing_dimms_capacity Bharata B Rao
2 siblings, 1 reply; 6+ messages in thread
From: Bharata B Rao @ 2015-01-23 3:37 UTC (permalink / raw)
To: qemu-devel; +Cc: imammedo, Bharata B Rao
Move pc_existing_dimms_capacity() to pc-dimm.c since it would be needed
by PowerPC memory hotplug code too.
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
hw/i386/pc.c | 24 ------------------------
hw/mem/pc-dimm.c | 25 +++++++++++++++++++++++++
include/hw/mem/pc-dimm.h | 1 +
3 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 125cf0a..2ec45a4 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1552,30 +1552,6 @@ void qemu_register_pc_machine(QEMUMachine *m)
g_free(name);
}
-static int pc_existing_dimms_capacity(Object *obj, void *opaque)
-{
- Error *local_err = NULL;
- uint64_t *size = opaque;
-
- if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
- DeviceState *dev = DEVICE(obj);
-
- if (dev->realized) {
- (*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
- &local_err);
- }
-
- if (local_err) {
- qerror_report_err(local_err);
- error_free(local_err);
- return 1;
- }
- }
-
- object_child_foreach(obj, pc_existing_dimms_capacity, opaque);
- return 0;
-}
-
static void pc_dimm_plug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index d431834..f02ce6e 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -22,6 +22,31 @@
#include "qemu/config-file.h"
#include "qapi/visitor.h"
#include "qemu/range.h"
+#include "qapi/qmp/qerror.h"
+
+int pc_existing_dimms_capacity(Object *obj, void *opaque)
+{
+ Error *local_err = NULL;
+ uint64_t *size = opaque;
+
+ if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
+ DeviceState *dev = DEVICE(obj);
+
+ if (dev->realized) {
+ (*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
+ &local_err);
+ }
+
+ if (local_err) {
+ qerror_report_err(local_err);
+ error_free(local_err);
+ return 1;
+ }
+ }
+
+ object_child_foreach(obj, pc_existing_dimms_capacity, opaque);
+ return 0;
+}
int qmp_pc_dimm_device_list(Object *obj, void *opaque)
{
diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index e1dcbbc..bbfa53f 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -78,4 +78,5 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp);
int qmp_pc_dimm_device_list(Object *obj, void *opaque);
+int pc_existing_dimms_capacity(Object *obj, void *opaque);
#endif
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v1 2/3] pc-dimm: Make pc_existing_dimms_capacity global
2015-01-23 3:37 ` [Qemu-devel] [PATCH v1 2/3] pc-dimm: Make pc_existing_dimms_capacity global Bharata B Rao
@ 2015-01-23 12:17 ` Igor Mammedov
0 siblings, 0 replies; 6+ messages in thread
From: Igor Mammedov @ 2015-01-23 12:17 UTC (permalink / raw)
To: Bharata B Rao; +Cc: qemu-devel
On Fri, 23 Jan 2015 09:07:02 +0530
Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:
> Move pc_existing_dimms_capacity() to pc-dimm.c since it would be needed
> by PowerPC memory hotplug code too.
>
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/pc.c | 24 ------------------------
> hw/mem/pc-dimm.c | 25 +++++++++++++++++++++++++
> include/hw/mem/pc-dimm.h | 1 +
> 3 files changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 125cf0a..2ec45a4 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1552,30 +1552,6 @@ void qemu_register_pc_machine(QEMUMachine *m)
> g_free(name);
> }
>
> -static int pc_existing_dimms_capacity(Object *obj, void *opaque)
> -{
> - Error *local_err = NULL;
> - uint64_t *size = opaque;
> -
> - if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
> - DeviceState *dev = DEVICE(obj);
> -
> - if (dev->realized) {
> - (*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
> - &local_err);
> - }
> -
> - if (local_err) {
> - qerror_report_err(local_err);
> - error_free(local_err);
> - return 1;
> - }
> - }
> -
> - object_child_foreach(obj, pc_existing_dimms_capacity, opaque);
> - return 0;
> -}
> -
> static void pc_dimm_plug(HotplugHandler *hotplug_dev,
> DeviceState *dev, Error **errp)
> {
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index d431834..f02ce6e 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -22,6 +22,31 @@
> #include "qemu/config-file.h"
> #include "qapi/visitor.h"
> #include "qemu/range.h"
> +#include "qapi/qmp/qerror.h"
> +
> +int pc_existing_dimms_capacity(Object *obj, void *opaque)
> +{
> + Error *local_err = NULL;
> + uint64_t *size = opaque;
> +
> + if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
> + DeviceState *dev = DEVICE(obj);
> +
> + if (dev->realized) {
> + (*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
> + &local_err);
> + }
> +
> + if (local_err) {
> + qerror_report_err(local_err);
> + error_free(local_err);
> + return 1;
> + }
> + }
> +
> + object_child_foreach(obj, pc_existing_dimms_capacity, opaque);
> + return 0;
> +}
>
> int qmp_pc_dimm_device_list(Object *obj, void *opaque)
> {
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index e1dcbbc..bbfa53f 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -78,4 +78,5 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
> int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp);
>
> int qmp_pc_dimm_device_list(Object *obj, void *opaque);
> +int pc_existing_dimms_capacity(Object *obj, void *opaque);
> #endif
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 3/3] pc-dimm: Add Error argument to pc_existing_dimms_capacity
2015-01-23 3:37 [Qemu-devel] [PATCH v1 0/3] Fix pc DIMMs capacity calculation Bharata B Rao
2015-01-23 3:37 ` [Qemu-devel] [PATCH v1 1/3] pc: Fix " Bharata B Rao
2015-01-23 3:37 ` [Qemu-devel] [PATCH v1 2/3] pc-dimm: Make pc_existing_dimms_capacity global Bharata B Rao
@ 2015-01-23 3:37 ` Bharata B Rao
2015-01-23 12:15 ` Igor Mammedov
2 siblings, 1 reply; 6+ messages in thread
From: Bharata B Rao @ 2015-01-23 3:37 UTC (permalink / raw)
To: qemu-devel; +Cc: imammedo, Bharata B Rao
Now that pc_existing_dimms_capacity() is an API, include Error pointer
as an argument and modify the caller appropriately.
Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
hw/i386/pc.c | 4 ++--
hw/mem/pc-dimm.c | 31 ++++++++++++++++++++++---------
include/hw/mem/pc-dimm.h | 2 +-
3 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2ec45a4..c7af6aa 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1584,8 +1584,8 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
goto out;
}
- if (pc_existing_dimms_capacity(OBJECT(machine), &existing_dimms_capacity)) {
- error_setg(&local_err, "failed to get total size of existing DIMMs");
+ existing_dimms_capacity = pc_existing_dimms_capacity(&local_err);
+ if (local_err) {
goto out;
}
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index f02ce6e..3e908b3 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -24,30 +24,43 @@
#include "qemu/range.h"
#include "qapi/qmp/qerror.h"
-int pc_existing_dimms_capacity(Object *obj, void *opaque)
+typedef struct pc_dimms_capacity {
+ uint64_t size;
+ Error **errp;
+} pc_dimms_capacity;
+
+static int pc_existing_dimms_capacity_internal(Object *obj, void *opaque)
{
- Error *local_err = NULL;
- uint64_t *size = opaque;
+ pc_dimms_capacity *cap = opaque;
+ uint64_t *size = &cap->size;
if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
DeviceState *dev = DEVICE(obj);
if (dev->realized) {
(*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
- &local_err);
+ cap->errp);
}
- if (local_err) {
- qerror_report_err(local_err);
- error_free(local_err);
+ if (cap->errp && *cap->errp) {
return 1;
}
}
-
- object_child_foreach(obj, pc_existing_dimms_capacity, opaque);
+ object_child_foreach(obj, pc_existing_dimms_capacity_internal, opaque);
return 0;
}
+uint64_t pc_existing_dimms_capacity(Error **errp)
+{
+ pc_dimms_capacity cap;
+
+ cap.size = 0;
+ cap.errp = errp;
+
+ pc_existing_dimms_capacity_internal(qdev_get_machine(), &cap);
+ return cap.size;
+}
+
int qmp_pc_dimm_device_list(Object *obj, void *opaque)
{
MemoryDeviceInfoList ***prev = opaque;
diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index bbfa53f..f7b80b4 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -78,5 +78,5 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp);
int qmp_pc_dimm_device_list(Object *obj, void *opaque);
-int pc_existing_dimms_capacity(Object *obj, void *opaque);
+uint64_t pc_existing_dimms_capacity(Error **errp);
#endif
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH 3/3] pc-dimm: Add Error argument to pc_existing_dimms_capacity
2015-01-23 3:37 ` [Qemu-devel] [PATCH 3/3] pc-dimm: Add Error argument to pc_existing_dimms_capacity Bharata B Rao
@ 2015-01-23 12:15 ` Igor Mammedov
0 siblings, 0 replies; 6+ messages in thread
From: Igor Mammedov @ 2015-01-23 12:15 UTC (permalink / raw)
To: Bharata B Rao; +Cc: qemu-devel
On Fri, 23 Jan 2015 09:07:03 +0530
Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:
> Now that pc_existing_dimms_capacity() is an API, include Error pointer
> as an argument and modify the caller appropriately.
>
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
> hw/i386/pc.c | 4 ++--
> hw/mem/pc-dimm.c | 31 ++++++++++++++++++++++---------
> include/hw/mem/pc-dimm.h | 2 +-
> 3 files changed, 25 insertions(+), 12 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 2ec45a4..c7af6aa 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1584,8 +1584,8 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
> goto out;
> }
>
> - if (pc_existing_dimms_capacity(OBJECT(machine), &existing_dimms_capacity)) {
> - error_setg(&local_err, "failed to get total size of existing DIMMs");
> + existing_dimms_capacity = pc_existing_dimms_capacity(&local_err);
> + if (local_err) {
> goto out;
> }
>
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index f02ce6e..3e908b3 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -24,30 +24,43 @@
> #include "qemu/range.h"
> #include "qapi/qmp/qerror.h"
Do you still need ^^^ that was added in previous patch for the sake of
qerror_report_err()?
>
> -int pc_existing_dimms_capacity(Object *obj, void *opaque)
> +typedef struct pc_dimms_capacity {
> + uint64_t size;
> + Error **errp;
> +} pc_dimms_capacity;
> +
> +static int pc_existing_dimms_capacity_internal(Object *obj, void *opaque)
> {
> - Error *local_err = NULL;
> - uint64_t *size = opaque;
> + pc_dimms_capacity *cap = opaque;
> + uint64_t *size = &cap->size;
>
> if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
> DeviceState *dev = DEVICE(obj);
>
> if (dev->realized) {
> (*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
> - &local_err);
> + cap->errp);
> }
>
> - if (local_err) {
> - qerror_report_err(local_err);
> - error_free(local_err);
> + if (cap->errp && *cap->errp) {
> return 1;
> }
> }
> -
> - object_child_foreach(obj, pc_existing_dimms_capacity, opaque);
> + object_child_foreach(obj, pc_existing_dimms_capacity_internal, opaque);
> return 0;
> }
>
> +uint64_t pc_existing_dimms_capacity(Error **errp)
> +{
> + pc_dimms_capacity cap;
> +
> + cap.size = 0;
> + cap.errp = errp;
> +
> + pc_existing_dimms_capacity_internal(qdev_get_machine(), &cap);
> + return cap.size;
> +}
> +
> int qmp_pc_dimm_device_list(Object *obj, void *opaque)
> {
> MemoryDeviceInfoList ***prev = opaque;
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index bbfa53f..f7b80b4 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -78,5 +78,5 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
> int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp);
>
> int qmp_pc_dimm_device_list(Object *obj, void *opaque);
> -int pc_existing_dimms_capacity(Object *obj, void *opaque);
> +uint64_t pc_existing_dimms_capacity(Error **errp);
> #endif
^ permalink raw reply [flat|nested] 6+ messages in thread