Change in vdsm[master]: Get list of nameservers for host

ibarkan at redhat.com ibarkan at redhat.com
Sun Apr 12 10:27:27 UTC 2015


Ido Barkan has posted comments on this change.

Change subject: Get list of nameservers for host
......................................................................


Patch Set 2: Code-Review-1

(6 comments)

https://gerrit.ovirt.org/#/c/39460/2//COMMIT_MSG
Commit Message:

Line 6: 
Line 7: Get list of nameservers for host
Line 8: 
Line 9: 1. Get a list of nameservers listed in /etc/resolv.conf
Line 10: 2. Perform unit testing for above functionality
it is hopefully obvious for everyone, so you can drop this line
Line 11: 
Line 12: Change-Id: I3ce8a0e2bf93b118620739b144e70c576f03cd08


https://gerrit.ovirt.org/#/c/39460/2/lib/vdsm/netinfo.py
File lib/vdsm/netinfo.py:

Line 90:     try:
Line 91:         with open(DNS_RESOLVE_CONF_FILE,'r') as file_object:
Line 92:             file_text = file_object.read()
Line 93:         nameservers_list = parse_dnss(file_text)
Line 94:         return nameservers_list
this can be in-lined since the expected result is already documented
Line 95:     except IOError :
Line 96:         logging.error('Unable to open file /etc/resolv.conf')
Line 97:         return 0
Line 98: 


Line 91:         with open(DNS_RESOLVE_CONF_FILE,'r') as file_object:
Line 92:             file_text = file_object.read()
Line 93:         nameservers_list = parse_dnss(file_text)
Line 94:         return nameservers_list
Line 95:     except IOError :
since this function returns a list, let's be consistent so we would not crash the caller. just return an empty list.

in fact, I wouldn't even try/except. /etc/resolve.conf is not expected to be missing or not readable in any supported Linux distribution. I would expet the system to explode in case an exception is raised here.
Line 96:         logging.error('Unable to open file /etc/resolv.conf')
Line 97:         return 0
Line 98: 
Line 99: 


Line 96:         logging.error('Unable to open file /etc/resolv.conf')
Line 97:         return 0
Line 98: 
Line 99: 
Line 100: def parse_dnss(file_text):
should be python 'private' (prefixed with '_')
Line 101:     dnss = []
Line 102:     for line in file_text.splitlines():
Line 103:         line.strip()
Line 104:         words = line.split()


Line 99: 
Line 100: def parse_dnss(file_text):
Line 101:     dnss = []
Line 102:     for line in file_text.splitlines():
Line 103:         line.strip()
this statement has no affect. strings are immutable in python hence all string methods return a copy of the original string.

this will do what you want:
words = line.strip().split()
Line 104:         words = line.split()
Line 105:         if words[0] == 'nameserver':
Line 106:             dnss.append(words[1])
Line 107:     return dnss


Line 946:             if iface == vdict['iface']:
Line 947:                 users.add(v)
Line 948:         return users
Line 949: 
Line 950: 
please remove this new blank line. it is unrelated to this commit.
Line 951: def ifaceUsed(iface):
Line 952:     """Lightweight implementation of bool(Netinfo.ifaceUsers()) that does not
Line 953:     require a NetInfo object."""
Line 954:     if os.path.exists(os.path.join(NET_PATH, iface, 'brport')):  # Is it a port


-- 
To view, visit https://gerrit.ovirt.org/39460
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3ce8a0e2bf93b118620739b144e70c576f03cd08
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Priya Avhad <avhad.priya24 at gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Priya Avhad <avhad.priya24 at gmail.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list