Bug 225915
Summary: | Merge Review: iscsi-initiator-utils | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Nobody's working on this, feel free to take it <nobody> | ||||
Component: | Package Review | Assignee: | Nobody's working on this, feel free to take it <nobody> | ||||
Status: | CLOSED CURRENTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | rawhide | CC: | adrian, mchristi, opensource | ||||
Target Milestone: | --- | Flags: | adrian:
fedora-review+
|
||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2008-12-12 22:17:48 UTC | Type: | --- | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Attachments: |
|
Description
Nobody's working on this, feel free to take it
2007-01-31 19:07:32 UTC
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 statically-linked-binary /sbin/iscsistart. 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': /home/adrian/devel/fedora/rpms/iscsi-initiator-utils/F-7/open-iscsi-2.0-865/usr/login.c:168: 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 next meeting. 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 rpmlint complained. 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 somewhere. 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. |