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

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?

Daniel