Re: [libvirt] [PATCH 5/7] conf: spice: Do more automation if autoport is requested

On 04/23/13 18:21, Daniel P. Berrange wrote: >On Tue, Apr 23, 2013 at 03:46:12PM +0200, Peter Krempa wrote: >>With autoport enabled, both ports were alocated. With enabling >>defaultMode or setting separate channel modes one of the ports may not >>be needed. This will allow later on doing this kind of change. >>--- >> docs/formatdomain.html.in | 2 +- >> src/conf/domain_conf.c | 5 ----- >> 2 files changed, 1 insertion(+), 6 deletions(-) >> >>diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in >>index 888c005..bb75943 100644 >>--- a/docs/formatdomain.html.in >>+++ b/docs/formatdomain.html.in >>@@ -3470,7 +3470,7 @@ qemu-kvm -net nic,model=? /dev/null >> while <code>tlsPort</code> gives an alternative secure >> port number. The <code>autoport</code> attribute is the >> new preferred syntax for indicating autoallocation of >>- both port numbers. The <code>listen</code> attribute is >>+ needed port numbers. The <code>listen</code> attribute is >> an IP address for the server to listen >> on. The <code>passwd</code> attribute provides a SPICE >> password in clear text. The <code>keymap</code> >>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >>index dc0ecaa..86a444c 100644 >>--- a/src/conf/domain_conf.c >>+++ b/src/conf/domain_conf.c >>@@ -7595,11 +7595,6 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, >> VIR_FREE(defaultMode); >> } >> >>- if (def->data.spice.port == -1 && def->data.spice.tlsPort == -1) { >>- /* Legacy compat syntax, used -1 for auto-port */ >>- def->data.spice.autoport = true; >>- } > >I'm not clear why this is safe. The idea is that if the user sends XML > > <graphics port='-1' tlsPort='-1'/> > >then libvirt would turn it into > > <graphics port='-1' tlsPort='-1' autoport='yes'/> > >with this removed, won't we be instead outputting > > <graphics port='-1' tlsPort='-1' autoport='no'/> > >despite the fact that it is auto-allocating the ports? Later on this will slightly change semantics: <graphics port='-1' tlsPort='-1' autoport='no'/> Will allocate both ports every time, even if one isn't needed because of other configuration (eg defaultMode="insecure")

whereas <graphics autoport='yes'/> will automatically determine which ports are needed according to configuration and allocate just those. (only the insecure port in case of the example above)