[libvirt] [PATCH] Fix vnc port determining for xend

Current libvirt checks xenstore for a xen guests fixed vnc port on xend > 3.0.3. At least on f8 though, hvm guests don't store the vnc port in xenstore, it is stored in the sexpr. Patch fixes the logic to look in the sexpr if the xenstore lookup appears to fail. This fixes setting static vnc ports for f8 xen hvm guests. Thanks, Cole diff --git a/src/xend_internal.c b/src/xend_internal.c index 2a687c3..0b62dd0 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -2121,6 +2121,10 @@ xenDaemonParseSxprGraphicsNew(virConnectPtr conn, goto no_memory; } else { int port = xenStoreDomainGetVNCPort(conn, def->id); + if (port == -1) { + // Didn't find port entry in xenstore + port = sexpr_int(node, "device/vfb/vncdisplay"); + } const char *listenAddr = sexpr_node(node, "device/vfb/vnclisten"); const char *vncPasswd = sexpr_node(node, "device/vfb/vncpasswd");; const char *keymap = sexpr_node(node, "device/vfb/keymap");

On Thu, Aug 28, 2008 at 04:48:06PM -0400, Cole Robinson wrote:
Current libvirt checks xenstore for a xen guests fixed vnc port on xend > 3.0.3. At least on f8 though, hvm guests don't store the vnc port in xenstore, it is stored in the sexpr.
Patch fixes the logic to look in the sexpr if the xenstore lookup appears to fail. This fixes setting static vnc ports for f8 xen hvm guests.
Hmm, I thought this was already covered by our test suite. If it isn't then we should add a test becuase the combinations of VNC / HVM / PV / Xen version are a mine-field. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Thu, Aug 28, 2008 at 04:48:06PM -0400, Cole Robinson wrote:
Current libvirt checks xenstore for a xen guests fixed vnc port on xend > 3.0.3. At least on f8 though, hvm guests don't store the vnc port in xenstore, it is stored in the sexpr.
Patch fixes the logic to look in the sexpr if the xenstore lookup appears to fail. This fixes setting static vnc ports for f8 xen hvm guests.
Sounds fine to me though I winder if it's not better to check on the given SExpr first and then do a Xenstore lookup only if needed. Seems to me a more generally coherent approach, any expected drawback to this ? 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/

On Thu, Sep 04, 2008 at 07:51:51AM +0200, Daniel Veillard wrote:
On Thu, Aug 28, 2008 at 04:48:06PM -0400, Cole Robinson wrote:
Current libvirt checks xenstore for a xen guests fixed vnc port on xend > 3.0.3. At least on f8 though, hvm guests don't store the vnc port in xenstore, it is stored in the sexpr.
Patch fixes the logic to look in the sexpr if the xenstore lookup appears to fail. This fixes setting static vnc ports for f8 xen hvm guests.
Sounds fine to me though I winder if it's not better to check on the given SExpr first and then do a Xenstore lookup only if needed. Seems to me a more generally coherent approach, any expected drawback to this ?
Historically XenD only ever put the VNC port in xenstore. Including it in the SEXPR is something new in 3.2.0 IIRC. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Daniel P. Berrange wrote:
On Thu, Sep 04, 2008 at 07:51:51AM +0200, Daniel Veillard wrote:
On Thu, Aug 28, 2008 at 04:48:06PM -0400, Cole Robinson wrote:
Current libvirt checks xenstore for a xen guests fixed vnc port on xend > 3.0.3. At least on f8 though, hvm guests don't store the vnc port in xenstore, it is stored in the sexpr.
Patch fixes the logic to look in the sexpr if the xenstore lookup appears to fail. This fixes setting static vnc ports for f8 xen hvm guests.
Sounds fine to me though I winder if it's not better to check on the given SExpr first and then do a Xenstore lookup only if needed. Seems to me a more generally coherent approach, any expected drawback to this ?
Historically XenD only ever put the VNC port in xenstore. Including it in the SEXPR is something new in 3.2.0 IIRC.
Daniel
Actually I'm pretty sure this has been breaking static vnc port since F8. So I'm not sure when this took effect but if you give it a shot on F8 you'll see xm list --long shows 'vncdisplay'. Thanks, Cole

Cole Robinson wrote:
Current libvirt checks xenstore for a xen guests fixed vnc port on xend > 3.0.3. At least on f8 though, hvm guests don't store the vnc port in xenstore, it is stored in the sexpr.
Patch fixes the logic to look in the sexpr if the xenstore lookup appears to fail. This fixes setting static vnc ports for f8 xen hvm guests.
Thanks, Cole
Should have pointed out earlier, but this patch is replaced by the one I posted later, "Add more xen vnc tests..." Thanks, Cole
participants (3)
-
Cole Robinson
-
Daniel P. Berrange
-
Daniel Veillard