On Wed, Jul 27, 2011 at 11:20:42AM -0400, Laine Stump wrote: > 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. This would be a change in behaviour - currently 'autoport' attribute is always present when we output XML, even for inactive guests, so there is a consistent way to determine if autoport is in use. > 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). One further thought, is should we even store 'port' on the listen element. I know technically this lets you configure different port numbers for different interfaces, but this feels somewhat error prone for clients. eg consider a guest A <listen addr='10.0.0.1' port='5902'> <listen addr='192.168.0.1' port='5909'> and guest B <listen addr='10.0.0.1' port='5904'> <listen addr='192.168.0.1' port='5902'> And DNS enter 'somehost.example.com' which has two A records somehost.example.com A 10.0.0.1 somehost.example.com A 192.168.0.1 Connecting to 'somehost.example.com' guest A you need to be very careful not to accidentally get guest B. So it has become much more tedious for a client app to connect to a guest if they have to worry about the fact that a VM may be listening on different ports for each IP address associated with a DNS name. Using different ports may sound unlikely, but it actually could be a fairly common problem if you use 'autoport' feature. I feel it is worthwhile for app developer sanity to *not* allow different port numbers per <listen> element, only IP addresses Should we find we need the extra flexibility in the future (unlikely) then we can still add it to teh XML at that time. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list