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

[Daniel P. Berrange]

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 :|