On 8/17/21 9:36 AM, Mark Cave-Ayland wrote: > On 17/08/2021 08:25, Thomas Huth wrote: > >> On 16/08/2021 15.55, 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 the device automaticaly, so it's >>> not obvious that a VGA device already exists. This patch changes >>> this behavior by displaying a message and exiting without crashing. >>> >>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/44 >>> Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com> >>> Signed-off-by: Jose R. Ziviani <jzivi...@suse.de> >>> --- >>> v2 to v3: Improved error message >>> v1 to v2: Use error_setg instead of error_report >>> >>> hw/display/vga-isa.c | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c >>> index 90851e730b..30d55b41c3 100644 >>> --- a/hw/display/vga-isa.c >>> +++ b/hw/display/vga-isa.c >>> @@ -33,6 +33,7 @@ >>> #include "hw/loader.h" >>> #include "hw/qdev-properties.h" >>> #include "qom/object.h" >>> +#include "qapi/error.h" >>> #define TYPE_ISA_VGA "isa-vga" >>> OBJECT_DECLARE_SIMPLE_TYPE(ISAVGAState, ISA_VGA) >>> @@ -61,6 +62,15 @@ static void vga_isa_realizefn(DeviceState *dev, >>> Error **errp) >>> MemoryRegion *vga_io_memory; >>> const MemoryRegionPortio *vga_ports, *vbe_ports; >>> + /* >>> + * make sure this device is not being added twice, if so >>> + * exit without crashing qemu >>> + */ >>> + if (qemu_ram_block_by_name("vga.vram")) { >>> + error_setg(errp, "'isa-vga' device already registered"); >>> + return; >>> + } >>> + >>> s->global_vmstate = true; >>> vga_common_init(s, OBJECT(dev)); >>> s->legacy_address_space = isa_address_space(isadev); >>> >> >> Reviewed-by: Thomas Huth <th...@redhat.com> > > Instead of checking for the presence of the vga.vram block, would it be > better to use the standard object_resolve_path_type() method to check > for the presence of the existing isa-vga device instead? See > https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg00717.html for > how this was done for virgl.
I remembered there was a nicer way but couldn't find it. If this patch were for 6.1, it was good enough. Now it will be merged in 6.2, I prefer Mark's suggestion. Jose, do you mind a v4?