On Tue, May 19, 2009 at 10:35:40AM +0100, Daniel P. Berrange wrote:
> We discussed adding a new XML element for representing video devices a
> few weeks back. This is my work in progress patch for the XML parsing
> routines supporting
>
> <video>
> <model type='vga|cirrus|vmvga|xen' vram='64' heads='1'/>
> </video>
>
>
> For compatability, if an existing guest has a <graphics> tag, but no
> <video> tag, then the parser automatically adds a <video> tag for
> type=cirrus. Still todo
>
> - Tweak the addition of default <video> tag a little so it uses the
> correct type for the type of guest - eg it should use type=xen in
> some cases.
> - Add RNG XML schemas & website docs
> - Make QEMU driver use this info for setting -vga argument
> - Make Xen drivers use this info for setting stdvga=1|0 config arg
> - Make VirtualBox use this info in whatever way it needs
[...]
> +static virDomainVideoDefPtr
> +virDomainVideoDefParseXML(virConnectPtr conn,
> + const xmlNodePtr node,
> + int flags ATTRIBUTE_UNUSED) {
[...]
> + if (vram &&
> + virStrToLong_ui(vram, NULL, 10, &def->vram) < 0) {
> + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
> + _("cannot parse video ram '%s'"), vram);
> + goto error;
> + } else {
> + switch (def->type) {
> + /* Wierd, QEMU defaults to 9 MB ??! */
> + case VIR_DOMAIN_VIDEO_TYPE_VGA:
> + case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
> + case VIR_DOMAIN_VIDEO_TYPE_VMVGA:
> + def->vram = 9 * 1024;
> + break;
> + case VIR_DOMAIN_VIDEO_TYPE_XEN:
> + /* Original PVFB hardcoded to 4 MB */
> + def->vram = 4 * 1024;
> + break;
> + }
> + }
Maybe a default initialization routine could be used based on the
def->type/conn->type
[...
> + /* For backwards compatability, if no <video> tag is set but there
> + * is a <graphics> tag, then we add a single video tag */
> + if (def->ngraphics && !def->nvideos) {
> + virDomainVideoDefPtr video;
> + if (VIR_ALLOC(video) < 0)
> + goto no_memory;
> + video->type = VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
> + video->vram = 9 * 1024;
> + video->heads = 1;
> + if (VIR_ALLOC_N(def->videos, 1) < 0) {
> + virDomainVideoDefFree(video);
> + goto no_memory;
> + }
> + def->videos[def->nvideos++] = video;
> + }
and reused to implement the case by case default init here too
Patch looks fine so far !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list