Re: [PATCH] Make sure we look up the IP address for the correct device (#469439)

[Joel Granados <jgranado redhat com>]

Looks ok,  no obvious wholes :)
----- "David Cantrell" <dcantrell redhat com> wrote:

> The VNC launch code in vnc.py needed an update to work better with
> NetworkManager.  When collecting the hostname and IP address, it was
> assuming the first device in the netdevices list is our active NIC,
> which may or may not be true.
> 
> Added getActiveNetDevs() in network.py to ask NetworkManager for a
> list of all currently configured interfaces.  Return a list of
> device names.  A list seems a bit pointless, but I'd like to have
> this in place now for future improvements where we might need to
> handle more than one active NIC during installation.  After all,
> NM can do that.
> 
> The message reported by anaconda once VNC is ready will contain the
> FQDN:DISPLAY_NUMBER (IP ADDRESS), if it can.  If it can't find your
> IP address, it leaves that out.  If it can't find your hostname, it
> also leaves that out.
> ---
>  anaconda   |    1 +
>  network.py |   30 ++++++++++++++++++++++++++
>  vnc.py     |   69
> +++++++++++++++++++++++------------------------------------
>  3 files changed, 58 insertions(+), 42 deletions(-)
> 
> diff --git a/anaconda b/anaconda
> index b8ae359..2731c43 100755
> --- a/anaconda
> +++ b/anaconda
> @@ -621,6 +621,7 @@ if __name__ == "__main__":
>      graphical_failed = 0
>      instClass = None                # the install class to use
>      vncS = vnc.VncServer()          # The vnc Server object.
> +    vncS.anaconda = anaconda
>      xserver_pid = None
>  
>      (opts, args) = parseOptions()
> diff --git a/network.py b/network.py
> index e7392cc..c986cda 100644
> --- a/network.py
> +++ b/network.py
> @@ -202,6 +202,36 @@ def hasActiveNetDev():
>      except:
>          return False
>  
> +# Return a list of device names (e.g., eth0) for all active devices.
> +# Returning a list here even though we will almost always have one
> +# device.  NM uses lists throughout its D-Bus communication, so
> trying
> +# to follow suit here.  Also, if this uses a list now, we can think
> +# about multihomed hosts during installation later.
> +def getActiveNetDevs():
> +    active_devs = set()
> +
> +    bus = dbus.SystemBus()
> +    nm = bus.get_object(isys.NM_SERVICE, isys.NM_MANAGER_PATH)
> +    nm_props_iface = dbus.Interface(nm, isys.DBUS_PROPS_IFACE)
> +
> +    active_connections = nm_props_iface.Get(isys.NM_MANAGER_IFACE,
> "ActiveConnections")
> +
> +    for connection in active_connections:
> +        active_connection = bus.get_object(isys.NM_SERVICE,
> connection)
> +        active_connection_props_iface =
> dbus.Interface(active_connection, isys.DBUS_PROPS_IFACE)
> +        devices =
> active_connection_props_iface.Get(isys.NM_MANAGER_IFACE, 'Devices')
> +
> +        for device_path in devices:
> +            device = bus.get_object(isys.NM_SERVICE, device_path)
> +            device_props_iface = dbus.Interface(device,
> isys.DBUS_PROPS_IFACE)
> +
> +            interface_name =
> device_props_iface.Get(isys.NM_MANAGER_IFACE, 'Interface')
> +            active_devs.add(interface_name)
> +
> +    ret = list(active_devs)
> +    ret.sort()
> +    return ret
> +
>  class NetworkDevice(SimpleConfigFile):
>      def __str__(self):
>          s = ""
> diff --git a/vnc.py b/vnc.py
> index b799457..2e5b4fd 100644
> --- a/vnc.py
> +++ b/vnc.py
> @@ -56,6 +56,7 @@ class VncServer:
>          self.pw_file = pw_file
>          self.pw_init_file = pw_init_file
>          self.connxinfo = None
> +        self.anaconda = None
>          self.log = logging.getLogger("anaconda.stdout")
>  
>      def recoverVNCPassword(self):
> @@ -100,52 +101,36 @@ class VncServer:
>          # see if we can sniff out network info
>          netinfo = network.Network()
>  
> -        # Look for the first configured interface and use its IP
> address for
> -        # the computer to connect to.
>          devices = netinfo.netdevices
> -        list = devices.keys()
> -        list.sort()
> -        dev = devices[list[0]]
> +        active_devs = network.getActiveNetDevs()
>  
> -        try:
> -            self.ip = isys.getIPAddress(dev.get("DEVICE"))
> -            log.info("ip of %s is %s" % (dev.get("DEVICE"),
> self.ip))
> -
> -            if self.ip == "127.0.0.1" or self.ip == "::1":
> -                self.ip = None
> -        except Exception, e:
> -            log.warning("Got an exception trying to get the self.ip
> addr "
> -                        "of %s: %s" % (dev.get("DEVICE"), e))
> -
> -        # If we have a real hostname that resolves against configured
> DNS
> -        # servers, use that for the name to connect to.
> -        if netinfo.hostname != "localhost.localdomain":
> -            if netinfo.lookupHostname() is not None:
> -                self.name = netinfo.hostname
> -            elif self.ip is None:
> -                # If we get here and there's no valid IP address,
> just use the
> -                # hostname and hope for the best (better than
> displaying nothing)
> -                self.name = netinfo.hostname
> -
> -        if self.name is not None:
> -            self.connxinfo = "%s:%s" % (self.name, self.display)
> +        if active_devs != []:
> +            dev = devices[active_devs[0]]
>  
> -        if self.ip is not None:
>              try:
> -                tmp = socket.inet_pton(socket.AF_INET6, self.ip)
> -                family = socket.AF_INET6
> -            except socket.error:
> -                family = socket.AF_INET
> +                self.ip = isys.getIPAddress(dev.get("DEVICE"))
> +                log.info("ip of %s is %s" % (dev.get("DEVICE"),
> self.ip))
> +
> +                if self.ip == "127.0.0.1" or self.ip == "::1":
> +                    self.ip = None
> +            except Exception, e:
> +                log.warning("Got an exception trying to get the
> self.ip addr "
> +                            "of %s: %s" % (dev.get("DEVICE"), e))
> +        else:
> +            self.ip = None
>  
> -            if family == socket.AF_INET6:
> -                ipstr = "[%s]" % self.ip
> -            else:
> -                ipstr = self.ip
> +        self.name = network.getDefaultHostname(self.anaconda)
> +        ipstr = self.ip
>  
> -            if self.connxinfo is None:
> -                self.connxinfo = "%s:%s" % (ipstr, self.display)
> -            else:
> -                self.connxinfo += " (%s)" % ipstr
> +        if self.ip.find(':') != -1:
> +            ipstr = "[%s]" % (self.ip,)
> +
> +        if (self.name is not None) and (not
> self.name.startswith('localhost')) and (ipstr is not None):
> +            self.connxinfo = "%s:%s (%s)" %
> (socket.getfqdn(name=self.name), self.display, ipstr,)
> +        elif ipstr is not None:
> +            self.connxinfo = "%s:%s" % (ipstr, self.display,)
> +        else:
> +            self.connxinfo = None
>  
>          # figure out product info
>          if self.name is not None:
> -- 
> 1.6.0.2
> 
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list redhat com
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list

-- 
Joel Andres Granados
Red Hat / Brno Czech Republic