Hi,
after elaborate experimenting, reading and failing I have a patchset and this is the Anaconda part. Combined with the new libiscsi one can log into the target specified in the firmware of his bnx2i card and utilize its partial offload capabilities for transfers.
I'd like to have it in soon, especially the first patch that pulls drivers and the extra daemon into the image so I can verify it works soon.
The important part is the libiscsi change but I don't think that anyone is interested in that (or ping me).
Ales
Includes brcm_iscsiuio daemon for bnx2i cards as well as loading the drivers and spawning the daemons in stage2 storage code.
Related: rhbz#442980 --- iutil.py | 10 ++++++++++ scripts/mk-images | 2 +- scripts/upd-instroot | 1 + storage/iscsi.py | 11 +++++++++++ 4 files changed, 23 insertions(+), 1 deletions(-)
diff --git a/iutil.py b/iutil.py index 67d4bfa..3c2a0d5 100644 --- a/iutil.py +++ b/iutil.py @@ -1050,3 +1050,13 @@ def get_sysfs_attr(path, attr): return None
return open(attribute, "r").read().strip() + +def find_program_in_path(prog, raise_on_error=False): + for d in os.environ["PATH"].split(":"): + full = "%s/%s" % (d, prog) + if os.access(full, os.X_OK): + return full + + if raise_on_error: + raise RuntimeError("Unable to locate a needed executable: '%s'" % prog) + return None diff --git a/scripts/mk-images b/scripts/mk-images index d5bacc9..9e0e025 100755 --- a/scripts/mk-images +++ b/scripts/mk-images @@ -101,7 +101,7 @@ USBMODS="$UMSMODS ohci-hcd uhci-hcd ehci-hcd usbhid mousedev usb-storage sd_mod FIREWIREMODS="ohci1394 sbp2 fw-ohci fw-sbp2 firewire-sbp2 firewire-ohci" SDMODS="mmc-block sdhci sdhci-pci" IDEMODS="ide-cd ide-cd_mod" -SCSIMODS="sr_mod sg st sd_mod scsi_mod iscsi_tcp iscsi_ibft scsi_wait_scan" +SCSIMODS="sr_mod sg st sd_mod scsi_mod iscsi_tcp iscsi_ibft scsi_wait_scan cxgb3i bnx2i be2iscsi" FSMODS="fat msdos vfat ext2 ext3 ext4 reiserfs jfs xfs gfs2 cifs fuse btrfs hfsplus" LVMMODS="dm-mod dm-zero dm-snapshot dm-mirror dm-multipath dm-round-robin dm-crypt" RAIDMODS="raid0 raid1 raid5 raid6 raid456 raid10 linear" diff --git a/scripts/upd-instroot b/scripts/upd-instroot index a4ec858..43fde85 100755 --- a/scripts/upd-instroot +++ b/scripts/upd-instroot @@ -415,6 +415,7 @@ lib/udev sbin/*gfs* sbin/arping sbin/badblocks +sbin/brcm_iscsiuio sbin/blockdev sbin/btrfsctl sbin/btrfsck diff --git a/storage/iscsi.py b/storage/iscsi.py index ff57cc5..8611d51 100644 --- a/storage/iscsi.py +++ b/storage/iscsi.py @@ -43,6 +43,8 @@ except ImportError: ISCSID="" INITIATOR_FILE="/etc/iscsi/initiatorname.iscsi"
+ISCSI_MODULES=['cxgb3i', 'bnx2i', 'be2iscsi'] + def find_iscsi_files(): global ISCSID if ISCSID == "": @@ -193,6 +195,15 @@ class iscsi(object): os.makedirs(fulldir, 0755)
log.info("iSCSI startup") + iutil.execWithRedirect('modprobe', ['-a'] + ISCSI_MODULES, + stdout="/dev/tty5", stderr="/dev/tty5") + # this is needed by Broadcom offload cards (bnx2i) + brcm_iscsiuio = iutil.find_program_in_path('brcm_iscsiuio', + raise_on_error=True) + log.debug("iscsi: brcm_iscsiuio is at %s" % brcm_iscsiuio) + iutil.execWithRedirect(brcm_iscsiuio, [], + stdout="/dev/tty5", stderr="/dev/tty5") + # run the daemon iutil.execWithRedirect(ISCSID, [], stdout="/dev/tty5", stderr="/dev/tty5") time.sleep(1)
Looks good...
Just a small note, you don't need to return None at the end of a function.
On Mon, 2010-11-29 at 15:37 +0100, Ales Kozumplik wrote:
Includes brcm_iscsiuio daemon for bnx2i cards as well as loading the drivers and spawning the daemons in stage2 storage code.
Related: rhbz#442980
iutil.py | 10 ++++++++++ scripts/mk-images | 2 +- scripts/upd-instroot | 1 + storage/iscsi.py | 11 +++++++++++ 4 files changed, 23 insertions(+), 1 deletions(-)
diff --git a/iutil.py b/iutil.py index 67d4bfa..3c2a0d5 100644 --- a/iutil.py +++ b/iutil.py @@ -1050,3 +1050,13 @@ def get_sysfs_attr(path, attr): return None
return open(attribute, "r").read().strip()
+def find_program_in_path(prog, raise_on_error=False):
- for d in os.environ["PATH"].split(":"):
full = "%s/%s" % (d, prog)if os.access(full, os.X_OK):return full- if raise_on_error:
raise RuntimeError("Unable to locate a needed executable: '%s'" % prog)- return None
diff --git a/scripts/mk-images b/scripts/mk-images index d5bacc9..9e0e025 100755 --- a/scripts/mk-images +++ b/scripts/mk-images @@ -101,7 +101,7 @@ USBMODS="$UMSMODS ohci-hcd uhci-hcd ehci-hcd usbhid mousedev usb-storage sd_mod FIREWIREMODS="ohci1394 sbp2 fw-ohci fw-sbp2 firewire-sbp2 firewire-ohci" SDMODS="mmc-block sdhci sdhci-pci" IDEMODS="ide-cd ide-cd_mod" -SCSIMODS="sr_mod sg st sd_mod scsi_mod iscsi_tcp iscsi_ibft scsi_wait_scan" +SCSIMODS="sr_mod sg st sd_mod scsi_mod iscsi_tcp iscsi_ibft scsi_wait_scan cxgb3i bnx2i be2iscsi" FSMODS="fat msdos vfat ext2 ext3 ext4 reiserfs jfs xfs gfs2 cifs fuse btrfs hfsplus" LVMMODS="dm-mod dm-zero dm-snapshot dm-mirror dm-multipath dm-round-robin dm-crypt" RAIDMODS="raid0 raid1 raid5 raid6 raid456 raid10 linear" diff --git a/scripts/upd-instroot b/scripts/upd-instroot index a4ec858..43fde85 100755 --- a/scripts/upd-instroot +++ b/scripts/upd-instroot @@ -415,6 +415,7 @@ lib/udev sbin/*gfs* sbin/arping sbin/badblocks +sbin/brcm_iscsiuio sbin/blockdev sbin/btrfsctl sbin/btrfsck diff --git a/storage/iscsi.py b/storage/iscsi.py index ff57cc5..8611d51 100644 --- a/storage/iscsi.py +++ b/storage/iscsi.py @@ -43,6 +43,8 @@ except ImportError: ISCSID="" INITIATOR_FILE="/etc/iscsi/initiatorname.iscsi"
+ISCSI_MODULES=['cxgb3i', 'bnx2i', 'be2iscsi']
def find_iscsi_files(): global ISCSID if ISCSID == "": @@ -193,6 +195,15 @@ class iscsi(object): os.makedirs(fulldir, 0755)
log.info("iSCSI startup")
iutil.execWithRedirect('modprobe', ['-a'] + ISCSI_MODULES,stdout="/dev/tty5", stderr="/dev/tty5")# this is needed by Broadcom offload cards (bnx2i)brcm_iscsiuio = iutil.find_program_in_path('brcm_iscsiuio',raise_on_error=True)log.debug("iscsi: brcm_iscsiuio is at %s" % brcm_iscsiuio)iutil.execWithRedirect(brcm_iscsiuio, [],stdout="/dev/tty5", stderr="/dev/tty5")# run the daemon iutil.execWithRedirect(ISCSID, [], stdout="/dev/tty5", stderr="/dev/tty5") time.sleep(1)
Ales Kozumplik (akozumpl@redhat.com) said:
Includes brcm_iscsiuio daemon for bnx2i cards as well as loading the drivers and spawning the daemons in stage2 storage code.
Why not have the daemon started by a udev rule on successful driver attach?
Bill
On 11/29/2010 05:55 PM, Bill Nottingham wrote:
Ales Kozumplik (akozumpl@redhat.com) said:
Includes brcm_iscsiuio daemon for bnx2i cards as well as loading the drivers and spawning the daemons in stage2 storage code.
Why not have the daemon started by a udev rule on successful driver attach?
Bill
Good point.
Mike, sorry for another mail today, is there a reason why there isn't a udev rule to start brcm_iscsiuio when bnx2i successfully attaches?
Ales
On 11/29/2010 06:24 PM, Ales Kozumplik wrote:
On 11/29/2010 05:55 PM, Bill Nottingham wrote:
Ales Kozumplik (akozumpl@redhat.com) said:
Includes brcm_iscsiuio daemon for bnx2i cards as well as loading the drivers and spawning the daemons in stage2 storage code.
Why not have the daemon started by a udev rule on successful driver attach?
Bill
Good point.
Mike, sorry for another mail today, is there a reason why there isn't a udev rule to start brcm_iscsiuio when bnx2i successfully attaches?
Ales
On a second thought: during normal boot brcm_iscsiuio is started along with iscsid and manual load of the drivers (they are not picked automatically for some reason related to how the partial offload work with the network drivers). This way everything's kept in one place and it's simple for the maintainer to change this, without having to touch udev rules that are always a bit of pain.
Anaconda just does (a subset of) what the /etc/init.d/iscsid does.
Ales
On 11/29/2010 11:24 AM, Ales Kozumplik wrote:
On 11/29/2010 05:55 PM, Bill Nottingham wrote:
Ales Kozumplik (akozumpl@redhat.com) said:
Includes brcm_iscsiuio daemon for bnx2i cards as well as loading the drivers and spawning the daemons in stage2 storage code.
Why not have the daemon started by a udev rule on successful driver attach?
Bill
Good point.
Mike, sorry for another mail today, is there a reason why there isn't a udev rule to start brcm_iscsiuio when bnx2i successfully attaches?
There is no technical reason why not. It is probably due to lack of experience with udev, so we did not see the option before.
On Mon, Nov 29, 2010 at 03:37:56PM +0100, Ales Kozumplik wrote:
+def find_program_in_path(prog, raise_on_error=False):
- for d in os.environ["PATH"].split(":"):
os.pathsep is the portable way to get ':'
full = "%s/%s" % (d, prog)
I'd use os.path.join() to make sure there aren't any ugly // in the path
if os.access(full, os.X_OK):return full
Kill the extra spaces please :)
- if raise_on_error:
raise RuntimeError("Unable to locate a needed executable: '%s'" % prog)- return None
@@ -43,6 +43,8 @@ except ImportError: ISCSID="" INITIATOR_FILE="/etc/iscsi/initiatorname.iscsi"
Spaces..
+ISCSI_MODULES=['cxgb3i', 'bnx2i', 'be2iscsi']
def find_iscsi_files(): global ISCSID if ISCSID == "": @@ -193,6 +195,15 @@ class iscsi(object): os.makedirs(fulldir, 0755)
And a couple more spaces.
Hi,
On 11/29/2010 07:51 PM, Brian C. Lane wrote:
On Mon, Nov 29, 2010 at 03:37:56PM +0100, Ales Kozumplik wrote:
+def find_program_in_path(prog, raise_on_error=False):
- for d in os.environ["PATH"].split(":"):
os.pathsep is the portable way to get ':'
I stole the function from master. I agree that os.pathsep() and os.path.join() are a better solution, but I prefer having the branches same even more.
+ISCSI_MODULES=['cxgb3i', 'bnx2i', 'be2iscsi']
- def find_iscsi_files(): global ISCSID if ISCSID == "":
@@ -193,6 +195,15 @@ class iscsi(object): os.makedirs(fulldir, 0755)
And a couple more spaces.
I can't see what spaces you mean. Are you talking about newline after ISCSI_MODULES or is your email client messing things up? In the web archive it looks as I intended, or are there problems still? https://www.redhat.com/archives/anaconda-devel-list/2010-November/msg00257.h...
Ales
os.pathsep is the portable way to get ':'
I stole the function from master. I agree that os.pathsep() and os.path.join() are a better solution, but I prefer having the branches same even more.
Reasonable.
Feel free to update master as well, though. There's plenty of places we could use os.pathsep if we really wanted to be picky.
- Chris
On Tue, Nov 30, 2010 at 09:50:20AM +0100, Ales Kozumplik wrote:
Hi,
On 11/29/2010 07:51 PM, Brian C. Lane wrote:
On Mon, Nov 29, 2010 at 03:37:56PM +0100, Ales Kozumplik wrote:
+def find_program_in_path(prog, raise_on_error=False):
- for d in os.environ["PATH"].split(":"):
os.pathsep is the portable way to get ':'
I stole the function from master. I agree that os.pathsep() and os.path.join() are a better solution, but I prefer having the branches same even more.
I say fix them both ;)
+ISCSI_MODULES=['cxgb3i', 'bnx2i', 'be2iscsi']
def find_iscsi_files(): global ISCSID if ISCSID == "": @@ -193,6 +195,15 @@ class iscsi(object): os.makedirs(fulldir, 0755)
And a couple more spaces.
I can't see what spaces you mean. Are you talking about newline after ISCSI_MODULES or is your email client messing things up? In the web archive it looks as I intended, or are there problems still? https://www.redhat.com/archives/anaconda-devel-list/2010-November/msg00257.h...
Sometimes I forget that most people's editors don't show extra whitespaces. The first one had about 8, the other two had 2 spaces all on lines with no following content.
Here's the vim config (from dcantrell) to show tabs as ___| and spaces as a gray . if you're interested.
" Hilight tabs and trailing spaces set list listchars=tab:|_,trail:. highlight SpecialKey ctermfg=DarkGray
On Tue, 2010-11-30 at 08:55 -0800, Brian C. Lane wrote:
On Tue, Nov 30, 2010 at 09:50:20AM +0100, Ales Kozumplik wrote:
Hi,
On 11/29/2010 07:51 PM, Brian C. Lane wrote:
On Mon, Nov 29, 2010 at 03:37:56PM +0100, Ales Kozumplik wrote:
+def find_program_in_path(prog, raise_on_error=False):
- for d in os.environ["PATH"].split(":"):
os.pathsep is the portable way to get ':'
I stole the function from master. I agree that os.pathsep() and os.path.join() are a better solution, but I prefer having the branches same even more.
I say fix them both ;)
+ISCSI_MODULES=['cxgb3i', 'bnx2i', 'be2iscsi']
def find_iscsi_files(): global ISCSID if ISCSID == "": @@ -193,6 +195,15 @@ class iscsi(object): os.makedirs(fulldir, 0755)
And a couple more spaces.
I can't see what spaces you mean. Are you talking about newline after ISCSI_MODULES or is your email client messing things up? In the web archive it looks as I intended, or are there problems still? https://www.redhat.com/archives/anaconda-devel-list/2010-November/msg00257.h...
Sometimes I forget that most people's editors don't show extra whitespaces. The first one had about 8, the other two had 2 spaces all on lines with no following content.
Here's the vim config (from dcantrell) to show tabs as ___| and spaces as a gray . if you're interested.
" Hilight tabs and trailing spaces set list listchars=tab:|_,trail:. highlight SpecialKey ctermfg=DarkGray
Anaconda-devel-list mailing list Anaconda-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/anaconda-devel-list
Ales is an emacs guy, so no vim help for him :)
but I wonder how did you find those whitespaces in the email? In evolution, even if I select the area, it does not show any trailing whitespaces...
Ales is an emacs guy, so no vim help for him :)
We'll have to work on that... ;)
but I wonder how did you find those whitespaces in the email? In evolution, even if I select the area, it does not show any trailing whitespaces...
I'm using mutt+vim for the replies, so it uses all my normal vimrc rules when I'm editing an email. I wouldn't be surprised if evolution/thunderbird/etc. stripped off some of the hanging spaces.
full = "%s/%s" % (d, prog)I'd use os.path.join() to make sure there aren't any ugly // in the path
Gotta be careful with os.path.join, though:
commit 9fb8b3c594fda5b6fcebbae18cbbf7aae4255fd7 Author: Chris Lumens clumens@redhat.com Date: Thu Mar 27 15:52:32 2008 -0400
os.path.join does not work the way we think it should.
"If any component is an absolute path, all previous components are thrown away, and joining continues."
-- http://docs.python.org/lib/module-os.path.html
- Chris
On Tue, 2010-11-30 at 10:23 -0500, Chris Lumens wrote:
full = "%s/%s" % (d, prog)I'd use os.path.join() to make sure there aren't any ugly // in the path
Gotta be careful with os.path.join, though:
commit 9fb8b3c594fda5b6fcebbae18cbbf7aae4255fd7 Author: Chris Lumens clumens@redhat.com Date: Thu Mar 27 15:52:32 2008 -0400
os.path.join does not work the way we think it should. "If any component is an absolute path, all previous components are thrown away, and joining continues." -- http://docs.python.org/lib/module-os.path.html
- Chris
Anaconda-devel-list mailing list Anaconda-devel-list@redhat.com https://www.redhat.com/mailman/listinfo/anaconda-devel-list
that's exactly why I stopped using it too, got almost my whole workstation wiped out, when I was testing the removing of files in lorax installtree...
On Tue, Nov 30, 2010 at 04:33:20PM +0100, Martin Gracik wrote:
On Tue, 2010-11-30 at 10:23 -0500, Chris Lumens wrote:
full = "%s/%s" % (d, prog)I'd use os.path.join() to make sure there aren't any ugly // in the path
Gotta be careful with os.path.join, though:
commit 9fb8b3c594fda5b6fcebbae18cbbf7aae4255fd7 Author: Chris Lumens clumens@redhat.com Date: Thu Mar 27 15:52:32 2008 -0400
os.path.join does not work the way we think it should. "If any component is an absolute path, all previous components are thrown away, and joining continues." -- http://docs.python.org/lib/module-os.path.htmlthat's exactly why I stopped using it too, got almost my whole workstation wiped out, when I was testing the removing of files in lorax installtree...
Ouch!
I don't think I'll be using os.path.join much in the future.
Thanks!
Related: rhbz#442980 --- storage/udev.py | 45 +++++++++++++++++++++++++++++++++++++++------ 1 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/storage/udev.py b/storage/udev.py index 1659b74..59b7519 100644 --- a/storage/udev.py +++ b/storage/udev.py @@ -510,40 +510,73 @@ def udev_device_get_multipath_name(info): return info['ID_MPATH_NAME'] return None
-# iscsi disks have ID_PATH in the form of: +# iscsi disks' ID_PATH form depends on the driver: +# for software iscsi: # ip-${iscsi_address}:${iscsi_port}-iscsi-${iscsi_tgtname}-lun-${lun} +# for partial offload iscsi: +# pci-${pci_address}-ip-${iscsi_address}:${iscsi_port}-iscsi-${iscsi_tgtname}-lun-${lun} # Note that in the case of IPV6 iscsi_address itself can contain : # too, but iscsi_port never contains : -def udev_device_is_iscsi(info): + +def udev_device_is_sw_iscsi(info): + # software iscsi try: path_components = udev_device_get_path(info).split("-")
if info["ID_BUS"] == "scsi" and len(path_components) >= 6 and \ - path_components[0] == "ip" and path_components[2] == "iscsi": + path_components[0] == "ip" and path_components[2] == "iscsi": + return True + except KeyError: + pass + + return False + +def udev_device_is_partoff_iscsi(info): + # partial offload iscsi + try: + path_components = udev_device_get_path(info).split("-") + + if info["ID_BUS"] == "scsi" and len(path_components) >= 8 and \ + path_components[2] == "ip" and path_components[4] == "iscsi": return True except KeyError: pass
return False
+def udev_device_is_iscsi(info): + return udev_device_is_sw_iscsi(info) or udev_device_is_partoff_iscsi(info) + def udev_device_get_iscsi_name(info): + name_field = 3 + if udev_device_is_partoff_iscsi(info): + name_field = 5 + path_components = udev_device_get_path(info).split("-")
# Tricky, the name itself contains atleast 1 - char - return "-".join(path_components[3:len(path_components)-2]) + return "-".join(path_components[name_field:len(path_components)-2])
def udev_device_get_iscsi_address(info): + address_field = 1 + if udev_device_is_partoff_iscsi(info): + address_field = 3 + path_components = udev_device_get_path(info).split("-")
# IPV6 addresses contain : within the address, so take everything # before the last : as address - return ":".join(path_components[1].split(":")[:-1]) + return ":".join(path_components[address_field].split(":")[:-1])
def udev_device_get_iscsi_port(info): + address_field = 1 + if udev_device_is_partoff_iscsi(info): + address_field = 3 + path_components = udev_device_get_path(info).split("-")
# IPV6 contains : within the address, the part after the last : is the port - return path_components[1].split(":")[-1] + return path_components[address_field].split(":")[-1]
# fcoe disks have ID_PATH in the form of: # For FCoE directly over the NIC (so no VLAN and thus no DCB):
anaconda-devel@lists.fedoraproject.org
-
Ales Kozumplik -
Bill Nottingham -
Brian C. Lane -
Chris Lumens -
Martin Gracik -
Mike Christie