Re: [PATCH] Forward port various iscsi fixes from 5.4 iscsi work

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



looks good,

but why do you use `if self._initiator == ""` ?
isn't `if not self._initiator` nicer? :)

--
Martin Gracik

----- Original Message -----
From: "Hans de Goede" <hdegoede@xxxxxxxxxx>
To: "Discussion of Development and Customization of the Red Hat Linux Installer" <anaconda-devel-list@xxxxxxxxxx>
Sent: Wednesday, May 6, 2009 10:25:30 AM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna
Subject: [PATCH] Forward port various iscsi fixes from 5.4 iscsi work

This patch contains the following small fixes:
- There is no need to convert None as username/pass into an empty string
  pylibiscsi will happily take either
- Only set the initiator name from ibft if the ibft flag is present
- Show an error when no username is specified, but a password /
  reverse username / pass is given
- Do not backtrace when the following happens:
  1) Manually add iscsi disk
  2) enter wrong IP / username without pass
  3) Fix this *and* change the initiator name initially choosen
- Allow having only a reverse password without a reverse username
---
 iw/autopart_type.py     |    9 ---------
 storage/iscsi.py        |   28 +++++++++++++++-------------
 textw/partition_text.py |    9 ---------
 3 files changed, 15 insertions(+), 31 deletions(-)

diff --git a/iw/autopart_type.py b/iw/autopart_type.py
index 1b35573..4d9953c 100644
--- a/iw/autopart_type.py
+++ b/iw/autopart_type.py
@@ -292,15 +292,6 @@ class PartitionTypeWindow(InstallWindow):
             user_in = dxml.get_widget("userinEntry").get_text().strip()
             pw_in = dxml.get_widget("passinEntry").get_text().strip()
 
-            if len(user) == 0:
-                user = None
-            if len(pw) == 0:
-                pw = None
-            if len(user_in) == 0:
-                user_in = None
-            if len(pw_in) == 0:
-                pw_in = None
-
             err = None
             try:
                 idx = target.rfind(":")
diff --git a/storage/iscsi.py b/storage/iscsi.py
index 92f190a..cfe0c55 100644
--- a/storage/iscsi.py
+++ b/storage/iscsi.py
@@ -97,12 +97,13 @@ class iscsi(object):
         self.initiatorSet = False
         self.started = False
 
-        try:
-            initiatorname = libiscsi.get_firmware_initiator_name()
-            self._initiator = initiatorname
-            self.initiatorSet = True
-        except:
-            pass
+        if flags.ibft:
+            try:
+                initiatorname = libiscsi.get_firmware_initiator_name()
+                self._initiator = initiatorname
+                self.initiatorSet = True
+            except:
+                pass
 
     def _getInitiator(self):
         if self._initiator != "":
@@ -111,12 +112,11 @@ class iscsi(object):
         return randomIname()
 
     def _setInitiator(self, val):
-        if self._initiator != "" and val != self._initiator:
+        if self.initiatorSet and val != self._initiator:
             raise ValueError, "Unable to change iSCSI initiator name once set"
         if len(val) == 0:
             raise ValueError, "Must provide a non-zero length string"
         self._initiator = val
-        self.initiatorSet = True
 
     initiator = property(_getInitiator, _setInitiator)
 
@@ -148,7 +148,7 @@ class iscsi(object):
         if not has_iscsi():
             return
 
-        if not self.initiatorSet:
+        if self._initiator == "":
             log.info("no initiator set")
             return
 
@@ -165,6 +165,7 @@ class iscsi(object):
         fd = os.open(INITIATOR_FILE, os.O_RDWR | os.O_CREAT)
         os.write(fd, "InitiatorName=%s\n" %(self.initiator))
         os.close(fd)
+        self.initiatorSet = True
 
         for dir in ['ifaces','isns','nodes','send_targets','slp','static']:
             fulldir = "/var/lib/iscsi/%s" % (dir,)
@@ -190,16 +191,16 @@ class iscsi(object):
 
         if not has_iscsi():
             raise IOError, _("iSCSI not available")
-        if not self.initiatorSet:
+        if self._initiator == "":
             raise ValueError, _("No initiator name set")
 
-        self.startup(intf)
-
-        if user:
+        if user or pw or user_in or pw_in:
             # Note may raise a ValueError
             authinfo = libiscsi.chapAuthInfo(username=user, password=pw,
                                              reverse_username=user_in,
                                              reverse_password=pw_in)
+        self.startup(intf)
+
         # Note may raise an IOError
         found_nodes = libiscsi.discover_sendtargets(address=ipaddr,
                                                     port=int(port),
@@ -250,6 +251,7 @@ class iscsi(object):
                 f.write(" --password %s" % auth.password)
                 if len(auth.reverse_username):
                     f.write(" --reverse-user %s" % auth.reverse_username)
+                if len(auth.reverse_password):
                     f.write(" --reverse-password %s" % auth.reverse_password)
             f.write("\n")
 
diff --git a/textw/partition_text.py b/textw/partition_text.py
index 1a790ca..48b7c18 100644
--- a/textw/partition_text.py
+++ b/textw/partition_text.py
@@ -223,15 +223,6 @@ class PartitionTypeWindow:
 
         (user, pw, user_in, pw_in) = entries[2:]
 
-        if len(user) == 0:
-            user = None
-        if len(pw) == 0:
-            pw = None
-        if len(user_in) == 0:
-            user_in = None
-        if len(pw_in) == 0:
-            pw_in = None
-
         target = entries[0].strip()
         try:
             idx = target.rfind(":")
-- 
1.6.2.2

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux