On Fri, 2006-02-03 at 14:04 -0800, Patrick Mansfield wrote:
> Add the iscsi.py module that has common code used by the iscsi gui and text
> install.
This will also probably want to change a bit with the refactoring that's
in progress.
Some comments on things that could be nicer/cleaner in-line.
Generally, we try to avoid os.popen and do iutil.execWithCapture
instead, mostly for consistency purposes (and some more historical bugs)
and iutil.execWithRedirect instead of system().
> diff -uprN -X /home/patman/dontdiff anaconda-10.91.12/iscsi.py iscsi-anaconda-10.91.12/iscsi.py
> --- anaconda-10.91.12/iscsi.py 1969-12-31 16:00:00.000000000 -0800
> +++ iscsi-anaconda-10.91.12/iscsi.py 2006-02-02 12:53:06.000000000 -0800
[snip]
> + def login_out(self, action):
[snip]
> + command = ("%s -m node" % (ISCSIADM, ))
> + log.info("Running %s" % (command, ))
> + records = os.popen(command, 'r').readlines()
Should use execWithCapture instead
> + regex = re.compile('\[ ( [^\]]* ) ].*', re.VERBOSE)
> + for i in records:
> + recnum = regex.sub(r'\1', i)
> + recnum = re.sub('\n', '', recnum)
This should be able to be more python-y. Something like the following
probably works (not having looked exactly).
for line in records:
recnum = line.split()[0][1:-1]
> + command = ("%s -m node -r %s %s" % (ISCSIADM, recnum, action))
> + log.info("Running %s" % (command, ))
> + os.system(command)
Should use execWithRedirect so that we can have a log of the output
> + def shutdown(self):
[snip]
> + # Note that iscsid has/had code to ignore 2 (SIGINT), hence the
> + # 9 (SIGKILL).
If we run iscsid with -p and save the pid in /tmp, then we can have a
much more robust kill.
> + def startup(self):
> + log.debug("Setting up %s" % (INITIATOR_FILE, ))
> + if os.path.exists(INITIATOR_FILE):
> + os.unlink(INITIATOR_FILE)
> + fd = os.open(INITIATOR_FILE, os.O_RDWR | os.O_CREAT)
> + os.write(fd, "InitiatorName=")
> + os.write(fd, self.initiator)
> + os.write(fd, "\n")
> + os.close(fd)
Should just do this as a single
os.write(fd, "InitiatorName=%s\n" %(self.initiator,))
instead of splitting it up into multiple calls.
> + log.info("Starting %s" % (ISCSID_BIN, ))
> + os.system(ISCSID_BIN)
execWithRedirect instead
> + command = ("%s -m discovery -t st -p %s:%s"
> + % (ISCSIADM, self.ipaddr, self.port))
> + log.info("Running %s" % (command, ))
> + os.system(command)
The same
> + self.login_out("--login")
Maybe it makes more sense to call this method "action"?
Jeremy
