[libvirt] [PATCHv3 1/2] conf: add <listen> subelement to domain <graphics> element

[Laine Stump]

On 07/26/2011 07:35 AM, Daniel P. Berrange wrote:
> On Mon, Jul 25, 2011 at 05:00:51AM -0400, Laine Stump wrote:
>> Once it's plugged in, the<listen element will be an optional
>> replacement for the "listen", "port", "tlsPort", and "autoport"
>> attributes that graphics elements already have. If the<listen>
>> type='address', it will have an attribute called 'address' which will
>> contain an IP address or dns name that the guest's display server
>> should listen on. If, however, type='network', the<listen>  element
>> should have an attribute called 'network' that will be set to the name
>> of a network configuration to get the IP address from.
>>
>> * docs/schemas/domain.rng: updated to allow the<listen>  element
>>
>> * docs/formatdomain.html.in: document the<listen>  element and its
>>    attributes.
>>
>> * src/conf/domain_conf.[hc]:
>>
>>    1) The domain parser, formatter, and data structure are modified to
>>       support 0 or more<listen>  subelements to each<graphics>
>>       element. The old style "legacy" attributes are also still
>>       accepted, and will be stored internally just as if they were a
>>       separate<listen>  element. On output (i.e. format), the first
>>       <listen>  element will be duplicated in the legacy attributes of
>>       the<graphic>  element.
>>
>>    2) The following attributes have been removed from the unions in
>>       virDomainGRaphicsDef for graphics types vnc, rdp, and spice:
>>
>>          listen
>>          port
>>          tlsPort
>>          autoport
>>
>>       These attributes are all present in the<listen>  subelement (aka
>>       virDomainGraphicsListenDef)
> I think it is a somewhat overkill to have 'autoport' be a setting
> per-<listen>  element. I can't imagine people want a fixed port
> number on one IP addr, but a dynamic port number on another IP
> addr. So we could just keep that on the top level element.

Although I agree with you for config purposes, it looks to me like the 
real use of autoport is that in the live XML it allows differentiating 
between ports that were manually specified and those that were 
automatically allocated (really, that seems like its main purpose, since 
simply not giving a port also implies autoport). If we have only a 
single autoport attribute for all the listens, we'll have to put in 
extra code that makes sure if they specify port for one listen, they do 
it for all of them. Otherwise there will be no way to know which ports 
to strip out when writing the inactive XML (since the same attribute is 
used for config and status)...

So really, nobody should ever need to explicitly say "autoport='yes'" 
anywhere in their config anyway, it just happens when they don't give 
any "port='...'".