On Tue, Aug 17, 2021 at 10:07:55AM +0200, Philippe Mathieu-Daudé wrote: > 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? >
Hello people! Thanks for reviewing it. Sure, I'll send a v4. It's not for 6.1 anyway. Thank you very much
signature.asc
Description: Digital signature