[Bug 744340] Review Request: targetcli - Configuration shell for kernel target subsystem

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=744340

Tomasz Torcz <zdzichu at irc.pl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|nobody at fedoraproject.org    |zdzichu at irc.pl
               Flag|                            |fedora-review?

--- Comment #2 from Tomasz Torcz <zdzichu at irc.pl> 2011-10-29 13:04:09 EDT ---
rpmlint output:
on SRPM:
targetcli.src: W: strange-permission targetcli.init 0775L
targetcli.src:11: W: macro-in-comment %{version}
targetcli.src:11: W: macro-in-comment %{version}
targetcli.src: W: invalid-url Source0: targetcli-1.99.2.gitb03ec79.tar.gz
1 packages and 0 specfiles checked; 0 errors, 8 warnings.


on RPM:
targetcli.noarch: E: explicit-lib-dependency python-rtslib
targetcli.noarch: W: spurious-executable-perm
/usr/share/man/man8/targetcli.8.gz
targetcli.noarch: W: service-default-enabled /etc/rc.d/init.d/targetcli
targetcli.noarch: W: service-default-enabled /etc/rc.d/init.d/targetcli
targetcli.noarch: W: no-reload-entry /etc/rc.d/init.d/targetcli
targetcli.noarch: E: subsys-not-used /etc/rc.d/init.d/targetcli
1 packages and 0 specfiles checked; 2 errors, 8 warnings.

(spelling fixes omitted)

The package is fine with all MUSTs in packagereview process.

But initscript is a no-no.

First, new packages for F16 and following version should provide systemd unit.
Optional SysV initscript need to go into subpackage.

Second, this scripts does many, many things which should be split:
- module load should be handled by snippets dropped in /etc/module-load.d/
- configfs mount should be handled by systemd .mount unit or left for a distro
to set up
- config restoration is handled by running few scripts. I see that you are
working with upstream to save config otherwise, but for know I recommend:
  - create systemd .service unit for each target type; Type=oneshot,
ConditionFile=/etc/target/lio_start.sh (and others), ExecStart the same .sh
  - create blocktarget.target, which requires:
    - sys-kernel-config.mount
    - all .service units created

This should make it fine.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.