[Libvir] [PATCH] Support parsing of new SEXPR for PVFB

[Daniel P. Berrange]

On Tue, Dec 05, 2006 at 05:16:54PM -0500, Daniel Veillard wrote:
> On Tue, Dec 05, 2006 at 07:57:33PM +0000, Daniel P. Berrange wrote:
> > The paravirt framebuffer patches for Xen finally got merged into upstream
> > xen-devel. In doing so, however, the syntax of the SEXPR for configuring
> > them changed completely. The existing framebuffer suppoirt in libvirt thus
> > only works with the 3.0.3 tree + PVFB patches. For 3.0.4 we need to work
> > with the new scheme. Fortunately, in 3.0.4 the xend_config_version flag
> > is bumped from 2 -> 3, so we have an easy value to hook off. 
> > 
> > The attached patch thus adds support for parsing an SEXPR looking like
> > 
> >     (device
> >         (vfb
> >             (vncunused 1)
> >             (vnclisten 127.0.0.1)
> >             (vncpasswd 123456)
> >             (type vnc)
> >         )
> >     )
> > 
> > And generating the <graphics type='vnc' port='5903'/>  XML tag to match.
> 
>   
> > Index: src/xend_internal.c
> > ===================================================================
> > RCS file: /data/cvs/libvirt/src/xend_internal.c,v
> > retrieving revision 1.78
> > diff -u -r1.78 xend_internal.c
> > --- src/xend_internal.c	22 Nov 2006 17:48:29 -0000	1.78
> > +++ src/xend_internal.c	5 Dec 2006 20:47:19 -0000
> > @@ -1736,6 +1736,19 @@
> >                                    tmp2);
> >  
> >              virBufferAdd(&buf, "    </interface>\n", 17);
> > +        } else if (!hvm &&
> > +                   sexpr_lookup(node, "device/vfb")) {
> 
>    I assume we don't need to check here for (xendConfigVersion >= 3) because
> that construct positively never happened before. 

Yeah, and in fact in RHEL / Fedora we explicitly don't want the version
check, because we're planning to backport the new style PVFB config to
the Xen 3.0.3 build we have.

> > +            /* New style graphics config for PV guests only in 3.0.4 */
> > +            tmp = sexpr_node(node, "device/vfb/type");
> > +
> > +            if (tmp && !strcmp(tmp, "sdl")) {
> > +                virBufferAdd(&buf, "    <graphics type='sdl'/>\n", 27);
> > +            } else if (tmp && !strcmp(tmp, "vnc")) {
> > +                int port = xenStoreDomainGetVNCPort(conn, domid);
> > +                if (port == -1)
> > +                    port = 5900 + domid;
> > +                virBufferVSprintf(&buf, "    <graphics type='vnc' port='%d'/>\n", port);
> > +            }
> 
>   Obviously the SEXPR contains more data, but the only one we used to export
> is the port number (which we have to extract in different way), do we want to
> make those extra informations about the IP listening and password part of
> the dumped XML ? I guess the answer is no because the XML is provided to
> non-root users via the proxy and this is a security information leak, right ?

Definitely don't want to include the password. We should add the vnclisten
data there though at some point in future - if only because it'll make it
easier for apps to sanity check. Adding the vnclisten data isn't a data leak
because you can already trivially see if with 'netstat'.

> > -    tmp = sexpr_fmt_node(root, "domain/image/%s/sdl", hvm ? "hvm" : "linux");
> > -    if (tmp != NULL) {
> > -        if (tmp[0] == '1')
> > -            virBufferAdd(&buf, "    <graphics type='sdl'/>\n", 27 );
> > +    /* Graphics device (HVM, or old (pre-3.0.4) style PV sdl config) */
> > +    if (hvm || xendConfigVersion < 3){
> > +        tmp = sexpr_fmt_node(root, "domain/image/%s/sdl", hvm ? "hvm" : "linux");
> > +        if (tmp != NULL) {
> > +            if (tmp[0] == '1')
> > +                virBufferAdd(&buf, "    <graphics type='sdl'/>\n", 27 );
> > +        }
> >      }
> 
>   Could we potentially have both sdl and vnc ? I'm wondering why this need to
> be limited it xendConfigVersion < 3 ...

I guess I could simply leave out that chunk /change - since that old style
config for PVFB simply won't ever appear in XenD SEXPR from 3.0.4 onwards. 

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|