Change in vdsm[master]: netconf: Add dhcp support for iproute2 configurator
asegurap at redhat.com
asegurap at redhat.com
Tue Jul 2 22:41:20 UTC 2013
Antoni Segura Puimedon has posted comments on this change.
Change subject: netconf: Add dhcp support for iproute2 configurator
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(4 inline comments)
Very cool addition. Thanks! Just a few minor comments.
....................................................
File vdsm/netconf/iproute2.py
Line 162: class DhcpClient():
Line 163: PID_FILE = '/var/run/dhclient-%s.pid'
Line 164: LEASE_DIR = '/var/lib/dhclient/'
Line 165: LEASE_FILE = LEASE_DIR + 'dhclient-%s.lease'
Line 166: DHCLIENT = str(utils.CommandPath('dhclient', '/sbin/dhclient'))
Instead of using str here, you should just use DHCLIENT.cmd on line 176.
Line 167:
Line 168: def __init__(self, iface):
Line 169: self.iface = iface
Line 170: self.pidFile = self.PID_FILE % self.iface
Line 172: utils.execCmd(['mkdir', self.LEASE_DIR])
Line 173: self.leaseFile = (self.LEASE_FILE % self.iface)
Line 174:
Line 175: def _dhclient(self):
Line 176: rc, out, err = utils.execCmd([self.DHCLIENT, '-1', '-pf',
Since I already request a change to this line in the comment above, I'd prefer to have a from utils import execCmd.
Line 177: self.pidFile, '-lf', self.leaseFile,
Line 178: self.iface])
Line 179: return rc, out, err
Line 180:
Line 189: return rc
Line 190:
Line 191: def shutdown(self):
Line 192: try:
Line 193: pid = int(file(self.pidFile).readline().strip())
'file' is deprecated, 'open' should be used instead.
Line 194: except IOError as e:
Line 195: if e.errno == os.errno.ENOENT:
Line 196: pass
Line 197: else:
Line 197: else:
Line 198: raise
Line 199: else:
Line 200: try:
Line 201: os.kill(pid, signal.SIGKILL)
I wonder if maybe we should use SIGTERM to let dhclient gracefully stop.
Line 202: except OSError as e:
Line 203: if e.errno == os.errno.ESRCH:
Line 204: pass
Line 205: else:
--
To view, visit http://gerrit.ovirt.org/15492
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea88e8693e47fa51edb89c33344332c88c5c964d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Giuseppe Vallarelli <gvallare at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
More information about the vdsm-patches
mailing list