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