Bug 714688 - Provide native systemd unit file
Provide native systemd unit file
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: iscsi-initiator-utils (Show other bugs)
19
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Chris Leech
Fedora Extras Quality Assurance
:
Depends On:
Blocks: SysVtoSystemd
  Show dependency treegraph
 
Reported: 2011-06-20 08:54 EDT by Jóhann B. Guðmundsson
Modified: 2014-05-26 05:46 EDT (History)
8 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-05-26 05:46:30 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Native systemd service file for iscsi service (411 bytes, text/plain)
2011-06-29 08:15 EDT, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for iscsid service (521 bytes, text/plain)
2011-06-29 09:14 EDT, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for iscsi service (395 bytes, text/plain)
2012-11-17 06:26 EST, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for iscsid service (427 bytes, text/plain)
2012-11-17 06:36 EST, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for iscsiuio service (335 bytes, text/plain)
2012-11-17 06:40 EST, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for Mikes upstream initscript (494 bytes, text/plain)
2012-11-19 06:19 EST, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for iscsi service (391 bytes, text/plain)
2012-11-19 06:24 EST, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for iscsid service (311 bytes, text/plain)
2012-11-19 06:26 EST, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for iscsi service (456 bytes, text/plain)
2012-11-19 06:49 EST, Jóhann B. Guðmundsson
no flags Details
iscsid modules to load (47 bytes, text/plain)
2012-11-19 07:34 EST, Tomasz Torcz
no flags Details
Native systemd service file for iscsi service (477 bytes, text/plain)
2012-11-19 07:41 EST, Jóhann B. Guðmundsson
no flags Details
Native systemd service file for iscsid service (311 bytes, text/plain)
2012-11-19 07:43 EST, Jóhann B. Guðmundsson
no flags Details
new nm-dispatcher script for iscsi (108 bytes, text/plain)
2012-11-19 09:08 EST, Jóhann B. Guðmundsson
no flags Details
Systemd patch with all the changes (18.98 KB, patch)
2012-11-19 18:34 EST, Jóhann B. Guðmundsson
no flags Details | Diff
Systemd patch with all the changes (20.74 KB, patch)
2012-11-20 04:38 EST, Jóhann B. Guðmundsson
no flags Details | Diff
Systemd patch with all the changes without the systemd requires macro (21.71 KB, patch)
2012-11-20 15:53 EST, Jóhann B. Guðmundsson
no flags Details | Diff
Systemd patch with iscsi pre shutdown check (26.30 KB, patch)
2012-11-21 13:28 EST, Jóhann B. Guðmundsson
no flags Details | Diff
Systemd patch with all the changes (27.11 KB, patch)
2012-11-21 13:47 EST, Jóhann B. Guðmundsson
no flags Details | Diff

  None (edit)
Description Jóhann B. Guðmundsson 2011-06-20 08:54:47 EDT
Description of problem:

https://fedoraproject.org/wiki/Features/SysVtoSystemd


Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:
Comment 1 Jóhann B. Guðmundsson 2011-06-27 12:49:36 EDT
What's the current status on this?

We need this in rawhide as soon as possible unless ofcourse you guys want to
block alpha.. 

https://fedoraproject.org/wiki/Packaging:Guidelines:Systemd
https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd
http://fedoraproject.org/wiki/Packaging:Tmpfiles.d
Comment 2 Hans de Goede 2011-06-28 04:29:16 EDT
The status of this is basically: "not going to happen" the iscsi init scripts are quite special doing a lot of stuff, but only under certain conditions. Unless someone volunteers to spend significant time on this, it is best to just stick with the old tried sysv initscripts.
Comment 3 Jóhann B. Guðmundsson 2011-06-28 06:02:16 EDT
Hum..

Ignoring the conversion wont simply make it go away it has to take place one time or another so we can just as well try to make atleast some head way into native systemd service files for iscsi worst case scenario is as you mention we stick with the old tried sysv initscripts. 

