[Libvir] PATCH: Don't auto-set VNC port to 5900+domid

[Daniel P. Berrange]

The current code for stuffing the VNC port number into the XML looks in
XenStore for the VNC port number, and if it does not find it there falls
back to using 5900+domid. This is a problem because it leaves open a
race condition where the VNC daemon for a guest may not have started up
yet, and so if an app requests the XML at this time it'll get potentially
bogus port number info. In the best case there will be nothing listening
on the bogus port, in the worst case a completely different domain will
be listening on that port.

This scenario is described here

http://www.redhat.com/archives/et-mgmt-tools/2007-February/msg00115.html

The reason we have the 5900+domid rule in there is to support Xen 3.0.2
or earlier, where the port number was never stored in XenStore. For any
Xen 3.0.3 or later, we should basically never use the 5900+domid fallback
rule. So the attached patch makes it conditional on xendConfigVersion < 2

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  -=| 
-------------- next part --------------
Index: src/xend_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/xend_internal.c,v
retrieving revision 1.92
diff -u -p -r1.92 xend_internal.c
--- src/xend_internal.c	22 Feb 2007 19:09:29 -0000	1.92
+++ src/xend_internal.c	22 Feb 2007 19:51:10 -0000
@@ -1569,7 +1569,13 @@ xend_parse_sexp_desc(virConnectPtr conn,
             } else if (tmp && !strcmp(tmp, "vnc")) {
                 int port = xenStoreDomainGetVNCPort(conn, domid);
                 const char *listenAddr = sexpr_node(node, "device/vfb/vnclisten");
-                if (port == -1)
+                /* For Xen >= 3.0.3, don't generate a fixed port mapping
+                 * because it will almost certainly be wrong ! Just leave
+                 * it as -1 which lets caller see that the VNC server isn't
+                 * present yet. Subsquent dumps of the XML will eventually
+                 * find the port in XenStore once VNC server has started
+                 */
+                if (port == -1 && xendConfigVersion < 2)
                     port = 5900 + domid;
                 if (listenAddr) {
                     virBufferVSprintf(&buf, "    <graphics type='vnc' port='%d' listen='%s'/>\n", port, listenAddr);