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

[Laine Stump]

On 07/26/2011 07:10 PM, Eric Blake wrote:
> On 07/26/2011 08:50 AM, Laine Stump wrote:
>>> 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.
>
> Is it that hard to add that additional validation?


Anything can be done, given enough time to think, code, and test all the 
corner cases. I don't see where the gain is in this case, though. I see 
autoport more as an attribute that's useful when examining live XML than 
as something useful to set in the config. Having a separate autoport for 
each port is then not overkill, but just natural. Trying to use a single 
autoport to indicate whether all of a set of ports were generated 
automatically or were manually configured is what's overkill - you're 
going to a lot of trouble to enforce an unnatural restriction for no 
practical gain.

Maybe the best thing is to only allow autoport in <listen> when (flags & 
INACTIVE) is false (live XML). This would mean that, as far as config 
was concerned, <listen> would *never* have any autoport in it (which 
would be fine with me); it would however still be in the data structure, 
and still output in live XML.


> Regardless of that answer, and whether this requires a v4, I'll go 
> ahead and review v3 code as if we decide that autoport-per-<listen> is 
> acceptable.
>