Now I propose that we approach this as we did in Bug 694738 ( iptables ) which involves splitting the old sysv init script into script snippets relevant to each starting/stopping trigger ( ExecStartPre=/usr/libexec/snippet1 ExecStartPost=/usr/libexec/snippet2 etc ) and simply would be called from the native systemd service file. That would allow us to cheat and convert the service with  the least amount of work and headache and without loosing the functionality of the old sysv legacy init script how does that sound?
Comment 4 Hans de Goede 2011-06-28 07:23:49 EDT
That sounds like a workable plan, but I simply have no time to work on this I'm afraid. Note that I'm not the iscsi-initiator-utils owner though (that would be Mike Christie), but I've done a lot of work on the initscripts in the past.

Mike, do you have time to work on this?

Regards,

Hans
Comment 5 Jóhann B. Guðmundsson 2011-06-28 15:04:36 EDT
Just out of curiosity in the long run is this service applicant for hardware activation unfortunately our guidelines are a bit short on how to do that 

"Hardware activation

Hardware activation occurs when a service is installed but only turns on if a
certain type of hardware is installed. Enabling of the service is normally done
with a udev rule. At this time we do not have further guidance on how to write
those udev rules. The service itself installs its .service files in the normal
places and are installed by the normal systemd scriptlets. These services
should never be enabled by the package as they will be enabled by udev. "

But you can read man systemd.device and look at the bluetooth service which is
doing that I believe..
Comment 6 Hans de Goede 2011-06-29 06:45:14 EDT
Parts of the current initscripts are likely candidates for hardware activation stuff, others are not. Note the Linux kernel contains a complete software iscsi initiator implementation, so if configured by the user / and admin iscsi can be used without any special hardware.
Comment 7 Jóhann B. Guðmundsson 2011-06-29 07:00:08 EDT
Just adding as side note since that I should have been a bit more clearer that the service file looks normal ( so looking at the bluetooh.service does not reveal anything ) but is activated by udev rule I think the general idea here is if hardware is present it trigger the service startup so there is no longer the need to "enable" the service it will just get activated if relevant hardware is present.

Example udev activations of target/services

/lib/udev/rules.d/99-systemd.rules

/lib/udev/rules.d/99-systemd.rules:SUBSYSTEM=="bluetooth", TAG+="systemd",
ENV{SYSTEMD_WANTS}="bluetooth.target"

Another example in /etc/udev/rules.d/99-hdapsd.rules

/etc/udev/rules.d/99-hdapsd.rules:SUBSYSTEM=="block", KERNEL=="sd[ab]",
ATTRS{removable}=="0", TAG="systemd", ENV{SYSTEMD_WANTS}="hdapsd@%k.service"

Vendor based would probably look something like this

ENV{ID_VENDOR_ID}=="$foo",ENV{ID_MODEL_ID}=="$bar" TAG+="systemd",
ENV{SYSTEMD_WANTS}="foo.target or bar.service" 

So once we have converted the relevant legacy init script an udev rule can be created and introduced later to activated the service or the user can manually start it if he is using the software iscsi initiator implementation.

I personally think this is an nifty udev+systemd feature which is not getting the attention it deserves unfortunately documentation about this is scarce as is
with so many other things ..
Comment 8 Hans de Goede 2011-06-29 07:38:38 EDT
Ok, let me try to provide some more details, there are 2 iscsi related sysv init scripts:

1) iscsi

It starts with loading helper modules for iscsi offloading capable cards, note that these modules should only be loaded if the iscsi functionality is used, so these kernel modules are not autoloaded based on pci-id or some such.

Its main function is to automatically log into any iscsi targets configured as such in the iscsi db in /var/lib/iscsi


2) iscsid

This starts the iscsidaemon which does the actual logging in, and stays running to handle re-connection  and re-login when necessary



The iscsid script gets run first and checks if iscsid is needed, it determines
this based on 2 factors:
1) is an iscsi target already active? (root on iscsi case)
2) are there targets configured in /var/lib/iscsi for auto login

Then the iscsi script will either run directly (in the old network scripts case), or from a NetworkManager trigger once networking is up.

To complicate things iscsiadm will start iscsid itself if it is not running yet when an iscsiadm command which needs it gets executed. One may be tempted to use socket activation for this, but note that in the case of / on iscsi iscsid needs to be started asap (for connection failure handling) and not wait for socket activation. Other then that socket activation should be fine. So assuming there is some way to force a non socket activated start for it, while still providing the socket to it, we could try to make iscisd do socket activation.

