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

[Martin Kletzander]

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