From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1L5PJV-0007Ii-3f for qemu-devel@nongnu.org; Wed, 26 Nov 2008 13:40:37 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1L5PJT-0007Dt-2d for qemu-devel@nongnu.org; Wed, 26 Nov 2008 13:40:36 -0500 Received: from [199.232.76.173] (port=38102 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1L5PJS-0007Da-VF for qemu-devel@nongnu.org; Wed, 26 Nov 2008 13:40:34 -0500 Received: from mx20.gnu.org ([199.232.41.8]:53116) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1L5PJS-0000WP-IO for qemu-devel@nongnu.org; Wed, 26 Nov 2008 13:40:34 -0500 Received: from mail.codesourcery.com ([65.74.133.4]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1L5PJN-0002V1-JM for qemu-devel@nongnu.org; Wed, 26 Nov 2008 13:40:30 -0500 From: Paul Brook Subject: Re: [Qemu-devel] [PATCH 0 of 7] [UPDATE] DisplayState interface change Date: Wed, 26 Nov 2008 18:40:22 +0000 References: <492D8B94.4000805@eu.citrix.com> In-Reply-To: <492D8B94.4000805@eu.citrix.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200811261840.22618.paul@codesourcery.com> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Stefano Stabellini > This patch changes the DisplayState interface adding support for > multiple frontends at the same time (sdl and vnc) and implements most > of the benefit of the shared_buf patch without the added complexity. I'm still not convinced you've got the abstraction right here. By my reading your shared buffer code is fairly fragile. If the users switches VCs (which will cause qemu to reallocate the surface independently of the device) then qemu will reallocate the buffer. Having both qemu and individual device code allocating surfaces seems wrong. It should be one or the other. It also feels messy the way that allocating the surface and notifying the DisplayState of the change are two separate operations. IMHO DisplayState should be an opaque structure as far as devices are concerned. They should never have to access its members (or deal with DisplaySurface) directly. > - do not use is_active_console, use is_graphic_console instead. This is wrong. There may be multiple graphic consoles. > This patch changes the graphical_console_init function to return an > allocated DisplayState instead of a QEMUConsole. You should also remove QEMUConsole. > + s->ds = graphic_console_init(s->update, s->invalidate, > + s->screen_dump, s->text_update, s); > + register_displaystate(s->ds); Why have you got separate allocate and register functions? Paul