From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40935) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TmsD8-0004M0-FY for qemu-devel@nongnu.org; Sun, 23 Dec 2012 15:31:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TmsD7-0007g5-4k for qemu-devel@nongnu.org; Sun, 23 Dec 2012 15:31:50 -0500 Received: from mx1.redhat.com ([209.132.183.28]:5134) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TmsD6-0007g0-TA for qemu-devel@nongnu.org; Sun, 23 Dec 2012 15:31:49 -0500 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qBNKVl7c014855 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sun, 23 Dec 2012 15:31:47 -0500 Date: Sun, 23 Dec 2012 22:31:38 +0200 From: Alon Levy Message-ID: <20121223203138.GB7485@garlic> References: <50C86E8B.5020705@redhat.com> <1355398603-22186-1-git-send-email-alevy@redhat.com> <1355398603-22186-2-git-send-email-alevy@redhat.com> <50C9C49C.10400@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <50C9C49C.10400@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/2] qxl: change rom so that 4096 < size < 8192 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org On Thu, Dec 13, 2012 at 01:05:48PM +0100, Gerd Hoffmann wrote: > On 12/13/12 12:36, Alon Levy wrote: > > This is a simpler solution to 869981, where migration breaks since qxl's > > rom bar size has changed. Instead of ignoring fields in QXLRom, which is what has > > actually changed, we remove some of the modes, a mechanism already > > accounted for by the guest. To reach exactly two pages and not one, we > > remove two out of four orientations, the remaining are normal and right > > turn (chosen arbitrarily). Leaving only normal would result in a single > > page which would also break migration. > > > > Added assertions to ensure changes in spice-protocol in the future > > causing increase or decrease of page size will result in failure at > > startup (could do this compile time also but not sure how). > > The assertions are not in the patch. > > > #define QXL_MODE_EX(x_res, y_res) \ > > QXL_MODE_16_32(x_res, y_res, 0), \ > > - QXL_MODE_16_32(y_res, x_res, 1), \ > > - QXL_MODE_16_32(x_res, y_res, 2), \ > > - QXL_MODE_16_32(y_res, x_res, 3) > > + QXL_MODE_16_32(x_res, y_res, 1) > > Why do you leave orientation = 1 in? Just to keep the size above 4K? > Shouldn't we just hardcode the rom size to 8k instead? Then assert that > everything fits into 8k? Or even better add a compile time check? > > While being at it it might be a good idea to move the mode table to a > fixed, large enougth offset (say 0x4096), so it doesn't move around > again in case we extend QXLRom one more time. This solution is breaking backward compatibility like Yonit pointed out. The fact that I can't produce a user that would break because of this doesn't prove there is no such user. I suggest we go back to the original patch I posted, breaking it into two like you requested. What do you say? > > cheers, > Gerd >