On Fri, 15 Jun 2018 16:04:47 +0200 David Hildenbrand <da...@redhat.com> wrote:
> We might get a call to get_memory_region() before the device has been > realized. We should return a consistent value, as the return value > will e.g. later on be used in the pre_plug handler. > > To avoid duplicating too much code, factor the initialization and checks > out into a helper function. > > Signed-off-by: David Hildenbrand <da...@redhat.com> Reviewed-by: Igor Mammedov <imamm...@redhat.com> > --- > hw/mem/nvdimm.c | 46 +++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 37 insertions(+), 9 deletions(-) > > diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c > index df7646488b..deba5719bc 100644 > --- a/hw/mem/nvdimm.c > +++ b/hw/mem/nvdimm.c > @@ -71,20 +71,24 @@ static void nvdimm_init(Object *obj) > NULL, NULL); > } > > -static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm, Error > **errp) > +static void nvdimm_prepare_memory_region(NVDIMMDevice *nvdimm, Error **errp) > { > - NVDIMMDevice *nvdimm = NVDIMM(dimm); > + PCDIMMDevice *dimm = PC_DIMM(nvdimm); > + uint64_t align, pmem_size, size; > + MemoryRegion *mr; > > - return nvdimm->nvdimm_mr; > -} > + if (nvdimm->nvdimm_mr) { > + return; > + } > > -static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp) > -{ > - MemoryRegion *mr = host_memory_backend_get_memory(dimm->hostmem); > - NVDIMMDevice *nvdimm = NVDIMM(dimm); > - uint64_t align, pmem_size, size = memory_region_size(mr); > + if (!dimm->hostmem) { > + error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property must be set"); > + return; > + } > > + mr = host_memory_backend_get_memory(dimm->hostmem); > align = memory_region_get_alignment(mr); > + size = memory_region_size(mr); > > pmem_size = size - nvdimm->label_size; > nvdimm->label_data = memory_region_get_ram_ptr(mr) + pmem_size; > @@ -108,6 +112,30 @@ static void nvdimm_realize(PCDIMMDevice *dimm, Error > **errp) > nvdimm->nvdimm_mr->align = align; > } > > +static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm, Error > **errp) > +{ > + NVDIMMDevice *nvdimm = NVDIMM(dimm); > + Error *local_err = NULL; > + > + if (!nvdimm->nvdimm_mr) { > + nvdimm_prepare_memory_region(nvdimm, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return NULL; > + } > + } > + return nvdimm->nvdimm_mr; > +} > + > +static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp) > +{ > + NVDIMMDevice *nvdimm = NVDIMM(dimm); > + > + if (!nvdimm->nvdimm_mr) { > + nvdimm_prepare_memory_region(nvdimm, errp); > + } > +} > + > /* > * the caller should check the input parameters before calling > * label read/write functions.