From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KiK5q-0006fp-2q for qemu-devel@nongnu.org; Tue, 23 Sep 2008 22:27:06 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KiK5o-0006fT-ME for qemu-devel@nongnu.org; Tue, 23 Sep 2008 22:27:05 -0400 Received: from [199.232.76.173] (port=58272 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KiK5o-0006fQ-F4 for qemu-devel@nongnu.org; Tue, 23 Sep 2008 22:27:04 -0400 Received: from rv-out-0708.google.com ([209.85.198.241]:42956) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KiK5o-00072I-2S for qemu-devel@nongnu.org; Tue, 23 Sep 2008 22:27:04 -0400 Received: by rv-out-0708.google.com with SMTP id f25so2107420rvb.22 for ; Tue, 23 Sep 2008 19:27:02 -0700 (PDT) Message-ID: Date: Wed, 24 Sep 2008 04:27:02 +0200 From: "andrzej zaborowski" In-Reply-To: <20080923152058.GA27742@jannic.reliablesolutions.de> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <20080922171558.GA11353@x61s.gondor.com> <48D7DF10.4030004@codemonkey.ws> <20080923152058.GA27742@jannic.reliablesolutions.de> Subject: [Qemu-devel] Re: [PATCH] Wrapper around dpy_copy to fix segfault with -vnc option Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Niehusmann Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org 2008/9/23 Jan Niehusmann : > On Mon, Sep 22, 2008 at 11:15:04PM +0200, andrzej zaborowski wrote: >> Yes, I don't think hw/ code should be concerned with what console is >> active. Logically the dpy_ functions should take the pointer returned >> from graphic_console_init() as first parameter. >> >> Please also check the code is formatted consistently with qemu. >> >> I didn't receive Jan's message but the check seems to not be enough >> because there can be multiple graphical consoles with different sizes >> - if I'm guessing correctly what this patch tries to fix. > > Based on these comments I had another look at the code. If there can > be multiple graphical consoles, the only sensible test is 'console > == active_console' where console must be provided by the caller. So, > indeed, a pointer to the console must be provided instead of a pointer > to the DisplayState. > > To make function names consistent, I called the function qemu_console_copy > in analogy to qemu_console_resize (which is a similar wrapper around > dpy_resize). I committed the patch slightly modified because I found this still doesn't account for all the cases. Imagine ds->dpy_copy is not set, the call does nothing and the screen is not updated until fully invalidated. We need to either implement a generic dpy_copy in console.c or have a fallback in hw/cirrus_vga.c depending on which is faster. Also note that till now hw/ files could only call into the ds->dpy_ functions from inside their own vga_hw_update callback, this guaranteed some consistency. The use of dpy_copy inside cirrus_vga.c changed this which is the source of these bugs. I hadn't noticed when this happened. Regards