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

[Laine Stump]

On 07/27/2011 02:41 AM, Laine Stump wrote:
> 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.
>

After more thinking, her eare two possible plans:


1) see above - the autoport attribute will only show up (or be 
recognized) for live XML; when writing the configuration, lack of a port 
will imply autoport=yes, and presence of port will imply autoport=no.

2) remove autoport from <listen> and store it in <graphics>. During the 
parsing of <graphics>, after all the listens are parsed, loop through 
them and verify that either all of them have a port, or none of them do 
(inactive XML only - for live XML they actually all should have a port).

Votes?

(After looking through the code again, I'm completely willing to 
implement (2), just want to make sure that's really what you want.)