Re: [PATCH] vga: don't abort when adding a duplicate isa-vga device

[Markus Armbruster]

Thomas Huth <thuth@redhat.com> writes:

> On 14/08/2021 01.36, Jose R. Ziviani wrote:
>> If users try to add an isa-vga device that was already registered,
>> still in command line, qemu will crash:
>> $ qemu-system-mips64el -M pica61 -device isa-vga
>> RAMBlock "vga.vram" already registered, abort!
>> Aborted (core dumped)
>> That particular board registers such device automaticaly, so it's
>> not obvious that a VGA device already exists. This patch changes
>> this behavior by displaying a message and ignoring that device,
>> starting qemu normally.
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/44
>> Signed-off-by: Jose R. Ziviani <jziviani@suse.de>
>> ---
>>   hw/display/vga-isa.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>> diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
>> index 90851e730b..69db502dde 100644
>> --- a/hw/display/vga-isa.c
>> +++ b/hw/display/vga-isa.c
>> @@ -61,6 +61,15 @@ static void vga_isa_realizefn(DeviceState *dev, Error 
>> **errp)
>>       MemoryRegion *vga_io_memory;
>>       const MemoryRegionPortio *vga_ports, *vbe_ports;
>> +    /*
>> +     * some machines register VGA by default, so instead of aborting
>> +     * it, show a message and ignore this device.
>> +     */
>> +    if (qemu_ram_block_by_name("vga.vram")) {
>> +        error_report("vga.vram is already registered, ignoring this 
>> device");
>> +        return;
>> +    }
>
> I think we should not ignore the error, but rather turn this into a
> proper error (instead of aborting).
>
> So if you replace error_report(...) with error_setg(errp, ...), the
> patch should be fine.

Agreed.

error_report() in a function with an Error **errp parameter is almost
always wrong.