From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [libvirt] [PATCH v2] libxl: Implement basic video device selection Date: Fri, 4 Apr 2014 13:56:09 +0100 Message-ID: <1396616169.4211.222.camel__19501.9388841049$1396616281$gmane$org@kazak.uk.xensource.com> References: <533D8207.8040402@redhat.com> <1396604199-8198-1-git-send-email-stefan.bader@canonical.com> <1396604917.4211.184.camel@kazak.uk.xensource.com> <533E8A1A.4030904@canonical.com> <1396607657.4211.190.camel@kazak.uk.xensource.com> <20140404125113.GA13990@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20140404125113.GA13990@redhat.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: "Daniel P. Berrange" Cc: libvir-list@redhat.com, Michal Privoznik , Stefan Bader , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Fri, 2014-04-04 at 14:51 +0200, Daniel P. Berrange wrote: > On Fri, Apr 04, 2014 at 11:34:17AM +0100, Ian Campbell wrote: > > On Fri, 2014-04-04 at 12:31 +0200, Stefan Bader wrote: > > > On 04.04.2014 11:48, Ian Campbell wrote: > > > > On Fri, 2014-04-04 at 11:36 +0200, Stefan Bader wrote: > > > >> + /* > > > >> + * Take the first defined video device (graphics card) to display > > > >> + * on the first graphics device (display). > > > >> + * Right now only type and vram info is used and anything beside > > > >> + * type xen and vga is mapped to cirrus. > > > >> + */ > > > >> + if (def->nvideos) { > > > >> + unsigned int min_vram = 8 * 1024; > > > >> + > > > >> + switch (def->videos[0]->type) { > > > >> + case VIR_DOMAIN_VIDEO_TYPE_VGA: > > > >> + case VIR_DOMAIN_VIDEO_TYPE_XEN: > > > >> + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD; > > > >> + /* > > > >> + * Libxl enforces a minimal VRAM size of 8M when using > > > >> + * LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL or > > > >> + * 16M for LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN. > > > >> + * Avoid build failures and go with the minimum if less > > > >> + * is specified. > > > > > > > > Build failures? Do you mean "domain build" rather than "libvirt build"? > > > > > > > > I'm not sure about this fixing up the GIGO from the user side, but > > > > that's a libvirt policy decision I suppose? If it were me I would just > > > > let libxl provide the error and expect people to fix their domain config > > > > rather than silently giving them something other than what they asked > > > > for. What if increasing the VRAM causes a cascading failure due e.g. to > > > > lack of memory? That's going to be tricky to debug I think! > > > > > > In the end its a start a domain with such a config. Which seems to be what I > > > would end up with in my testing with an admittedly older version of virt-manager > > > connecting to a libvirtd running an initial version of this patch without that part. > > > The error seen on the front-end was something along the lines of "failed to get > > > enough memory to start the guest" (the libxl log on the other side had the > > > better error message). And the gui always reduces the memory below the minimum > > > for both the options (VGA and "Xen"). > > > That is the reason I went for "meh, go for the minimum anyways". > > > > Does the libvirt protocol require the client to provide all the sizes > > etc with no provision for asking the server side to pick a sane default? > > The XML does not have to include the VGA ram size. If it is omitted the > we fill in a default value after initial parsing is done. I guess the issue is that whatever client Stefan is using is including the VGA ram size, with a value which it turns out is not allowed. > That defualt can be hypervisor specific Great! Ian. > > > And btw, it is already confusing enough as with cirrus, I get a 9M by default > > > which is passed on to qemu on the command line and then ignored by it and one > > > gets 32M in any way... > > Yeah, we were silently ignoring ram size for cirrus since QEMU doesn't let > it be configured. Only QXL and I think stdvga is configurable. > > > Regards, > Daniel