[Qemu-devel] [PATCH v1 05/11] spapr: move memory hotplug size check into plug code

[David Hildenbrand <david@redhat.com>]

This might look like a step backwards, but it is not. get_memory_region()
should not be called on uninititalized devices. In general, only
properties should be access, but no "derived" satte like the memory
region.

1. We need duplicate error checks if memdev is actually already set.
   realize() performs these checks, no need to duplicate.
2. This is bad practise as one can see when looking at the NVDIMM
   implemetation. The call does not return sane data before the device
   is realized. Although spapr does not use NVDIMM, conceptually it is
   wrong.

So let's just move this call to the right place. We can then cleanup
get_memory_region().

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/ppc/spapr.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f59999daac..a5f1bbd58a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3153,6 +3153,12 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
     align = memory_region_get_alignment(mr);
     size = memory_region_size(mr);
 
+    if (size % SPAPR_MEMORY_BLOCK_SIZE) {
+        error_setg(&local_err, "Hotplugged memory size must be a multiple of "
+                   "%lld MB", SPAPR_MEMORY_BLOCK_SIZE / M_BYTE);
+        goto out;
+    }
+
     pc_dimm_memory_plug(dev, MACHINE(ms), align, &local_err);
     if (local_err) {
         goto out;
@@ -3186,9 +3192,6 @@ static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
 {
     const sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(hotplug_dev);
     PCDIMMDevice *dimm = PC_DIMM(dev);
-    PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
-    MemoryRegion *mr;
-    uint64_t size;
     char *mem_dev;
 
     if (!smc->dr_lmb_enabled) {
@@ -3196,18 +3199,6 @@ static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
         return;
     }
 
-    mr = ddc->get_memory_region(dimm, errp);
-    if (!mr) {
-        return;
-    }
-    size = memory_region_size(mr);
-
-    if (size % SPAPR_MEMORY_BLOCK_SIZE) {
-        error_setg(errp, "Hotplugged memory size must be a multiple of "
-                      "%lld MB", SPAPR_MEMORY_BLOCK_SIZE / M_BYTE);
-        return;
-    }
-
     mem_dev = object_property_get_str(OBJECT(dimm), PC_DIMM_MEMDEV_PROP, NULL);
     if (mem_dev && !kvmppc_is_mem_backend_page_size_ok(mem_dev)) {
         error_setg(errp, "Memory backend has bad page size. "
-- 
2.17.1