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

[Daniel P. Berrange]

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 :|