[libvirt] [RFC][PATCH] change the meaning of vnc port in xml

Now the vnc port in xml file reflects the REAL port hypervisor will listen on for vnc connection. But vnc ports usually start from 5900, it's common to say 0 for 5900, 1 for 5901, and so on. This patch forbids negative vnc port number in xml, and maps port number below 5900 to 5900+port, but no change to those greater than 5900. --- I have no idea whether this behaviour is sane or not. But my vnc client(vinagre) has the same behaviour, i.e., treats port 0 as 5900, 1 as 5901, but 5900 as 5900, 5901 as 5901. src/conf/domain_conf.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0cea8eb..b3cbc34 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6423,6 +6423,12 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, if (flags & VIR_DOMAIN_XML_INACTIVE) def->data.vnc.port = 0; def->data.vnc.autoport = 1; + } else if (def->data.vnc.port < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid vnc port %s"), port); + goto error; + } else if (def->data.vnc.port < 5900) { + def->data.vnc.port += 5900; } } else { def->data.vnc.port = 0; -- 1.7.10.2

On 10/16/2012 11:54 AM, Hu Tao wrote:
Now the vnc port in xml file reflects the REAL port hypervisor will listen on for vnc connection. But vnc ports usually start from 5900, it's common to say 0 for 5900, 1 for 5901, and so on.
This patch forbids negative vnc port number in xml, and maps port number below 5900 to 5900+port, but no change to those greater than 5900. ---
I have no idea whether this behaviour is sane or not. But my vnc client(vinagre) has the same behaviour, i.e., treats port 0 as 5900, 1 as 5901, but 5900 as 5900, 5901 as 5901.
src/conf/domain_conf.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0cea8eb..b3cbc34 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6423,6 +6423,12 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, if (flags & VIR_DOMAIN_XML_INACTIVE) def->data.vnc.port = 0; def->data.vnc.autoport = 1; + } else if (def->data.vnc.port < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid vnc port %s"), port); + goto error; + } else if (def->data.vnc.port < 5900) { + def->data.vnc.port += 5900; } } else { def->data.vnc.port = 0;
It is most probably sane (I myself can think of any scenario this could break), but it has to be mentioned in the documentation as well. Also another questions arise when dealing with spice ports. Do we also implement it there or will we leave the inconsistency? And if yes, how do we treat port and tlsPort then? Nevertheless, I must say I'm fond of this idea. Martin

On Tue, Oct 16, 2012 at 05:54:50PM +0800, Hu Tao wrote:
Now the vnc port in xml file reflects the REAL port hypervisor will listen on for vnc connection. But vnc ports usually start from 5900, it's common to say 0 for 5900, 1 for 5901, and so on.
This patch forbids negative vnc port number in xml, and maps port number below 5900 to 5900+port, but no change to those greater than 5900. ---
I have no idea whether this behaviour is sane or not. But my vnc client(vinagre) has the same behaviour, i.e., treats port 0 as 5900, 1 as 5901, but 5900 as 5900, 5901 as 5901.
src/conf/domain_conf.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0cea8eb..b3cbc34 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6423,6 +6423,12 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, if (flags & VIR_DOMAIN_XML_INACTIVE) def->data.vnc.port = 0; def->data.vnc.autoport = 1; + } else if (def->data.vnc.port < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid vnc port %s"), port); + goto error; + } else if (def->data.vnc.port < 5900) { + def->data.vnc.port += 5900; } } else { def->data.vnc.port = 0;
NACK, I don't think we want such magic behaviour in the XML. The XML should concern itself *only* with real port numbers. It is upto the client applications to decide whether to translate these into VNC display numbers for the user. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange
-
Hu Tao
-
Martin Kletzander