[libvirt] [PATCH] Fixes for VNC port handling

[john.levon at sun.com]

# HG changeset patch
# User john.levon at sun.com
# Date 1233118371 28800
# Node ID f91041f0c607be72c4b5695f081d20716521f6c5
# Parent  8fce2ce6108eb1ff1865ecc21d57114db036f2a2
Fixes for VNC port handling

When parsing sexpr, the VNC port should not be ignored, even when vncunused is
set. Fix the parsing of vncdisplay, which was broken due to strtol() (never
use this function!).

Signed-off-by: John Levon <john.levon at sun.com>

diff --git a/src/xend_internal.c b/src/xend_internal.c
--- a/src/xend_internal.c
+++ b/src/xend_internal.c
@@ -612,7 +612,9 @@ sexpr_get(virConnectPtr xend, const char
  *
  * convenience function to lookup an int value in the S-Expression
  *
- * Returns the value found or 0 if not found (but may not be an error)
+ * Returns the value found or 0 if not found (but may not be an error).
+ * This function suffers from the flaw that zero is both a correct
+ * return value and an error indicator: careful!
  */
 static int
 sexpr_int(const struct sexpr *sexpr, const char *name)
@@ -2091,15 +2093,16 @@ xenDaemonParseSxprGraphicsNew(virConnect
                 port = xenStoreDomainGetVNCPort(conn, def->id);
                 xenUnifiedUnlock(priv);
 
+                // Didn't find port entry in xenstore
                 if (port == -1) {
-                    // Didn't find port entry in xenstore
-                    port = sexpr_int(node, "device/vfb/vncdisplay");
+                    const char *value = sexpr_node(node,
+                        "device/vfb/vncdisplay");
+                    if (value != NULL)
+                        port = strtol(value, NULL, 0);
                 }
 
-                if ((unused && STREQ(unused, "1")) || port == -1) {
+                if ((unused && STREQ(unused, "1")) || port == -1)
                     graphics->data.vnc.autoport = 1;
-                    port = -1;
-                }
 
                 if (port >= 0 && port < 5900)
                     port += 5900;
diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-autoport.sexpr b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.sexpr
new file mode 100644
--- /dev/null
+++ b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.sexpr
@@ -0,0 +1,85 @@
+(domain
+    (domid 21)
+    (on_crash destroy)
+    (uuid e0c172e6-4ad8-7353-0ece-515d2f181365)
+    (bootloader_args )
+    (vcpus 1)
+    (name domu-224)
+    (on_poweroff destroy)
+    (on_reboot destroy)
+    (bootloader )
+    (maxmem 512)
+    (memory 512)
+    (shadow_memory 5)
+    (cpu_weight 256)
+    (cpu_cap 0)
+    (features )
+    (on_xend_start ignore)
+    (on_xend_stop shutdown)
+    (start_time 1233108538.42)
+    (cpu_time 907.159661051)
+    (online_vcpus 1)
+    (image
+        (hvm
+            (kernel /usr/lib/xen/boot/hvmloader)
+            (boot d)
+            (device_model /usr/lib/xen/bin/qemu-dm)
+            (keymap en-us)
+            (localtime 1)
+            (pae 1)
+            (serial pty)
+            (usb 1)
+            (usbdevice tablet)
+            (notes (SUSPEND_CANCEL 1))
+        )
+    )
+    (status 2)
+    (state r-----)
+    (store_mfn 131070)
+    (device
+        (vif
+            (bridge e1000g0)
+            (mac 00:16:3e:1b:e8:18)
+            (script vif-vnic)
+            (uuid 7da8c614-018b-dc87-6bfc-a296a95bca4f)
+            (backend 0)
+        )
+    )
+    (device
+        (vbd
+            (uname phy:/iscsi/winxp)
+            (uuid 65e19258-f4a2-a9ff-3b31-469ceaf4ec8d)
+            (mode w)
+            (dev hda:disk)
+            (backend 0)
+            (bootable 1)
+        )
+    )
+    (device
+        (vbd
+            (uname file:/net/heaped/export/netimage/windows/xp-sp2-vol.iso)
+            (uuid 87d9383b-f0ad-11a4-d668-b965f55edc3f)
+            (mode r)
+            (dev hdc:cdrom)
+            (backend 0)
+            (bootable 0)
+        )
+    )
+    (device (vkbd (backend 0)))
+    (device
+        (vfb
+            (vncunused 1)
+            (keymap en-us)
+            (type vnc)
+            (uuid 09666ad1-0c94-d79c-1439-99e05394ee51)
+            (location localhost:5900)
+        )
+    )
+    (device
+        (console
+            (protocol vt100)
+            (location 3)
+            (uuid cabfc0f5-1c9c-0e6f-aaa8-9974262aff66)
+        )
+    )
+)
diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml
new file mode 100644
--- /dev/null
+++ b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml
@@ -0,0 +1,48 @@
+<domain type='xen' id='21'>
+  <name>domu-224</name>
+  <uuid>e0c172e6-4ad8-7353-0ece-515d2f181365</uuid>
+  <memory>524288</memory>
+  <currentMemory>524288</currentMemory>
+  <vcpu>1</vcpu>
+  <os>
+    <type>hvm</type>
+    <loader>/usr/lib/xen/boot/hvmloader</loader>
+    <boot dev='cdrom'/>
+  </os>
+  <features>
+    <pae/>
+  </features>
+  <clock offset='localtime'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>destroy</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
+    <disk type='block' device='disk'>
+      <driver name='phy'/>
+      <source dev='/iscsi/winxp'/>
+      <target dev='hda' bus='ide'/>
+    </disk>
+    <disk type='file' device='cdrom'>
+      <driver name='file'/>
+      <source file='/net/heaped/export/netimage/windows/xp-sp2-vol.iso'/>
+      <target dev='hdc' bus='ide'/>
+      <readonly/>
+    </disk>
+    <interface type='bridge'>
+      <mac address='00:16:3e:1b:e8:18'/>
+      <source bridge='e1000g0'/>
+      <script path='vif-vnic'/>
+      <target dev='vif21.0'/>
+    </interface>
+    <serial type='pty'>
+      <target port='0'/>
+    </serial>
+    <console type='pty'>
+      <target port='0'/>
+    </console>
+    <input type='tablet' bus='usb'/>
+    <input type='mouse' bus='ps2'/>
+    <graphics type='vnc' port='5900' autoport='yes' keymap='en-us'/>
+  </devices>
+</domain>