[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/4] Fix bad error handling after memory_region_
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 2/4] Fix bad error handling after memory_region_init_ram() |
Date: |
Mon, 14 Sep 2015 09:57:13 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Peter Crosthwaite <address@hidden> writes:
> On Fri, Sep 11, 2015 at 7:51 AM, Markus Armbruster <address@hidden> wrote:
>> Symptom:
>>
>> $ qemu-system-x86_64 -m 10000000
>> Unexpected error in ram_block_add() at /work/armbru/qemu/exec.c:1456:
>> upstream-qemu: cannot set up guest memory 'pc.ram': Cannot allocate
>> memory
>> Aborted (core dumped)
>>
>> Root cause: commit ef701d7 screwed up handling of out-of-memory
>> conditions. Before the commit, we report the error and exit(1), in
>> one place, ram_block_add(). The commit lifts the error handling up
>> the call chain some, to three places. Fine. Except it uses
>> &error_abort in these places, changing the behavior from exit(1) to
>> abort(), and thus undoing the work of commit 3922825 "exec: Don't
>> abort when we can't allocate guest memory".
>>
>> The three places are:
>>
>> * memory_region_init_ram()
>>
>> Commit 4994653 (right after commit ef701d7) lifted the error
>> handling further, through memory_region_init_ram(), multiplying the
>> incorrect use of &error_abort. Later on, imitation of existing
>> (bad) code may have created more.
>>
>> * memory_region_init_ram_ptr()
>>
>> The &error_abort is still there.
>>
>> * memory_region_init_rom_device()
>>
>> Doesn't need fixing, because commit 33e0eb5 (soon after commit
>> ef701d7) lifted the error handling further, and in the process
>> changed it from &error_abort to passing it up the call chain.
>> Correct, because the callers are realize() methods.
>>
>> Fix the error handling after memory_region_init_ram() with a
>> Coccinelle semantic patch:
>>
>> @r@
>> expression mr, owner, name, size, err;
>> position p;
>> @@
>> memory_region_init_ram(mr, owner, name, size,
>> (
>> - &error_abort
>> + &error_fatal
>> |
>> address@hidden
>> )
>> );
>> @script:python@
>> p << r.p;
>> @@
>> print "%s:%s:%s" % (p[0].file, p[0].line, p[0].column)
>>
>> When the last argument is &error_abort, it gets replaced by
>> &error_fatal. This is the fix.
>>
>> If the last argument is anything else, its position is reported. This
>> lets us check the fix is complete. Four positions get reported:
>>
>> * ram_backend_memory_alloc()
>>
>> Error is passed up the call chain, ultimately through
>> user_creatable_complete(). As far as I can tell, it's callers all
>> handle the error sanely.
>>
>> * fsl_imx25_realize(), fsl_imx31_realize(), dp8393x_realize()
>>
>
> This is super modern code that is the exception to the rule doing it right.
Progress :)
>> DeviceClass.realize() methods, errors handled sanely further up the
>> call chain.
>>
>> We're good. Test case again behaves:
>>
>> $ qemu-system-x86_64 -m 10000000
>> qemu-system-x86_64: cannot set up guest memory 'pc.ram': Cannot allocate
>> memory
>> [Exit 1 ]
>>
>> The next commits will repair the rest of commit ef701d7's damage.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
[...]
> So the changes here fall into a few different categories.
>
> * Machine init code - error_fatal() is definately right (for the
> moment, unless we want to support complete machine hotplug or
> something crazy like that).
> * SysBusDevice::init functions - These should be propagatable, but we
> are really getting what we deserve with error_fatal(). They should be
> desysbusified then can be converted to realize. Out of scope of this
> series though.
> * Device Realize functions (incl a few SoCs). In these cases we should
> propagate for the sake of hotplug failure (or other reasons). I have
> flagged the easy ones below.
> * Common helper functions that are missing Error ** even though their
> callers have them. We should added them (particular in VGA).
>
> I think we should try and get the realize and helper ones right and do
> the machine init and SBD::init ones later.
Makes sense. In fact, I got the obvious realize ones sketched in my
tree, but then forgot to mention it in my cover letter. Let me compare
your notes to my sketch.
[...]
>> --- a/hw/arm/stm32f205_soc.c
>> +++ b/hw/arm/stm32f205_soc.c
>> @@ -71,7 +71,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc,
>> Error **errp)
>> MemoryRegion *flash_alias = g_new(MemoryRegion, 1);
>>
>> memory_region_init_ram(flash, NULL, "STM32F205.flash", FLASH_SIZE,
>> - &error_abort);
>> + &error_fatal);
>
>
> This should propagate
Yes.
>> memory_region_init_alias(flash_alias, NULL, "STM32F205.flash.alias",
>> flash, 0, FLASH_SIZE);
>>
>> @@ -84,7 +84,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc,
>> Error **errp)
>> memory_region_add_subregion(system_memory, 0, flash_alias);
>>
>> memory_region_init_ram(sram, NULL, "STM32F205.sram", SRAM_SIZE,
>> - &error_abort);
>> + &error_fatal);
>> vmstate_register_ram_global(sram);
>> memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
>>
This, too.
[...]
>> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
>> index a4e7b5c..37dc0b0 100644
>> --- a/hw/arm/xilinx_zynq.c
>> +++ b/hw/arm/xilinx_zynq.c
>> @@ -167,7 +167,7 @@ static void zynq_init(MachineState *machine)
>>
>> /* 256K of on-chip memory */
>> memory_region_init_ram(ocm_ram, NULL, "zynq.ocm_ram", 256 << 10,
>> - &error_abort);
>> + &error_fatal);
>> vmstate_register_ram_global(ocm_ram);
>> memory_region_add_subregion(address_space_mem, 0xFFFC0000, ocm_ram);
>>
>> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
>> index 2955f3b..43b3e5a 100644
>> --- a/hw/arm/xlnx-zynqmp.c
>> +++ b/hw/arm/xlnx-zynqmp.c
>> @@ -113,7 +113,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error
>> **errp)
>> char *ocm_name = g_strdup_printf("zynqmp.ocm_ram_bank_%d", i);
>>
>> memory_region_init_ram(&s->ocm_ram[i], NULL, ocm_name,
>> - XLNX_ZYNQMP_OCM_RAM_SIZE, &error_abort);
>> + XLNX_ZYNQMP_OCM_RAM_SIZE, &error_fatal);
>
> This should propagate.
Yes.
>> vmstate_register_ram_global(&s->ocm_ram[i]);
>> memory_region_add_subregion(get_system_memory(),
>> XLNX_ZYNQMP_OCM_RAM_0_ADDRESS +
>> diff --git a/hw/block/onenand.c b/hw/block/onenand.c
>> index 1b2c893..58eff50 100644
>> --- a/hw/block/onenand.c
>> +++ b/hw/block/onenand.c
>> @@ -786,7 +786,7 @@ static int onenand_initfn(SysBusDevice *sbd)
>> s->otp = memset(g_malloc((64 + 2) << PAGE_SHIFT),
>> 0xff, (64 + 2) << PAGE_SHIFT);
>> memory_region_init_ram(&s->ram, OBJECT(s), "onenand.ram",
>> - 0xc000 << s->shift, &error_abort);
>> + 0xc000 << s->shift, &error_fatal);
>> vmstate_register_ram_global(&s->ram);
>> ram = memory_region_get_ram_ptr(&s->ram);
>> s->boot[0] = ram + (0x0000 << s->shift);
>> diff --git a/hw/cris/axis_dev88.c b/hw/cris/axis_dev88.c
>> index 3cae480..b57051e 100644
>> --- a/hw/cris/axis_dev88.c
>> +++ b/hw/cris/axis_dev88.c
>> @@ -277,7 +277,7 @@ void axisdev88_init(MachineState *machine)
>> /* The ETRAX-FS has 128Kb on chip ram, the docs refer to it as the
>> internal memory. */
>> memory_region_init_ram(phys_intmem, NULL, "axisdev88.chipram",
>> INTMEM_SIZE,
>> - &error_abort);
>> + &error_fatal);
>> vmstate_register_ram_global(phys_intmem);
>> memory_region_add_subregion(address_space_mem, 0x38000000, phys_intmem);
>>
>> diff --git a/hw/display/cg3.c b/hw/display/cg3.c
>> index 34dcbc3..d2a0d97 100644
>> --- a/hw/display/cg3.c
>> +++ b/hw/display/cg3.c
>> @@ -281,7 +281,7 @@ static void cg3_initfn(Object *obj)
>> CG3State *s = CG3(obj);
>>
>> memory_region_init_ram(&s->rom, NULL, "cg3.prom", FCODE_MAX_ROM_SIZE,
>> - &error_abort);
>> + &error_fatal);
>> memory_region_set_readonly(&s->rom, true);
>> sysbus_init_mmio(sbd, &s->rom);
>>
Do this in cg3_realizefn() instead, so we can propagate?
>> @@ -310,7 +310,7 @@ static void cg3_realizefn(DeviceState *dev, Error **errp)
>> }
>>
>> memory_region_init_ram(&s->vram_mem, NULL, "cg3.vram", s->vram_size,
>> - &error_abort);
>> + &error_fatal);
>> memory_region_set_log(&s->vram_mem, true, DIRTY_MEMORY_VGA);
>> vmstate_register_ram_global(&s->vram_mem);
>> sysbus_init_mmio(sbd, &s->vram_mem);
This should propagate.
>> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
>> index 2288238..9c961da 100644
>> --- a/hw/display/qxl.c
>> +++ b/hw/display/qxl.c
>> @@ -1970,14 +1970,14 @@ static void qxl_realize_common(PCIQXLDevice *qxl,
>> Error **errp)
>>
>> qxl->rom_size = qxl_rom_size();
>> memory_region_init_ram(&qxl->rom_bar, OBJECT(qxl), "qxl.vrom",
>> - qxl->rom_size, &error_abort);
>> + qxl->rom_size, &error_fatal);
>
> Propagate.
Yes.
>> vmstate_register_ram(&qxl->rom_bar, &qxl->pci.qdev);
>> init_qxl_rom(qxl);
>> init_qxl_ram(qxl);
>>
>> qxl->guest_surfaces.cmds = g_new0(QXLPHYSICAL, qxl->ssd.num_surfaces);
>> memory_region_init_ram(&qxl->vram_bar, OBJECT(qxl), "qxl.vram",
>> - qxl->vram_size, &error_abort);
>> + qxl->vram_size, &error_fatal);
>> vmstate_register_ram(&qxl->vram_bar, &qxl->pci.qdev);
>> memory_region_init_alias(&qxl->vram32_bar, OBJECT(qxl), "qxl.vram32",
>> &qxl->vram_bar, 0, qxl->vram32_size);
This, too.
>> @@ -2079,7 +2079,7 @@ static void qxl_realize_secondary(PCIDevice *dev,
>> Error **errp)
>> qxl->id = device_id++;
>> qxl_init_ramsize(qxl);
>> memory_region_init_ram(&qxl->vga.vram, OBJECT(dev), "qxl.vgavram",
>> - qxl->vga.vram_size, &error_abort);
>> + qxl->vga.vram_size, &error_fatal);
>
> Propagate.
Yes.
>> vmstate_register_ram(&qxl->vga.vram, &qxl->pci.qdev);
>> qxl->vga.vram_ptr = memory_region_get_ram_ptr(&qxl->vga.vram);
>> qxl->vga.con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl);
[...]
>> --- a/hw/display/tcx.c
>> +++ b/hw/display/tcx.c
>> @@ -945,7 +945,7 @@ static void tcx_initfn(Object *obj)
>> TCXState *s = TCX(obj);
>>
>> memory_region_init_ram(&s->rom, NULL, "tcx.prom", FCODE_MAX_ROM_SIZE,
>> - &error_abort);
>> + &error_fatal);
>
> I guess this one is particularly difficult, and indicates the RAM init
> needs to move to realize.
Similar to cg3_initfn().
>> memory_region_set_readonly(&s->rom, true);
>> sysbus_init_mmio(sbd, &s->rom);
>>
>> @@ -1007,7 +1007,7 @@ static void tcx_realizefn(DeviceState *dev, Error
>> **errp)
>> char *fcode_filename;
>>
>> memory_region_init_ram(&s->vram_mem, OBJECT(s), "tcx.vram",
>> - s->vram_size * (1 + 4 + 4), &error_abort);
>> + s->vram_size * (1 + 4 + 4), &error_fatal);
>
> Propagate.
Yes.
>> vmstate_register_ram_global(&s->vram_mem);
>> memory_region_set_log(&s->vram_mem, true, DIRTY_MEMORY_VGA);
>> vram_base = memory_region_get_ram_ptr(&s->vram_mem);
>> diff --git a/hw/display/vga.c b/hw/display/vga.c
>> index b35d523..9f68394 100644
>> --- a/hw/display/vga.c
>> +++ b/hw/display/vga.c
>> @@ -2139,7 +2139,7 @@ void vga_common_init(VGACommonState *s, Object *obj,
>> bool global_vmstate)
>>
>
> Can this function accept error ** ? Most of the callers are realize fns.
Then it should take Error **errp.
>> s->is_vbe_vmstate = 1;
>> memory_region_init_ram(&s->vram, obj, "vga.vram", s->vram_size,
>> - &error_abort);
>> + &error_fatal);
>
> Then this becomes a propagation.
Yes.
>> vmstate_register_ram(&s->vram, global_vmstate ? NULL : DEVICE(obj));
>> xen_register_framebuffer(&s->vram);
>> s->vram_ptr = memory_region_get_ram_ptr(&s->vram);
>> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
>> index 7f397d3..8e93509 100644
>> --- a/hw/display/vmware_vga.c
>> +++ b/hw/display/vmware_vga.c
>> @@ -1244,7 +1244,7 @@ static void vmsvga_init(DeviceState *dev, struct
>> vmsvga_state_s *s,
>>
>> s->fifo_size = SVGA_FIFO_SIZE;
>> memory_region_init_ram(&s->fifo_ram, NULL, "vmsvga.fifo", s->fifo_size,
>> - &error_abort);
>> + &error_fatal);
>
> Can add errp to this function from caller and propagate.
Yes.
[...]
>> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
>> index c63f45d..c93426b 100644
>> --- a/hw/pci-host/prep.c
>> +++ b/hw/pci-host/prep.c
>> @@ -302,7 +302,7 @@ static void raven_realize(PCIDevice *d, Error **errp)
>> d->config[0x34] = 0x00; // capabilities_pointer
>>
>> memory_region_init_ram(&s->bios, OBJECT(s), "bios", BIOS_SIZE,
>> - &error_abort);
>> + &error_fatal);
>> memory_region_set_readonly(&s->bios, true);
>> memory_region_add_subregion(get_system_memory(), (uint32_t)(-BIOS_SIZE),
>> &s->bios);
Propagate.
>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>> index ccea628..b0bf540 100644
>> --- a/hw/pci/pci.c
>> +++ b/hw/pci/pci.c
>> @@ -2081,7 +2081,7 @@ static void pci_add_option_rom(PCIDevice *pdev, bool
>> is_default_rom,
>> snprintf(name, sizeof(name), "%s.rom",
>> object_get_typename(OBJECT(pdev)));
>> }
>> pdev->has_rom = true;
>> - memory_region_init_ram(&pdev->rom, OBJECT(pdev), name, size,
>> &error_abort);
>> + memory_region_init_ram(&pdev->rom, OBJECT(pdev), name, size,
>> &error_fatal);
>
> Propagate.
Yes.
> Regards,
> Peter
>
>> vmstate_register_ram(&pdev->rom, &pdev->qdev);
>> ptr = memory_region_get_ram_ptr(&pdev->rom);
>> load_image(path, ptr);
You got a few more than me, I got a few more than you; lovely :)
Three kinds of changes to be done, I think:
A. Fix use of &error_fatal in functions that already have an Error **
parameter
Propagate the error. Trivial. Could probably go through my tree as
a single patch. Should start with my sketch (appended) to save some
typing.
B. Add Error ** parameter to functions called from functions that have an
Error ** parameter
Should be straightforward. May want to route it through the
relevant maintainer, though.
C. Move calls that can fail from .instance_init() to .realize()
Route through the relevant maintainer.
If you'd like to pitch in, let me know, so we don't duplicate work.
Here's my (incomplete!) sketch:
diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index 4d26a7e..e3bec1d 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -71,7 +71,11 @@ static void stm32f205_soc_realize(DeviceState *dev_soc,
Error **errp)
MemoryRegion *flash_alias = g_new(MemoryRegion, 1);
memory_region_init_ram(flash, NULL, "STM32F205.flash", FLASH_SIZE,
- &error_fatal);
+ &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
memory_region_init_alias(flash_alias, NULL, "STM32F205.flash.alias",
flash, 0, FLASH_SIZE);
@@ -83,8 +87,11 @@ static void stm32f205_soc_realize(DeviceState *dev_soc,
Error **errp)
memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, flash);
memory_region_add_subregion(system_memory, 0, flash_alias);
- memory_region_init_ram(sram, NULL, "STM32F205.sram", SRAM_SIZE,
- &error_fatal);
+ memory_region_init_ram(sram, NULL, "STM32F205.sram", SRAM_SIZE, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
vmstate_register_ram_global(sram);
memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 43b3e5a..2d57f27 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -113,7 +113,11 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error
**errp)
char *ocm_name = g_strdup_printf("zynqmp.ocm_ram_bank_%d", i);
memory_region_init_ram(&s->ocm_ram[i], NULL, ocm_name,
- XLNX_ZYNQMP_OCM_RAM_SIZE, &error_fatal);
+ XLNX_ZYNQMP_OCM_RAM_SIZE, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
vmstate_register_ram_global(&s->ocm_ram[i]);
memory_region_add_subregion(get_system_memory(),
XLNX_ZYNQMP_OCM_RAM_0_ADDRESS +
diff --git a/hw/display/cg3.c b/hw/display/cg3.c
index d2a0d97..adf847f 100644
--- a/hw/display/cg3.c
+++ b/hw/display/cg3.c
@@ -294,6 +294,7 @@ static void cg3_realizefn(DeviceState *dev, Error **errp)
{
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
CG3State *s = CG3(dev);
+ Error *err = NULL;
int ret;
char *fcode_filename;
@@ -310,7 +311,11 @@ static void cg3_realizefn(DeviceState *dev, Error **errp)
}
memory_region_init_ram(&s->vram_mem, NULL, "cg3.vram", s->vram_size,
- &error_fatal);
+ &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
memory_region_set_log(&s->vram_mem, true, DIRTY_MEMORY_VGA);
vmstate_register_ram_global(&s->vram_mem);
sysbus_init_mmio(sbd, &s->vram_mem);
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 9c961da..d8a381f 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1931,6 +1931,7 @@ static void qxl_init_ramsize(PCIQXLDevice *qxl)
static void qxl_realize_common(PCIQXLDevice *qxl, Error **errp)
{
uint8_t* config = qxl->pci.config;
+ Error *err = NULL;
uint32_t pci_device_rev;
uint32_t io_size;
@@ -1970,14 +1971,22 @@ static void qxl_realize_common(PCIQXLDevice *qxl, Error
**errp)
qxl->rom_size = qxl_rom_size();
memory_region_init_ram(&qxl->rom_bar, OBJECT(qxl), "qxl.vrom",
- qxl->rom_size, &error_fatal);
+ qxl->rom_size, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
vmstate_register_ram(&qxl->rom_bar, &qxl->pci.qdev);
init_qxl_rom(qxl);
init_qxl_ram(qxl);
qxl->guest_surfaces.cmds = g_new0(QXLPHYSICAL, qxl->ssd.num_surfaces);
memory_region_init_ram(&qxl->vram_bar, OBJECT(qxl), "qxl.vram",
- qxl->vram_size, &error_fatal);
+ qxl->vram_size, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
vmstate_register_ram(&qxl->vram_bar, &qxl->pci.qdev);
memory_region_init_alias(&qxl->vram32_bar, OBJECT(qxl), "qxl.vram32",
&qxl->vram_bar, 0, qxl->vram32_size);
@@ -2075,11 +2084,16 @@ static void qxl_realize_secondary(PCIDevice *dev, Error
**errp)
{
static int device_id = 1;
PCIQXLDevice *qxl = PCI_QXL(dev);
+ Error *err = NULL;
qxl->id = device_id++;
qxl_init_ramsize(qxl);
memory_region_init_ram(&qxl->vga.vram, OBJECT(dev), "qxl.vgavram",
- qxl->vga.vram_size, &error_fatal);
+ qxl->vga.vram_size, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
vmstate_register_ram(&qxl->vga.vram, &qxl->pci.qdev);
qxl->vga.vram_ptr = memory_region_get_ram_ptr(&qxl->vga.vram);
qxl->vga.con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl);
diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 4635800..623683f 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -1002,12 +1002,17 @@ static void tcx_realizefn(DeviceState *dev, Error
**errp)
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
TCXState *s = TCX(dev);
ram_addr_t vram_offset = 0;
+ Error *err = NULL;
int size, ret;
uint8_t *vram_base;
char *fcode_filename;
memory_region_init_ram(&s->vram_mem, OBJECT(s), "tcx.vram",
- s->vram_size * (1 + 4 + 4), &error_fatal);
+ s->vram_size * (1 + 4 + 4), &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
vmstate_register_ram_global(&s->vram_mem);
memory_region_set_log(&s->vram_mem, true, DIRTY_MEMORY_VGA);
vram_base = memory_region_get_ram_ptr(&s->vram_mem);
diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index c93426b..9af1323 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -294,6 +294,7 @@ static void raven_pcihost_initfn(Object *obj)
static void raven_realize(PCIDevice *d, Error **errp)
{
RavenPCIState *s = RAVEN_PCI_DEVICE(d);
+ Error *err = NULL;
char *filename;
int bios_size = -1;
@@ -301,8 +302,11 @@ static void raven_realize(PCIDevice *d, Error **errp)
d->config[0x0D] = 0x10; // latency_timer
d->config[0x34] = 0x00; // capabilities_pointer
- memory_region_init_ram(&s->bios, OBJECT(s), "bios", BIOS_SIZE,
- &error_fatal);
+ memory_region_init_ram(&s->bios, OBJECT(s), "bios", BIOS_SIZE, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
memory_region_set_readonly(&s->bios, true);
memory_region_add_subregion(get_system_memory(), (uint32_t)(-BIOS_SIZE),
&s->bios);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index b0bf540..88acbeb 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2020,6 +2020,7 @@ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr,
int size)
static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom,
Error **errp)
{
+ Error *err = NULL;
int size;
char *path;
void *ptr;
@@ -2081,7 +2082,11 @@ static void pci_add_option_rom(PCIDevice *pdev, bool
is_default_rom,
snprintf(name, sizeof(name), "%s.rom",
object_get_typename(OBJECT(pdev)));
}
pdev->has_rom = true;
- memory_region_init_ram(&pdev->rom, OBJECT(pdev), name, size, &error_fatal);
+ memory_region_init_ram(&pdev->rom, OBJECT(pdev), name, size, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
vmstate_register_ram(&pdev->rom, &pdev->qdev);
ptr = memory_region_get_ram_ptr(&pdev->rom);
load_image(path, ptr);
--
2.4.3
- [Qemu-devel] [PATCH 0/4] Don't abort when we can't allocate guest memory (again), Markus Armbruster, 2015/09/11
- [Qemu-devel] [PATCH 3/4] loader: Fix memory_region_init_resizeable_ram() error handling, Markus Armbruster, 2015/09/11
- [Qemu-devel] [PATCH 4/4] memory: Fix bad error handling in memory_region_init_ram_ptr(), Markus Armbruster, 2015/09/11
- [Qemu-devel] [PATCH 1/4] error: New error_fatal, Markus Armbruster, 2015/09/11
- [Qemu-devel] [PATCH 2/4] Fix bad error handling after memory_region_init_ram(), Markus Armbruster, 2015/09/11
- Re: [Qemu-devel] [PATCH 0/4] Don't abort when we can't allocate guest memory (again), Markus Armbruster, 2015/09/11
- Re: [Qemu-devel] [PATCH 0/4] Don't abort when we can't allocate guest memory (again), Peter Crosthwaite, 2015/09/14