[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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).


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

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]