Hi,
On 4/2/20 9:58 AM, André Przywara wrote:
> On 26/03/2020 15:24, Alexandru Elisei wrote:
>> A vesa device is used by the SDL, GTK or VNC framebuffers. Don't allow the
>> user to specify more than one of these options because kvmtool will create
>> identical devices and bad things will happen:
>>
>> $ ./lkvm run -c2 -m2048 -k bzImage --sdl --gtk
>> # lkvm run -k bzImage -m 2048 -c 2 --name guest-10159
>> Error: device region [d0000000-d012bfff] would overlap device region [d0000000-d012bfff]
>> *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 ***
>> *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 ***
>> *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 ***
>> ======= Backtrace: =========
>> ======= Backtrace: =========
>> /lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7fae0ed447e5]
>> ======= Backtrace: =========
>> /lib/x86_64-linux-gnu/libc.so.6/lib/x86_64-linux-gnu/libc.so.6(+0x8037a)[0x7fae0ed4d37a]
>> (+0x777e5)[0x7fae0ed447e5]
>> /lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7fae0ed447e5]
>> /lib/x86_64-linux-gnu/libc.so.6(+0x8037a)[0x7fae0ed4d37a]
>> /lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7fae0ed5153c]
>> *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 ***
>> /lib/x86_64-linux-gnu/libglib-2.0.so.0(g_string_free+0x3b)[0x7fae0f814dab]
>> /lib/x86_64-linux-gnu/libglib-2.0.so.0(g_string_free+0x3b)[0x7fae0f814dab]
>> /usr/lib/x86_64-linux-gnu/libgtk-3.so.0(+0x21121c)[0x7fae1023321c]
>> /usr/lib/x86_64-linux-gnu/libgtk-3.so.0(+0x21121c)[0x7fae1023321c]
>> ======= Backtrace: =========
>> Aborted (core dumped)
>>
>> The vesa device is explicitly created during the initialization phase of
>> the above framebuffers. Remove the superfluous check for their existence.
>>
> Not really happy about this pointer comparison, but I don't see a better
> way, and it's surely good enough for that purpose.
I've been giving this patch some thought. I think the fix is wrong. The problem is
that the user is allowed to specify more than one of --gtk, --sdl and --vnc. I'll
rewrite the patch to fix that, and print an error message that is actually useful
for the user.
Thanks,
Alex
>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
> Reviewed-by: Andre Przywara <andre.przywara@xxxxxxx>
>
> Cheers,
> Andre
>
>> ---
>> hw/vesa.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/vesa.c b/hw/vesa.c
>> index dd59a112330b..8071ad153f27 100644
>> --- a/hw/vesa.c
>> +++ b/hw/vesa.c
>> @@ -61,8 +61,11 @@ struct framebuffer *vesa__init(struct kvm *kvm)
>> BUILD_BUG_ON(!is_power_of_two(VESA_MEM_SIZE));
>> BUILD_BUG_ON(VESA_MEM_SIZE < VESA_BPP/8 * VESA_WIDTH * VESA_HEIGHT);
>>
>> - if (!kvm->cfg.vnc && !kvm->cfg.sdl && !kvm->cfg.gtk)
>> - return NULL;
>> + if (device__find_dev(vesa_device.bus_type, vesa_device.dev_num) == &vesa_device) {
>> + r = -EEXIST;
>> + goto out_error;
>> + }
>> +
>> vesa_base_addr = pci_get_io_port_block(PCI_IO_SIZE);
>> r = ioport__register(kvm, vesa_base_addr, &vesa_io_ops, PCI_IO_SIZE, NULL);
>> if (r < 0)
>>
