| Message ID | 20180920103243.28474-6-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | memory-device: complete refactoring + virtio-pmem | expand |
On Thu, Sep 20, 2018 at 12:32:26PM +0200, David Hildenbrand wrote: > Let's properly forward the errors, so errors from get_region_size() / > get_plugged_size() can be handled. > > Users right now call both functions after the device has been realized, > which is will never fail, so it is fine to continue using error_abort. > > While at it, remove a leftover error check (suggedted by Igor). > > Signed-off-by: David Hildenbrand <david@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > hw/mem/memory-device.c | 9 +++------ > hw/mem/pc-dimm.c | 5 +++-- > include/hw/mem/memory-device.h | 4 ++-- > 3 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c > index 909566756a..bdcee6fd55 100644 > --- a/hw/mem/memory-device.c > +++ b/hw/mem/memory-device.c > @@ -60,7 +60,7 @@ static int memory_device_used_region_size(Object *obj, void *opaque) > const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(obj); > > if (dev->realized) { > - *size += mdc->get_region_size(md); > + *size += mdc->get_region_size(md, &error_abort); > } > } > > @@ -169,10 +169,7 @@ uint64_t memory_device_get_free_addr(MachineState *ms, const uint64_t *hint, > uint64_t md_size, md_addr; > > md_addr = mdc->get_addr(md); > - md_size = mdc->get_region_size(md); > - if (*errp) { > - goto out; > - } > + md_size = mdc->get_region_size(md, &error_abort); > > if (ranges_overlap(md_addr, md_size, new_addr, size)) { > if (hint) { > @@ -236,7 +233,7 @@ static int memory_device_plugged_size(Object *obj, void *opaque) > const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(obj); > > if (dev->realized) { > - *size += mdc->get_plugged_size(md); > + *size += mdc->get_plugged_size(md, &error_abort); > } > } > > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c > index fb6bcaedc4..4bf1a0acc9 100644 > --- a/hw/mem/pc-dimm.c > +++ b/hw/mem/pc-dimm.c > @@ -236,14 +236,15 @@ static uint64_t pc_dimm_md_get_addr(const MemoryDeviceState *md) > return dimm->addr; > } > > -static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md) > +static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md, > + Error **errp) > { > /* dropping const here is fine as we don't touch the memory region */ > PCDIMMDevice *dimm = PC_DIMM(md); > const PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(md); > MemoryRegion *mr; > > - mr = ddc->get_memory_region(dimm, &error_abort); > + mr = ddc->get_memory_region(dimm, errp); > if (!mr) { > return 0; > } > diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h > index 2853b084b5..f02b229837 100644 > --- a/include/hw/mem/memory-device.h > +++ b/include/hw/mem/memory-device.h > @@ -33,8 +33,8 @@ typedef struct MemoryDeviceClass { > InterfaceClass parent_class; > > uint64_t (*get_addr)(const MemoryDeviceState *md); > - uint64_t (*get_plugged_size)(const MemoryDeviceState *md); > - uint64_t (*get_region_size)(const MemoryDeviceState *md); > + uint64_t (*get_plugged_size)(const MemoryDeviceState *md, Error **errp); > + uint64_t (*get_region_size)(const MemoryDeviceState *md, Error **errp); > void (*fill_device_info)(const MemoryDeviceState *md, > MemoryDeviceInfo *info); > } MemoryDeviceClass;
On Thu, 20 Sep 2018 12:32:26 +0200 David Hildenbrand <david@redhat.com> wrote: > Let's properly forward the errors, so errors from get_region_size() / > get_plugged_size() can be handled. > > Users right now call both functions after the device has been realized, > which is will never fail, so it is fine to continue using error_abort. > > While at it, remove a leftover error check (suggedted by Igor). > > Signed-off-by: David Hildenbrand <david@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/mem/memory-device.c | 9 +++------ > hw/mem/pc-dimm.c | 5 +++-- > include/hw/mem/memory-device.h | 4 ++-- > 3 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c > index 909566756a..bdcee6fd55 100644 > --- a/hw/mem/memory-device.c > +++ b/hw/mem/memory-device.c > @@ -60,7 +60,7 @@ static int memory_device_used_region_size(Object *obj, void *opaque) > const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(obj); > > if (dev->realized) { > - *size += mdc->get_region_size(md); > + *size += mdc->get_region_size(md, &error_abort); > } > } > > @@ -169,10 +169,7 @@ uint64_t memory_device_get_free_addr(MachineState *ms, const uint64_t *hint, > uint64_t md_size, md_addr; > > md_addr = mdc->get_addr(md); > - md_size = mdc->get_region_size(md); > - if (*errp) { > - goto out; > - } > + md_size = mdc->get_region_size(md, &error_abort); > > if (ranges_overlap(md_addr, md_size, new_addr, size)) { > if (hint) { > @@ -236,7 +233,7 @@ static int memory_device_plugged_size(Object *obj, void *opaque) > const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(obj); > > if (dev->realized) { > - *size += mdc->get_plugged_size(md); > + *size += mdc->get_plugged_size(md, &error_abort); > } > } > > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c > index fb6bcaedc4..4bf1a0acc9 100644 > --- a/hw/mem/pc-dimm.c > +++ b/hw/mem/pc-dimm.c > @@ -236,14 +236,15 @@ static uint64_t pc_dimm_md_get_addr(const MemoryDeviceState *md) > return dimm->addr; > } > > -static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md) > +static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md, > + Error **errp) > { > /* dropping const here is fine as we don't touch the memory region */ > PCDIMMDevice *dimm = PC_DIMM(md); > const PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(md); > MemoryRegion *mr; > > - mr = ddc->get_memory_region(dimm, &error_abort); > + mr = ddc->get_memory_region(dimm, errp); > if (!mr) { > return 0; > } > diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h > index 2853b084b5..f02b229837 100644 > --- a/include/hw/mem/memory-device.h > +++ b/include/hw/mem/memory-device.h > @@ -33,8 +33,8 @@ typedef struct MemoryDeviceClass { > InterfaceClass parent_class; > > uint64_t (*get_addr)(const MemoryDeviceState *md); > - uint64_t (*get_plugged_size)(const MemoryDeviceState *md); > - uint64_t (*get_region_size)(const MemoryDeviceState *md); > + uint64_t (*get_plugged_size)(const MemoryDeviceState *md, Error **errp); > + uint64_t (*get_region_size)(const MemoryDeviceState *md, Error **errp); > void (*fill_device_info)(const MemoryDeviceState *md, > MemoryDeviceInfo *info); > } MemoryDeviceClass;
diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index 909566756a..bdcee6fd55 100644 --- a/hw/mem/memory-device.c +++ b/hw/mem/memory-device.c @@ -60,7 +60,7 @@ static int memory_device_used_region_size(Object *obj, void *opaque) const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(obj); if (dev->realized) { - *size += mdc->get_region_size(md); + *size += mdc->get_region_size(md, &error_abort); } } @@ -169,10 +169,7 @@ uint64_t memory_device_get_free_addr(MachineState *ms, const uint64_t *hint, uint64_t md_size, md_addr; md_addr = mdc->get_addr(md); - md_size = mdc->get_region_size(md); - if (*errp) { - goto out; - } + md_size = mdc->get_region_size(md, &error_abort); if (ranges_overlap(md_addr, md_size, new_addr, size)) { if (hint) { @@ -236,7 +233,7 @@ static int memory_device_plugged_size(Object *obj, void *opaque) const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(obj); if (dev->realized) { - *size += mdc->get_plugged_size(md); + *size += mdc->get_plugged_size(md, &error_abort); } } diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index fb6bcaedc4..4bf1a0acc9 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -236,14 +236,15 @@ static uint64_t pc_dimm_md_get_addr(const MemoryDeviceState *md) return dimm->addr; } -static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md) +static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md, + Error **errp) { /* dropping const here is fine as we don't touch the memory region */ PCDIMMDevice *dimm = PC_DIMM(md); const PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(md); MemoryRegion *mr; - mr = ddc->get_memory_region(dimm, &error_abort); + mr = ddc->get_memory_region(dimm, errp); if (!mr) { return 0; } diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h index 2853b084b5..f02b229837 100644 --- a/include/hw/mem/memory-device.h +++ b/include/hw/mem/memory-device.h @@ -33,8 +33,8 @@ typedef struct MemoryDeviceClass { InterfaceClass parent_class; uint64_t (*get_addr)(const MemoryDeviceState *md); - uint64_t (*get_plugged_size)(const MemoryDeviceState *md); - uint64_t (*get_region_size)(const MemoryDeviceState *md); + uint64_t (*get_plugged_size)(const MemoryDeviceState *md, Error **errp); + uint64_t (*get_region_size)(const MemoryDeviceState *md, Error **errp); void (*fill_device_info)(const MemoryDeviceState *md, MemoryDeviceInfo *info); } MemoryDeviceClass;
Let's properly forward the errors, so errors from get_region_size() / get_plugged_size() can be handled. Users right now call both functions after the device has been realized, which is will never fail, so it is fine to continue using error_abort. While at it, remove a leftover error check (suggedted by Igor). Signed-off-by: David Hildenbrand <david@redhat.com> --- hw/mem/memory-device.c | 9 +++------ hw/mem/pc-dimm.c | 5 +++-- include/hw/mem/memory-device.h | 4 ++-- 3 files changed, 8 insertions(+), 10 deletions(-)