diff mbox

[v7,RESEND,4/8] memory: add parameter errp to memory_region_init_rom_device

Message ID 540EE299.3080104@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Sept. 9, 2014, 11:20 a.m. UTC 
Il 09/09/2014 07:27, Hu Tao ha scritto:
> Add parameter errp to memory_region_init_rom_device and update all call
> sites to pass in &error_abort.
> 
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>

Better not use error_abort if we can avoid it, and here it's particularly
easy...

Paolo


> ---
>  hw/block/pflash_cfi01.c | 2 +-
>  hw/block/pflash_cfi02.c | 2 +-
>  include/exec/memory.h   | 4 +++-
>  memory.c                | 5 +++--
>  4 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 593fbc5..4d51bfd 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -773,7 +773,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
>      memory_region_init_rom_device(
>          &pfl->mem, OBJECT(dev),
>          pfl->be ? &pflash_cfi01_ops_be : &pflash_cfi01_ops_le, pfl,
> -        pfl->name, total_len);
> +        pfl->name, total_len, &error_abort);
>      vmstate_register_ram(&pfl->mem, DEVICE(pfl));
>      pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
>      sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
> index e196f4d..29bb0dc 100644
> --- a/hw/block/pflash_cfi02.c
> +++ b/hw/block/pflash_cfi02.c
> @@ -608,7 +608,7 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
>  
>      memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl), pfl->be ?
>                                    &pflash_cfi02_ops_be : &pflash_cfi02_ops_le,
> -                                  pfl, pfl->name, chip_len);
> +                                  pfl, pfl->name, chip_len, &error_abort);
>      vmstate_register_ram(&pfl->orig_mem, DEVICE(pfl));
>      pfl->storage = memory_region_get_ram_ptr(&pfl->orig_mem);
>      pfl->chip_len = chip_len;
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index dcb35a2..4921d27 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -388,13 +388,15 @@ void memory_region_init_alias(MemoryRegion *mr,
>   * @ops: callbacks for write access handling.
>   * @name: the name of the region.
>   * @size: size of the region.
> + * @errp: pointer to Error*, to store an error if it happens.
>   */
>  void memory_region_init_rom_device(MemoryRegion *mr,
>                                     struct Object *owner,
>                                     const MemoryRegionOps *ops,
>                                     void *opaque,
>                                     const char *name,
> -                                   uint64_t size);
> +                                   uint64_t size,
> +                                   Error **errp);
>  
>  /**
>   * memory_region_init_reservation: Initialize a memory region that reserves
> diff --git a/memory.c b/memory.c
> index ffe24ff..38c29df 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1202,7 +1202,8 @@ void memory_region_init_rom_device(MemoryRegion *mr,
>                                     const MemoryRegionOps *ops,
>                                     void *opaque,
>                                     const char *name,
> -                                   uint64_t size)
> +                                   uint64_t size,
> +                                   Error **errp)
>  {
>      memory_region_init(mr, owner, name, size);
>      mr->ops = ops;
> @@ -1210,7 +1211,7 @@ void memory_region_init_rom_device(MemoryRegion *mr,
>      mr->terminates = true;
>      mr->rom_device = true;
>      mr->destructor = memory_region_destructor_rom_device;
> -    mr->ram_addr = qemu_ram_alloc(size, mr, &error_abort);
> +    mr->ram_addr = qemu_ram_alloc(size, mr, errp);
>  }
>  
>  void memory_region_init_iommu(MemoryRegion *mr,
>

Comments

Hu Tao Sept. 10, 2014, 2:05 a.m. UTC | #1 
On Tue, Sep 09, 2014 at 01:20:57PM +0200, Paolo Bonzini wrote:
> Il 09/09/2014 07:27, Hu Tao ha scritto:
> > Add parameter errp to memory_region_init_rom_device and update all call
> > sites to pass in &error_abort.
> > 
> > Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> 
> Better not use error_abort if we can avoid it, and here it's particularly
> easy...

Is error_abort deprecated?

Regards,
Hu
Paolo Bonzini Sept. 10, 2014, 9:11 a.m. UTC | #2 
Il 10/09/2014 04:05, Hu Tao ha scritto:
>> > 
>> > Better not use error_abort if we can avoid it, and here it's particularly
>> > easy...
> Is error_abort deprecated?

No, not at at all.  It is useful whenever you know that an error cannot
happen.  However, if it makes sense and it is easy, error propagation is
better.  For every patch today that removes an exit(1), tomorrow we
could have a patch that removes an &error_abort.

Paolo
diff mbox

Patch

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index f15b4fe..688184e 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -750,6 +750,7 @@  static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
     int ret;
     uint64_t blocks_per_device, device_len;
     int num_devices;
+    Error *local_err = NULL;
 
     total_len = pfl->sector_len * pfl->nb_blocs;
 
@@ -770,7 +771,12 @@  static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
     memory_region_init_rom_device(
         &pfl->mem, OBJECT(dev),
         pfl->be ? &pflash_cfi01_ops_be : &pflash_cfi01_ops_le, pfl,
-        pfl->name, total_len, &error_abort);
+        pfl->name, total_len, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return NULL;
+    }
+
     vmstate_register_ram(&pfl->mem, DEVICE(pfl));
     pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 29bb0dc..39b44f3 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -597,6 +597,7 @@  static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
     pflash_t *pfl = CFI_PFLASH02(dev);
     uint32_t chip_len;
     int ret;
+    Error *local_err = NULL;
 
     chip_len = pfl->sector_len * pfl->nb_blocs;
     /* XXX: to be fixed */
@@ -608,7 +609,12 @@  static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
 
     memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl), pfl->be ?
                                   &pflash_cfi02_ops_be : &pflash_cfi02_ops_le,
-                                  pfl, pfl->name, chip_len, &error_abort);
+                                  pfl, pfl->name, chip_len, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return NULL;
+    }
+
     vmstate_register_ram(&pfl->orig_mem, DEVICE(pfl));
     pfl->storage = memory_region_get_ram_ptr(&pfl->orig_mem);
     pfl->chip_len = chip_len;