So with exception of the iscsi offload helper modules, about which I know too little. I think in an ideal world, this is how iscsi should work with systemd:

Hardware + socket based activation of iscsid (so either start it asap if iscsi targets are already logged into, or wait till iscsiadm needs iscsid to login to targets).

Have some trigger script which gets run each time a network interface becomes available (has completed either ipv4 or ipv6 address confiuration).

This script would walk over all iscsi targets in /var/lib/iscsi:
1) Marked for autologin? No -> continue with next target
2) Already logged in? Yes -> next target
3) Check if the target ip is reachable (by looking at the routing table, could
   use ip route to command for this. No -> next target
4) Tell iscsiadm to login to the target
5) continue with next target
Comment 9 Jóhann B. Guðmundsson 2011-06-29 08:15:43 EDT
Created attachment 510446 [details]
Native systemd service file for iscsi service

To test 

Drop this file into /lib/systemd/system directory 

# systemctl daemon-reload 
# systemctl start/stop/status iscsi.service
Comment 10 Jóhann B. Guðmundsson 2011-06-29 09:14:23 EDT
Created attachment 510465 [details]
Native systemd service file for iscsid service

I'm kinda leaning towards splitting "force_start" into separate service file and keep the normal startup in the regular one.

Also loading and unloading all these modules is it necessary afaik modules should be loaded via autoloading based on bus information, or via /etc/modules-load.d/*.conf. and unloading a module from the kernel should not be done except for debugging purposes
Comment 11 Jóhann B. Guðmundsson 2011-07-15 10:57:36 EDT
Alpha is closing fast how do you guys want to proceed with this?
Comment 12 Hans de Goede 2011-07-16 08:37:56 EDT
First of all thanks for the proposed service files, and some comments:

WRT iscsi.service:
-This should somehow force the start of iscsid.service, but only if the 
 ExecStartPre condition is true

WRT iscsid.service:
-You're starting iscsid unconditionally here, so even on systems where it is
 not necessary, this is undesirable

(In reply to comment #11)
> Alpha is closing fast how do you guys want to proceed with this?

I think it would be best to just keep using the old scripts for F-16 and target having native systemd files for F-17, rationale:
-As noted before this really needs someone with a significant amount of
 time to work on it, fixing the issues mentioned above, and doing some
 serious testing in various possible scenarios.
-If we do native systemd files (which we should) we should try to get them
 upstream, so that all distro's will be using the same set of systemd
 service files, this requires coordination with other distibutions
Comment 13 Jóhann B. Guðmundsson 2011-07-18 04:17:53 EDT
(In reply to comment #12)
> I think it would be best to just keep using the old scripts for F-16 and target
> having native systemd files for F-17, rationale:
> -As noted before this really needs someone with a significant amount of
>  time to work on it, fixing the issues mentioned above, and doing some
>  serious testing in various possible scenarios.
> -If we do native systemd files (which we should) we should try to get them
>  upstream, so that all distro's will be using the same set of systemd
>  service files, this requires coordination with other distibutions

Is it not possible to do the same as was done with iptables just move the legacy sysv init script to /usr/libexec and call from the start stop section in the unit file from there as in 

ExecStart=/usr/libexec/iscsi start
ExecStop=/usr/libexec/iscsi stop

and same for iscsid 

You would then technically make the switch to systemd but just call the legacy sysv init script which then would allow for any changes to be made in the future.

If that's not doable I guess we have to remove iscsi from the base group and or make sure it does not get installed by default.
Comment 14 Kostas Georgiou 2011-07-18 19:32:00 EDT
What is the benefit of moving the sysv init scripts to a new location and have systemd call them? Is it just so we can say we now use systemd?
Comment 15 Jóhann B. Guðmundsson 2011-07-18 20:11:09 EDT
As is mentioned here  http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd

"Packages are strictly forbidden from migrating to systemd within updates to a Fedora release. The migration is only allowed between Fedora releases."

switching to native systemd unit even if that does nothing more than calling the legacy sysv init scripts allows for a proper convertion happening later in the whole release cycle. 

Given that this is part of Base group the alternative is to remove it from it to not block the alpha release.
Comment 16 Ken Dreyer 2011-07-26 00:40:22 EDT
(In reply to comment #15)
> switching to native systemd unit even if that does nothing more than calling
> the legacy sysv init scripts allows for a proper convertion happening later in
> the whole release cycle. 

To be honest that just sounds like you're getting by on a technicality.
Comment 17 Jóhann B. Guðmundsson 2011-07-26 01:25:16 EDT
(In reply to comment #16)
> (In reply to comment #15)
> > switching to native systemd unit even if that does nothing more than calling
> > the legacy sysv init scripts allows for a proper convertion happening later in
> > the whole release cycle. 
> 
> To be honest that just sounds like you're getting by on a technicality.

I'll take it then as you have time converting this then exellent

we need it done preferable before thursday thanks.
Comment 18 Jóhann B. Guðmundsson 2012-02-12 08:11:58 EST
Ping what's the current status of this? 

Is upstream looking at this or ???
Comment 19 Mike Christie 2012-02-14 18:29:40 EST
(In reply to comment #18)
> Ping what's the current status of this? 
> 
> Is upstream looking at this or ???

Yes and no. I am also the upstream maintainer. Someone upstream was working on it but they got busy with other stuff.

What you suggest in comment #13 is workable. I am working on it now. I am going to email you some questions.
Comment 20 Jóhann B. Guðmundsson 2012-08-07 05:56:44 EDT
Where are we at with this for F18+ ?
Comment 21 Jóhann B. Guðmundsson 2012-11-17 06:26:33 EST
Created attachment 646756 [details]
Native systemd service file for iscsi service

Couple of fixes
Comment 22 Jóhann B. Guðmundsson 2012-11-17 06:36:51 EST
Created attachment 646757 [details]
Native systemd service file for iscsid service

Couple of fixes
Comment 23 Jóhann B. Guðmundsson 2012-11-17 06:40:05 EST
Created attachment 646758 [details]
Native systemd service file for iscsiuio service

Splitting out and introducing iscsiuio.service
Comment 24 Jóhann B. Guðmundsson 2012-11-17 06:47:55 EST
I never heard from Mike as he mentioned he was going to do in comment 19 and the Dan's are hitting some issues [1] with iscsi not being migrated to native systemd units with libvirt-sandbox/lxc-libvirt so we need to get a move on this.

I will need some feedback on missing functionality and ordering dependency in those units along with if the required modules aren't already loaded in the Fedora kernel and or if we should not then put the loading of them into an isci conf file under /etc/modules-load.d/ ( man modules-load.d for details )

1. http://lists.freedesktop.org/archives/systemd-devel/2012-November/007396.html
Comment 25 Tomasz Torcz 2012-11-19 04:57:13 EST
Johann,

there are several problems with iscsid.service:

1) forking is not reliable. I would suggest dropping Type= and PIDfile, and invoking iscsid with "-f". It would be perfect if upstream added Type=notify support, it shouldn't be hard

2) There is no ExecStopPre=; it should be just ExecStop=; but see 3)

3) there is no "!" operator to negate exit status

For iscsi.service, Wants=tgtd.service is IMO completely wrong. It is not reasonable to use iSCSI to access local storage. (tgtd and targetcli - latter of which is newer and in-kernel - provide "target" which is like server, it shares devices on network; this bug is about initiator-utils, acting as a client accessing remote storage).

This whole iscsi[d] script stack is quite convoluted and should be rethough. I think there should be:

1) iscsid.service starting the daemon. (could it be socket-activated? I don't know)

2) iscsi-autologin.service, with ExecStart=/usr/sbin/iscsiadm -m node --loginall=automatic, with Requires=iscsid.service; this is *THE* service making remote storage available locally.
Comment 26 Jóhann B. Guðmundsson 2012-11-19 06:00:11 EST
(In reply to comment #25)
> Johann,
> 
> there are several problems with iscsid.service:
> 
> 1) forking is not reliable. I would suggest dropping Type= and PIDfile, and
> invoking iscsid with "-f". It would be perfect if upstream added Type=notify
> support, it shouldn't be hard

The pid file gets written to

systemctl status iscsiuio.service iscsid.service && cat /var/run/iscsi*
iscsiuio.service - iSCSI UserSpace I/O driver
	  Loaded: loaded (/etc/systemd/system/iscsiuio.service; disabled)
	  Active: active (running) since Mon, 19 Nov 2012 10:36:55 +0000; 39s ago
	    Docs: man:iscsiuio(8)
	          man:iscsid(8)
	 Process: 23935 ExecStart=/usr/sbin/iscsiuio (code=exited, status=0/SUCCESS)
	Main PID: 23936 (iscsiuio)
	  CGroup: name=systemd:/system/iscsiuio.service
		  └ 23936 /usr/sbin/iscsiuio


iscsid.service - Open-iSCSI
	  Loaded: loaded (/etc/systemd/system/iscsid.service; disabled)
	  Active: active (running) since Mon, 19 Nov 2012 10:36:55 +0000; 38s ago
	    Docs: man:iscsid(8)
	          man:iscsiuio(8)
	          man:iscsiadm(8)
	 Process: 23929 ExecStop=/sbin/iscsiadm -k 0 2 (code=exited, status=0/SUCCESS)
	 Process: 23940 ExecStart=/usr/sbin/iscsid (code=exited, status=0/SUCCESS)
	Main PID: 23942 (iscsid)
	  CGroup: name=systemd:/system/iscsid.service
		  ├ 23941 /usr/sbin/iscsid
		  └ 23942 /usr/sbin/iscsid

Nov 19 10:36:55 localhost.localdomain iscsid[23940]: iSCSI logger with pid=23941 started!
Nov 19 10:36:56 localhost.localdomain iscsid[23941]: transport class version 2.0-870. iscsid version 2.0-872.18.f17
Nov 19 10:36:56 localhost.localdomain iscsid[23941]: iSCSI daemon with pid=23942 started!


> 2) There is no ExecStopPre=; it should be just ExecStop=; but see 3)

Hmm thought it was there 

> 3) there is no "!" operator to negate exit status
> 
> For iscsi.service, Wants=tgtd.service is IMO completely wrong. It is not
> reasonable to use iSCSI to access local storage. (tgtd and targetcli -
> latter of which is newer and in-kernel - provide "target" which is like
> server, it shares devices on network; this bug is about initiator-utils,
> acting as a client accessing remote storage).

That just means that the LSB header for iscsi is wrong since I only added it because of "# Should-Stop:       tgtd"

> This whole iscsi[d] script stack is quite convoluted and should be rethough.
> I think there should be:
> 
> 1) iscsid.service starting the daemon. (could it be socket-activated? I
> don't know)
> 
> 2) iscsi-autologin.service, with ExecStart=/usr/sbin/iscsiadm -m node
> --loginall=automatic, with Requires=iscsid.service; this is *THE* service
> making remote storage available locally.

We cant call the service "iscsi-autologin.service" then we lose backwards compatibility with "iscsi" . 

The thing is here if mikes https://github.com/mikechristie/open-iscsi/blob/master/etc/initd/initd.redhat init scripts are correct and what we are shipping wrong then we could probably just drop the iscsi init script and the unit. 

I'll upload a unit that's migrated from mikes upstream one
Comment 27 Jóhann B. Guðmundsson 2012-11-19 06:19:26 EST
Created attachment 647701 [details]
Native systemd service file for Mikes upstream initscript

Note that unloading modules is frowned upon by some in the community so that section should be removed afaik...
Comment 28 Jóhann B. Guðmundsson 2012-11-19 06:24:30 EST
Created attachment 647706 [details]
Native systemd service file for iscsi service

Removing Wants as per comment adding an missing After for iscsid
Comment 29 Jóhann B. Guðmundsson 2012-11-19 06:26:31 EST
Created attachment 647707 [details]
Native systemd service file for iscsid service

Cleanup as per comment also adding After iscsiuio.service
Comment 30 Jóhann B. Guðmundsson 2012-11-19 06:49:15 EST
Created attachment 647710 [details]
Native systemd service file for iscsi service

Adding an missing Require and BindTo along with sync to the unit which makes sense and should take care of the order in the unit. 

We can if we want also add restart on failure to iscsiuio.service and iscsid.service

Now we need the spec file adjustment and comments from the maintainers on the units 

Why for example is it necessary to removing kernel modules on shutdown
Comment 31 Tomasz Torcz 2012-11-19 07:34:53 EST
Created attachment 647716 [details]
iscsid modules to load

Attached file should be installed as /usr/lib/modules-load.d/iscsid.conf

iscsi.service needs RemainAfterExit=yes

With those two unit works correctly on my setup.

(I cannot test root-on-iSCSI => iscsid in initramfs case; I'm not sure if iscsid uses proper '@' protection on transition, like mdmonitor)
Comment 32 Jóhann B. Guðmundsson 2012-11-19 07:41:49 EST
Created attachment 647717 [details]
Native systemd service file for iscsi service

Adding the missing RemainAfterExit for the type oneshot unit
Comment 33 Jóhann B. Guðmundsson 2012-11-19 07:43:28 EST
Created attachment 647718 [details]
Native systemd service file for iscsid service

Fixing a type caught by Tomasz in his testing
Comment 34 Jóhann B. Guðmundsson 2012-11-19 09:08:05 EST
Created attachment 647770 [details]
new nm-dispatcher script for iscsi

Created new networkmanager dispatch script for iscsi. 

I think the most appropriate way is to use systemctl try-restart here as opposed to use what you guys where using. 

From the man page

try-restart [NAME...]
           Restart one or more units specified on the command line if the units are running. Do nothing if units are not running. Note that for compatibility with Red Hat init
           scripts condrestart is equivalent to this command.
Comment 35 Jóhann B. Guðmundsson 2012-11-19 12:23:40 EST
Managed to dig up https://bugzilla.redhat.com/show_bug.cgi?id=692230#c32 which explains why tgtd is there in the LSB header. It's enough to throw the services in the After line to keep iscsid from starting before tgdt has been started
Comment 36 Jóhann B. Guðmundsson 2012-11-19 18:34:29 EST
Created attachment 648180 [details]
Systemd patch with all the changes

Patch with all the changes thus obsoleting all the submitted unit files except Mike's migration encase he wants it upstream.
Comment 37 Jóhann B. Guðmundsson 2012-11-20 04:38:17 EST
Created attachment 648360 [details]
Systemd patch with all the changes

Fixing typos, using the  %systemd_requires macros nicely wraps Requires(post): systemd and similar %systemd_requires macros which nicely wraps Requires(post): systemd and similar in the spec file
Comment 38 Jóhann B. Guðmundsson 2012-11-20 15:53:15 EST
Created attachment 648830 [details]
Systemd patch with all the changes without the systemd requires macro

Reverting using systemd macro for requires since I've just been informed that usage of macro's for requires are frowned upon by FPC. 

It boils down to situations like this...
 %foo_requires
 Requires: bar >= 7
 foo_requires expaqnds to Requires: bar = 6
 Which wins?
 Which wins if the lines are in the reverse order?
 How was the packager to know that Requires: bar was part of that?

Hence denied and the systemd_requires macro subjected to removal in the not to distant future to avoid usage,confusion and violation of the FPG...
Comment 39 Chris Leech 2012-11-21 11:19:53 EST
I don't see what in the attached unit files is supposed to keep the sessions alive on system shutdown/reboot if we have the root filesystem mounted over iSCSI.

And in testing iSCSI root, I'm getting I/O errors and hung reboots.
Comment 40 Jóhann B. Guðmundsson 2012-11-21 11:40:49 EST
(In reply to comment #39)
> I don't see what in the attached unit files is supposed to keep the sessions
> alive on system shutdown/reboot if we have the root filesystem mounted over
> iSCSI.

Sorry not following you have to be a bit more specific on what parts of the initscript are keeping sessions alive across system shutdown/reboot
 
> And in testing iSCSI root, I'm getting I/O errors and hung reboots.

How about attaching those? 

Can you confirm that iscsi on root is working without these modification on F17?
Comment 41 Chris Leech 2012-11-21 12:11:39 EST
(In reply to comment #40)
> (In reply to comment #39)
> > I don't see what in the attached unit files is supposed to keep the sessions
> > alive on system shutdown/reboot if we have the root filesystem mounted over
> > iSCSI.
> 
> Sorry not following you have to be a bit more specific on what parts of the
> initscript are keeping sessions alive across system shutdown/reboot

The iscsi script doesn't stop sessions if it sees a mount with the _netdev tag in mtab.  The iscsid script doesn't shutdown iscsid if there are active sessions.  That's all in the stop() functions of each respective script.
  
> > And in testing iSCSI root, I'm getting I/O errors and hung reboots.
> 
> How about attaching those? 
>
> Can you confirm that iscsi on root is working without these modification on
> F17?

Yes, it was working fine.
Comment 42 Jóhann B. Guðmundsson 2012-11-21 12:39:20 EST
(In reply to comment #41)
> (In reply to comment #40)
> > (In reply to comment #39)
> > > I don't see what in the attached unit files is supposed to keep the sessions
> > > alive on system shutdown/reboot if we have the root filesystem mounted over
> > > iSCSI.
> > 
> > Sorry not following you have to be a bit more specific on what parts of the
> > initscript are keeping sessions alive across system shutdown/reboot
> 
> The iscsi script doesn't stop sessions if it sees a mount with the _netdev
> tag in mtab.  The iscsid script doesn't shutdown iscsid if there are active
> sessions.  That's all in the stop() functions of each respective script.

Then we need to put that in a separated script that gets executed with in an ExecStop=/path/to/script before we actually stop the service as in...

[Unit]
Description=Login and scanning of iSCSI devices
Documentation=man:iscsiadm(8) man:iscsid(8)
Requires=iscsid.service
BindTo=iscsid.service
After=network.target NetworkManager-wait-online.service iscsid.service
ConditionPathExists=/etc/iscsi/initiatorname.iscsi

[Service]
Type=oneshot
ExecStart=/sbin/iscsiadm -m node --loginall=automatic
ExecStop=/bin/sync
ExecStop=/usr/libexec/iscsi-check-active-sessions
ExecStop=/sbin/iscsiadm -m node --logoutall=automatic
RemainAfterExit=true

[Install]
WantedBy=multi-user.target

and 

[Unit]
Description=Open-iSCSI
Documentation=man:iscsid(8) man:iscsiuio(8) man:iscsiadm(8)
After=network.target NetworkManager-wait-online.service iscsiuio.service tgtd.service, please Add=targetcli.service

[Service]
Type=forking
PIDFile=/var/run/iscsid.pid
ExecStart=/usr/sbin/iscsid
ExecStop=/usr/libexec/iscsi-check-active-sessions
ExecStop=/sbin/iscsiadm -k 0 2

[Install]
WantedBy=multi-user.target
Comment 43 Jóhann B. Guðmundsson 2012-11-21 13:28:26 EST
Created attachment 649338 [details]
Systemd patch with iscsi pre shutdown check

Check if this works and or just adjust the script if necessary also take the time and read http://www.freedesktop.org/wiki/Software/systemd/RootStorageDaemons
Comment 44 Jóhann B. Guðmundsson 2012-11-21 13:47:40 EST
Created attachment 649352 [details]
Systemd patch with all the changes

removing again! the mess in the after line since I somehow managed to copy the wrong unit file into the git branch...
Comment 45 Fedora End Of Life 2013-04-03 10:10:54 EDT
This bug appears to have been reported against 'rawhide' during the Fedora 19 development cycle.
Changing version to '19'.

(As we did not run this process for some time, it could affect also pre-Fedora 19 development
cycle bugs. We are very sorry. It will help us with cleanup during Fedora 19 End Of Life. Thank you.)

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora19
Comment 46 Jóhann B. Guðmundsson 2014-05-26 05:46:30 EDT
Given that I have left the project and a new individual may or may not continue with systemd integration in the project by submitting new feature following whatever demands FPC and FESCo might have and thus new units in the process I'm closing this and all remaining bugs I had submitted for this particular feature as WONTFIX

Note You need to log in before you can comment on or make changes to this bug.