Red Hat Bugzilla – Bug 225915
Merge Review: iscsi-initiator-utils
Last modified: 2008-12-12 17:17:48 EST
Fedora Merge Review: iscsi-initiator-utils
Initial Owner: firstname.lastname@example.org
The soon attached patches will fix following problems with this package:
* empty debuginfo package
* E: iscsi-initiator-utils executable-marked-as-config-file /etc/rc.d/init.d/iscsi
* E: iscsi-initiator-utils executable-marked-as-config-file /etc/rc.d/init.d/iscsid
* W: iscsi-initiator-utils service-default-enabled /etc/rc.d/init.d/iscsi
* W: iscsi-initiator-utils service-default-enabled /etc/rc.d/init.d/iscsid
* using Requires(...) instead of Prereq
* changed buildroot
* using --preserve-timestamps for install
* replaced hardcoded paths with rpm macros
* conditional execution of chkconfig in scriptlets
The remaining warnings from rpmlint:
E: iscsi-initiator-utils non-readable /etc/iscsi/iscsid.conf 0600
W: iscsi-initiator-utils summary-not-capitalized iSCSI daemon and utility programs
E: iscsi-initiator-utils statically-linked-binary /sbin/iscsistart
W: iscsi-initiator-utils no-reload-entry /etc/rc.d/init.d/iscsi
W: iscsi-initiator-utils no-reload-entry /etc/rc.d/init.d/iscsid
The only which should probably be looked at is to get FESCO approval for
As I get following warning from the compiler:
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables -Wall -Wstrict-prototypes -I../include -DLinux
-DNETLINK_ISCSI=8 -D_GNU_SOURCE -static netlink.o util.o io.o auth.o login.o
log.o md5.o sha1.o iscsi_sysfs.o idbm.o initiator.o queue.o actor.o mgmt_ipc.o
isns.o transport.o iscsistart.o statics.o -o iscsistart
login.o: In function `resolve_address':
warning: Using 'getaddrinfo' in statically linked applications requires at
runtime the shared libraries from the glibc version used for linking
I am not sure, with this warning message, if static linking makes much sense here.
Created attachment 157612 [details]
Patch against devel branch
Very nice. Thanks a lot. I was trying to fix up the fc7 rpm, but when I ran
rpmlint I did not see the statically built warning. I also just got stuck on
some other warning I see you fixed.
Do you just check in the patch or do I?
Also /sbin/iscsistart is statically linked because it is used in the initramfs
like how nash or some of the lvm tools are. If we are doing dymaically linked
tools in the initramfs then I can fix that up. If not how do I get approval?
I can check in the changes on the F7 and/or devel branch if you remove the
pkg.acl file or add me to it.
I do not know about initramfs but if it is necessary FESCO will probably
approve. I can point FESCO to this ticket so that they can discuss this on their
Dynamically linked in the initramfs is fine for F7 and later.
I am not sure how to do the ACL stuff yet. I got to read that, but am a little
short on time.
I merged your patch into devel. I am also working on porting it to FC7. I will
also fix up the package so it does not compile iscsistart as static. Thanks
again for the review and patch.
Changing the service to not be enabled by default will break things for users
who actually have iSCSI as an FS during install. We don't do service enablement
in anaconda (and really, would prefer to keep things that way just because it's
the slope to madness...)
Really, the iscsi startup bits should probably be integrated into rc.sysinit
instead of being a separate initscript.
I have no problem if the iscsi services are enabled by default if there is (and
there seems to be) a good reason for it. I was just proposing the change because
From my point of view this review is approved because all open items have been
addressed and you close this bug.
A minor thing: I saw your check in into CVS and for F-7 the line in the
changelog was much longer than 80 characters. You should probably wrap that line
Jeremy, I will fix that up and start looking into rc.sysinit integration.
Adrian, I will follow 80 chars from now on. I already built the package and did
the release stuff, so it is on its way out. I will check in the change, so on
the next release that will be fixed.
Hey, one question about this package,
For iscsi it was requested that we create a library that would read special
iscsi boot values. For example IBM/intel want to read the value to use for boot
from the sysfs acpi tree, or for IBM and ppc from the open firmware tree.
iscsistart would use this library, and anaconda could then also use this library
if you wanted to do something like when you hit the advanced storage button,
anaconda could check for those special values being set and could then use them
for install and boot.
For the library, should I create another package/rpm and submit it for review or
can I just include it in this one? I thought I read that it must go in a new
devel package, but I cannot find where I read that.
According to comment: 8 and the fedora-review flag, this review was successfully completed, therefore I close it.