[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] PATCH: Work in progress suport for video device config



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 veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]