From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH V3 1/1] libxl: set stub domain size based on VRAM size Date: Wed, 15 Jul 2015 10:45:03 +0100 Message-ID: <1436953503.11153.36.camel@citrix.com> References: <1436650242-1067-1-git-send-email-eshelton@pobox.com> <1436650242-1067-2-git-send-email-eshelton@pobox.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZFJFU-0000Pw-CY for xen-devel@lists.xenproject.org; Wed, 15 Jul 2015 09:45:08 +0000 In-Reply-To: <1436650242-1067-2-git-send-email-eshelton@pobox.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Eric Shelton Cc: xen-devel@lists.xenproject.org, wei.liu2@citrix.com, ian.jackson@eu.citrix.com, samuel.thibault@ens-lyon.org, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On Sat, 2015-07-11 at 17:30 -0400, Eric Shelton wrote: > Allocate additional memory to the stub domain for qemu-traditional if > more than 4 MB is assigned to the video adapter to avoid out of memory > condition for QEMU. In reply to v2 (after you'd sent v3, I think) Ian J said: > Well, it does also reduce the stubdom memory when the video memory is > <4Mby. That sounds plausible to me but at the very least the commit > message needs changing, and maybe we should wait a bit to see if one > of the qemu experts objects. libxl__domain_build_info_setdefault ensures, for qemu-xen-traditional, that video_memkb is either precisely 4M (TYPE_CIRRUS) or >= 8M (TYPE_STD) but allows a free choice for TYPE_NONE, defaulting to 0 (I'd almost expect it to insist on 0, but there you are). I suggest adding a second paragraph to the commit message: For LIBXL_VGA_INTERFACE_TYPE_STD + CIRRUS video_memkb is always at least 4MB. However if LIBXL_VGA_INTERFACE_TYPE_STD is selected the video_memkb defaults to 0, resulting in a stubdom which is 4MB smaller than before. It seems unlikely that VGA disable would require more memory then the other options, so this should be ok. I'm happy to add that while I commit and will Ack + do so tomorrow unless somebody objects during today. Any Ack's from the mini-os or QEMU maintainers would still be appreciated (and would cause me to potentially commit sooner). Ian. > > Signed-off-by: Eric Shelton > --- > tools/libxl/libxl_dm.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > index 317a8eb..9a5a937 100644 > --- a/tools/libxl/libxl_dm.c > +++ b/tools/libxl/libxl_dm.c > @@ -1087,6 +1087,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss) > > dm_config->b_info.max_vcpus = 1; > - dm_config->b_info.max_memkb = 32 * 1024; > + dm_config->b_info.max_memkb = 28 * 1024 + > + guest_config->b_info.video_memkb; > dm_config->b_info.target_memkb = dm_config->b_info.max_memkb; > > dm_config->b_info.u.pv.features = "";