[PATCH] scsi:iscsi: Record session's startup mode in kernel

6 views
Skip to first unread message

Wenchao Hao

unread,
Nov 22, 2022, 3:14:55 AM11/22/22
to Lee Duncan, Chris Leech, Mike Christie, James E . J . Bottomley, Martin K . Petersen, open-...@googlegroups.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, liuzhi...@huawei.com, linfe...@huawei.com, Wenchao Hao
There are 3 iscsi session's startup mode which are onboot, manual and
automatic. We can boot from iSCSI disks with help of dracut's service
in initrd, which would set node's startup mode to onboot, then create
iSCSI sessions.

While the configure of onboot mode is recorded in file of initrd stage
and would be lost when switch to rootfs. Even if we update the startup
mode to onboot by hand after switch to rootfs, it is possible that the
configure would be covered by another discovery command.

root would be mounted on iSCSI disks when boot from iSCSI disks, if the
sessions is logged out, the related disks would be removed, which would
cause the whole system halt.

So we need record session's start up mode in kernel and check this
mode before logout this session.

Signed-off-by: Wenchao Hao <haowe...@huawei.com>
---
drivers/infiniband/ulp/iser/iscsi_iser.c | 1 +
drivers/scsi/be2iscsi/be_iscsi.c | 1 +
drivers/scsi/bnx2i/bnx2i_iscsi.c | 1 +
drivers/scsi/cxgbi/libcxgbi.c | 1 +
drivers/scsi/iscsi_tcp.c | 1 +
drivers/scsi/libiscsi.c | 5 +++++
drivers/scsi/qedi/qedi_iscsi.c | 1 +
drivers/scsi/qla4xxx/ql4_os.c | 1 +
drivers/scsi/scsi_transport_iscsi.c | 4 ++++
include/scsi/iscsi_if.h | 1 +
include/scsi/libiscsi.h | 1 +
11 files changed, 18 insertions(+)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 620ae5b2d80d..778c023673ea 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -947,6 +947,7 @@ static umode_t iser_attr_is_visible(int param_type, int param)
case ISCSI_PARAM_IFACE_NAME:
case ISCSI_PARAM_INITIATOR_NAME:
case ISCSI_PARAM_DISCOVERY_SESS:
+ case ISCSI_PARAM_NODE_STARTUP:
return S_IRUGO;
default:
return 0;
diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c
index 8aeaddc93b16..a21a4d9ab8b8 100644
--- a/drivers/scsi/be2iscsi/be_iscsi.c
+++ b/drivers/scsi/be2iscsi/be_iscsi.c
@@ -1401,6 +1401,7 @@ umode_t beiscsi_attr_is_visible(int param_type, int param)
case ISCSI_PARAM_LU_RESET_TMO:
case ISCSI_PARAM_IFACE_NAME:
case ISCSI_PARAM_INITIATOR_NAME:
+ case ISCSI_PARAM_NODE_STARTUP:
return S_IRUGO;
default:
return 0;
diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c
index a3c800e04a2e..d1fb06d8a92e 100644
--- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
+++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
@@ -2237,6 +2237,7 @@ static umode_t bnx2i_attr_is_visible(int param_type, int param)
case ISCSI_PARAM_BOOT_ROOT:
case ISCSI_PARAM_BOOT_NIC:
case ISCSI_PARAM_BOOT_TARGET:
+ case ISCSI_PARAM_NODE_STARTUP:
return S_IRUGO;
default:
return 0;
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index af281e271f88..111b2ac78964 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -3063,6 +3063,7 @@ umode_t cxgbi_attr_is_visible(int param_type, int param)
case ISCSI_PARAM_TGT_RESET_TMO:
case ISCSI_PARAM_IFACE_NAME:
case ISCSI_PARAM_INITIATOR_NAME:
+ case ISCSI_PARAM_NODE_STARTUP:
return S_IRUGO;
default:
return 0;
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 5fb1f364e815..47a73fb3e4b0 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -1036,6 +1036,7 @@ static umode_t iscsi_sw_tcp_attr_is_visible(int param_type, int param)
case ISCSI_PARAM_TGT_RESET_TMO:
case ISCSI_PARAM_IFACE_NAME:
case ISCSI_PARAM_INITIATOR_NAME:
+ case ISCSI_PARAM_NODE_STARTUP:
return S_IRUGO;
default:
return 0;
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index d95f4bcdeb2e..1f2b0a9a029e 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -3576,6 +3576,8 @@ int iscsi_set_param(struct iscsi_cls_conn *cls_conn,
break;
case ISCSI_PARAM_LOCAL_IPADDR:
return iscsi_switch_str_param(&conn->local_ipaddr, buf);
+ case ISCSI_PARAM_NODE_STARTUP:
+ return iscsi_switch_str_param(&session->node_startup, buf);
default:
return -ENOSYS;
}
@@ -3712,6 +3714,9 @@ int iscsi_session_get_param(struct iscsi_cls_session *cls_session,
else
len = sysfs_emit(buf, "\n");
break;
+ case ISCSI_PARAM_NODE_STARTUP:
+ len = sysfs_emit(buf, "%s\n", session->node_startup);
+ break;
default:
return -ENOSYS;
}
diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c
index 31ec429104e2..b947a5bca380 100644
--- a/drivers/scsi/qedi/qedi_iscsi.c
+++ b/drivers/scsi/qedi/qedi_iscsi.c
@@ -1437,6 +1437,7 @@ static umode_t qedi_attr_is_visible(int param_type, int param)
case ISCSI_PARAM_BOOT_ROOT:
case ISCSI_PARAM_BOOT_NIC:
case ISCSI_PARAM_BOOT_TARGET:
+ case ISCSI_PARAM_NODE_STARTUP:
return 0444;
default:
return 0;
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 9e849f6b0d0f..1cb7c6dbe9d3 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -468,6 +468,7 @@ static umode_t qla4_attr_is_visible(int param_type, int param)
case ISCSI_PARAM_DISCOVERY_PARENT_IDX:
case ISCSI_PARAM_DISCOVERY_PARENT_TYPE:
case ISCSI_PARAM_LOCAL_IPADDR:
+ case ISCSI_PARAM_NODE_STARTUP:
return S_IRUGO;
default:
return 0;
diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index c3fe5ecfee59..39c14d2a8aad 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -4367,6 +4367,7 @@ iscsi_session_attr(tsid, ISCSI_PARAM_TSID, 0);
iscsi_session_attr(def_taskmgmt_tmo, ISCSI_PARAM_DEF_TASKMGMT_TMO, 0);
iscsi_session_attr(discovery_parent_idx, ISCSI_PARAM_DISCOVERY_PARENT_IDX, 0);
iscsi_session_attr(discovery_parent_type, ISCSI_PARAM_DISCOVERY_PARENT_TYPE, 0);
+iscsi_session_attr(node_startup, ISCSI_PARAM_NODE_STARTUP, 0);

static ssize_t
show_priv_session_state(struct device *dev, struct device_attribute *attr,
@@ -4488,6 +4489,7 @@ static struct attribute *iscsi_session_attrs[] = {
&dev_attr_sess_def_taskmgmt_tmo.attr,
&dev_attr_sess_discovery_parent_idx.attr,
&dev_attr_sess_discovery_parent_type.attr,
+ &dev_attr_sess_node_startup.attr,
NULL,
};

@@ -4587,6 +4589,8 @@ static umode_t iscsi_session_attr_is_visible(struct kobject *kobj,
return S_IRUGO;
else if (attr == &dev_attr_priv_sess_target_id.attr)
return S_IRUGO;
+ else if (attr == &dev_attr_sess_node_startup.attr)
+ param = ISCSI_PARAM_NODE_STARTUP;
else {
WARN_ONCE(1, "Invalid session attr");
return 0;
diff --git a/include/scsi/iscsi_if.h b/include/scsi/iscsi_if.h
index 5225a23f2d0e..e46e69c1fd02 100644
--- a/include/scsi/iscsi_if.h
+++ b/include/scsi/iscsi_if.h
@@ -610,6 +610,7 @@ enum iscsi_param {
ISCSI_PARAM_DISCOVERY_PARENT_IDX,
ISCSI_PARAM_DISCOVERY_PARENT_TYPE,
ISCSI_PARAM_LOCAL_IPADDR,
+ ISCSI_PARAM_NODE_STARTUP,
/* must always be last */
ISCSI_PARAM_MAX,
};
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index 654cc3918c94..af4ccdcc1140 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -327,6 +327,7 @@ struct iscsi_session {
char *boot_target;
char *portal_type;
char *discovery_parent_type;
+ char *node_startup;
uint16_t discovery_parent_idx;
uint16_t def_taskmgmt_tmo;
uint16_t tsid;
--
2.35.3

Mike Christie

unread,
Nov 22, 2022, 11:59:31 AM11/22/22
to Wenchao Hao, Lee Duncan, Chris Leech, James E . J . Bottomley, Martin K . Petersen, open-...@googlegroups.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, liuzhi...@huawei.com, linfe...@huawei.com
On 11/22/22 3:30 PM, Wenchao Hao wrote:
> There are 3 iscsi session's startup mode which are onboot, manual and
> automatic. We can boot from iSCSI disks with help of dracut's service
> in initrd, which would set node's startup mode to onboot, then create
> iSCSI sessions.
>
> While the configure of onboot mode is recorded in file of initrd stage
> and would be lost when switch to rootfs. Even if we update the startup
> mode to onboot by hand after switch to rootfs, it is possible that the
> configure would be covered by another discovery command.
>
> root would be mounted on iSCSI disks when boot from iSCSI disks, if the
> sessions is logged out, the related disks would be removed, which would
> cause the whole system halt.

The userspace tools check for this already don't they? Running iscsiadm
on the root disk returns a failure and message about it being in use.

Userspace can check the session's disks and see if they are mounted and
what they are being used for.

Wenchao Hao

unread,
Nov 22, 2022, 12:45:24 PM11/22/22
to Mike Christie, Wenchao Hao, Lee Duncan, Chris Leech, James E . J . Bottomley, Martin K . Petersen, open-...@googlegroups.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, liuzhi...@huawei.com, linfe...@huawei.com
On Wed, Nov 23, 2022 at 1:27 AM Mike Christie
<michael....@oracle.com> wrote:
>
> On 11/22/22 3:30 PM, Wenchao Hao wrote:
> > There are 3 iscsi session's startup mode which are onboot, manual and
> > automatic. We can boot from iSCSI disks with help of dracut's service
> > in initrd, which would set node's startup mode to onboot, then create
> > iSCSI sessions.
> >
> > While the configure of onboot mode is recorded in file of initrd stage
> > and would be lost when switch to rootfs. Even if we update the startup
> > mode to onboot by hand after switch to rootfs, it is possible that the
> > configure would be covered by another discovery command.
> >
> > root would be mounted on iSCSI disks when boot from iSCSI disks, if the
> > sessions is logged out, the related disks would be removed, which would
> > cause the whole system halt.
>
> The userspace tools check for this already don't they? Running iscsiadm
> on the root disk returns a failure and message about it being in use.
>

It seems we did not check.

> Userspace can check the session's disks and see if they are mounted and
> what they are being used for.

It's hard to check if iSCSI disk is in used. If iSCSI disk is used to
build multipath device mapper,
, and lvm is built on these dm devices, the root is mounted on these
lvm devices, like following:

sde 8:64 0 60G 0 disk
└─360014051a174917ce514486bca53b324 253:4 0 60G 0 mpath
├─lvm-root 253:0 0 38.3G 0 lvm /
├─lvm-swap 253:1 0 2.1G 0 lvm [SWAP]
└─lvm-home 253:2 0 18.7G 0 lvm /home

It's too coupling to check these dm devices.

Lee Duncan

unread,
Nov 22, 2022, 3:00:20 PM11/22/22
to Wenchao Hao, Chris Leech, Mike Christie, James E . J . Bottomley, Martin K . Petersen, open-...@googlegroups.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, liuzhi...@huawei.com, linfe...@huawei.com
The iscsiadm/iscsid tools refuse to logout of an ONBOOT session.

--
Lee Duncan

Wenchao Hao

unread,
Nov 22, 2022, 11:41:58 PM11/22/22
to Lee Duncan, Chris Leech, Mike Christie, James E . J . Bottomley, Martin K . Petersen, open-...@googlegroups.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, liuzhi...@huawei.com, linfe...@huawei.com
Sorry I did not highlight the key points. The root reason we need to record
node_startup mode in kernel is userspace's node_startup mode is unreliable in
some scenarios:

1. iscsi node and session is created in initrd, the configure files of these
nodes would be lost after we switch to rootfs
2. someone do iscsiadm -m discovery but did not specify the operation mode,
the iscsi node's node_startup would be updated to which specified in iscsid.conf
3. someone do iscsiadm -m node -o update to update nodes' configure

What's more, it seems "iscsiadm/iscsid" only refuse to logout of an ONBOOT
session when logout is specificed by "--logoutall". We still can logout an
ONBOOT session with "iscsiadm -m node -u comamnd".

Based on these analysis, I think we should record the node_startup mode in kernel
and check in userspace to avoid logout ONBOOT sessions.

Lee Duncan

unread,
Nov 23, 2022, 11:47:40 AM11/23/22
to Wenchao Hao, Chris Leech, Mike Christie, James E . J . Bottomley, Martin K . Petersen, open-...@googlegroups.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, liuzhi...@huawei.com, linfe...@huawei.com
That is not my experience. When I boot from iscsi root, after the system
is running, if I tell the iscsiadm to logout of the root iscsi target it
refuses. I will test again to verify.

> 2. someone do iscsiadm -m discovery but did not specify the operation mode,
> the iscsi node's node_startup would be updated to which specified in iscsid.conf

The default on iscsiadm discovery mode is to replace the info on a
discovered target, but there are other modes. And they don't effect the
current (root) session.

> 3. someone do iscsiadm -m node -o update to update nodes' configure

Again, does not effect the currently-running session, and can be
considered shooting oneself in the foot.

>
> What's more, it seems "iscsiadm/iscsid" only refuse to logout of an ONBOOT
> session when logout is specificed by "--logoutall". We still can logout an
> ONBOOT session with "iscsiadm -m node -u comamnd".

Again, I don't believe that's correct. I will test it.

Mike Christie

unread,
Nov 23, 2022, 12:21:50 PM11/23/22
to Wenchao Hao, Lee Duncan, Chris Leech, James E . J . Bottomley, Martin K . Petersen, open-...@googlegroups.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, liuzhi...@huawei.com, linfe...@huawei.com
On 11/22/22 10:41 PM, Wenchao Hao wrote:
> Sorry I did not highlight the key points. The root reason we need to record
> node_startup mode in kernel is userspace's node_startup mode is unreliable in
> some scenarios:
>
> 1. iscsi node and session is created in initrd, the configure files of these
> nodes would be lost after we switch to rootfs
> 2. someone do iscsiadm -m discovery but did not specify the operation mode,
> the iscsi node's node_startup would be updated to which specified in iscsid.conf
> 3. someone do iscsiadm -m node -o update to update nodes' configure
>
> What's more, it seems "iscsiadm/iscsid" only refuse to logout of an ONBOOT
> session when logout is specificed by "--logoutall". We still can logout an
> ONBOOT session with "iscsiadm -m node -u comamnd".

logout_by_startup does go by the startup setting, but I think you missed the
session_in_use related code. It checks the mounts and holders already. Just
change it for whatever you need. I think your lvm use case should be covered
by the holder check. If not, add it.

Wenchao Hao

unread,
Nov 23, 2022, 12:27:22 PM11/23/22
to Lee Duncan, Wenchao Hao, Chris Leech, Mike Christie, James E . J . Bottomley, Martin K . Petersen, open-...@googlegroups.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, liuzhi...@huawei.com, linfe...@huawei.com
I cloned the source code from github and built to test.
Here is my brief test steps:

1. discovery an iSCSI node and login, the default startup
mode is manual
2. using iscsiadm -m node -o update command to update
the startup mode to onboot(it did not refused)
3. logout the session with iscsiadm -m node -u
the logout succeed too.

Wenchao Hao

unread,
Nov 23, 2022, 12:39:06 PM11/23/22
to Mike Christie, Wenchao Hao, Lee Duncan, Chris Leech, James E . J . Bottomley, Martin K . Petersen, open-...@googlegroups.com, linux...@vger.kernel.org, linux-...@vger.kernel.org, liuzhi...@huawei.com, linfe...@huawei.com
I did not enable the iscsid.safe_logout in iscsid.conf, so the session
still could be logged out.
If tested with iscsid.safe_logout set to "Yes", the issue is solved.

Thanks a lot

Ulrich Windl

unread,
Nov 24, 2022, 5:06:09 AM11/24/22
to open-iscsi, haowe...@huawei.com
>>> "'Lee Duncan' via open-iscsi" <open-...@googlegroups.com> schrieb am
23.11.2022 um 17:47 in Nachricht
<0f7258d5-ff8e-fa4e...@suse.com>:
> On 11/22/22 20:41, Wenchao Hao wrote:

...
> Again, I don't believe that's correct. I will test it.
...
Maybe a session capture (via serial line or so) to show real facts would be helpful for the discussion.
Personally I think that information the kernel needs to continue working (e.g. the mount table) should be in the kernel.
Maybe user-land tools can manage the info there (in the kernel, via API), but the primary source should be the kernel.

Regards,
Ulrich


Wenchao Hao

unread,
Nov 24, 2022, 10:30:20 AM11/24/22
to open-iscsi
On Thursday, November 24, 2022 at 6:06:09 PM UTC+8 Uli wrote:
>>> "'Lee Duncan' via open-iscsi" <open-...@googlegroups.com> schrieb am
23.11.2022 um 17:47 in Nachricht
<0f7258d5-ff8e-fa4e...@suse.com>:
> On 11/22/22 20:41, Wenchao Hao wrote:

...
> Again, I don't believe that's correct. I will test it.
...
Maybe a session capture (via serial line or so) to show real facts would be helpful for the discussion.

Sorry, I can not understand this, could you describe more detail?

Ulrich Windl

unread,
Nov 25, 2022, 2:11:39 AM11/25/22
to open-iscsi
>>> "'Wenchao Hao' via open-iscsi" <open-...@googlegroups.com> schrieb am
24.11.2022 um 16:30 in Nachricht
<2d0439ba-7fb7-47ef...@googlegroups.com>:
> On Thursday, November 24, 2022 at 6:06:09 PM UTC+8 Uli wrote:
>
>> >>> "'Lee Duncan' via open-iscsi" <open-...@googlegroups.com> schrieb am
>> 23.11.2022 um 17:47 in Nachricht
>> <0f7258d5-ff8e-fa4e...@suse.com>:
>> > On 11/22/22 20:41, Wenchao Hao wrote:
>>
>> ...
>> > Again, I don't believe that's correct. I will test it.
>> ...
>> Maybe a session capture (via serial line or so) to show real facts would
>> be helpful for the discussion.
>
>
> Sorry, I can not understand this, could you describe more detail?

Wenchao Hao claimed something would not work correctly, and you doubted it.
So it would have bean easiest to demonstrate the issue (by Wenchao Hao) by some session capture starting from kernel boot.
That's what I trued to say. Probably easier than believing or not.

Regards,
Ulrich

>
>
>>
>> Personally I think that information the kernel needs to continue working
>> (e.g. the mount table) should be in the kernel.
>> Maybe user-land tools can manage the info there (in the kernel, via API),
>> but the primary source should be the kernel.
>>
>> Regards,
>> Ulrich
>>
>>
>>
>
> --
> You received this message because you are subscribed to the Google Groups
> "open-iscsi" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to open-iscsi+...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/open-iscsi/2d0439ba-7fb7-47ef-b52c-a866dc0c
> 86e1n%40googlegroups.com.




Ulrich Windl

unread,
Nov 25, 2022, 2:13:04 AM11/25/22
to open-iscsi
>>> Ulrich Windl schrieb am 25.11.2022 um 08:11 in Nachricht <63806AA5.95B : 161 :
60728>:
>>>> "'Wenchao Hao' via open-iscsi" <open-...@googlegroups.com> schrieb am
> 24.11.2022 um 16:30 in Nachricht
> <2d0439ba-7fb7-47ef...@googlegroups.com>:
> > On Thursday, November 24, 2022 at 6:06:09 PM UTC+8 Uli wrote:
> >
> >> >>> "'Lee Duncan' via open-iscsi" <open-...@googlegroups.com> schrieb am
> >> 23.11.2022 um 17:47 in Nachricht
> >> <0f7258d5-ff8e-fa4e...@suse.com>:
> >> > On 11/22/22 20:41, Wenchao Hao wrote:
> >>
> >> ...
> >> > Again, I don't believe that's correct. I will test it.
> >> ...
> >> Maybe a session capture (via serial line or so) to show real facts would
> >> be helpful for the discussion.
> >
> >
> > Sorry, I can not understand this, could you describe more detail?
>

Sorry, I swapped the roles of Wenchao Hao and Lee Duncan. Completely my fault...

Lee Duncan

unread,
Nov 30, 2022, 2:53:56 PM11/30/22
to Wenchao Hao, Wenchao Hao, Chris Leech, Mike Christie, open-...@googlegroups.com
[A much-reduced CC list, since my reply doesn't need wide distribution.]
>>>>> ISCSI_PARAM_DISCOVERY_PARENpatches.suse/block-switch-polling-to-be-bio-based.patchT_TYPE,
Let me start by saying I agree with you now, that there *is* an issue.
But your test was flawed.

After you log into a target, changing the Node database does nothing.
The node database is only referenced with you login using it, e.g.
"iscsiadm -m node ... -l".

But even if you logged out and then back into the target, thereby using
the updated Node DB entries, it would not have worked.

For one thing, "iscsiadm -m session -u" just logs out of all sessions,
as far as I can see, based on testing and code inspection. So that is a
problem.

Note that the iscsi.service systemd service file on SLES does "not" do
that. It instead logs of of "manual" and "automatic" session, but only
ones that are listed in the Node database.

And as you pointed out, any knowledge iscsid has of existing sessions is
lost if the daemmon dies or is stopped, then restarted. At that point,
the only knowledge is has about each session is what it finds in sysfs.

I have done some testing with your new kernel change that adds a
"node_startup" sysfs string attribute to session data. I modified
open-iscsi to pass in the node startup value, and that's a good start.
The next step is adding a "startup" value in the session structure,
filling it in from sysfs (or current state), and refusing to logout out
of sesions that have this set to "onboot", which all sounds fairly
simple. I also want to test with "iscsiadm -m fw -l", which is what I
believe is used when booting from software iscsi (i.e. iBFT).

Have you already worked on the open-iscsi side of this? No reason for
duplicate development.

--
Lee Duncan

Mike Christie

unread,
Nov 30, 2022, 3:08:26 PM11/30/22
to Lee Duncan, Wenchao Hao, Wenchao Hao, Chris Leech, open-...@googlegroups.com
On 11/30/22 1:53 PM, Lee Duncan wrote:
> Have you already worked on the open-iscsi side of this? No reason for duplicate development.

I think you missed his reply where he said he was missed the
iscsid.safe_logout setting.



Lee Duncan

unread,
Nov 30, 2022, 6:05:31 PM11/30/22
to Mike Christie, Wenchao Hao, Wenchao Hao, Chris Leech, open-...@googlegroups.com
No, I saw that, but I thought there were some cases that might be missed
by checking for mounts. I also think having to set "safe" mode globally
is inefficient, but that's a separate issue.

Was the plan to retrace the changes submitted and accepted upstream for
adding a sysfs entry for node_startup that won't be used? Or is the plan
to populate that attribute for user consumption, even though it isn't
needed for this particular problem?
--
Lee

Mike Christie

unread,
Nov 30, 2022, 6:53:47 PM11/30/22
to Lee Duncan, Wenchao Hao, Wenchao Hao, Chris Leech, open-...@googlegroups.com
On 11/30/22 5:05 PM, Lee Duncan wrote:
> On 11/30/22 12:08, Mike Christie wrote:
>> On 11/30/22 1:53 PM, Lee Duncan wrote:
>>> Have you already worked on the open-iscsi side of this? No reason for duplicate development.
>>
>> I think you missed his reply where he said he was missed the
>> iscsid.safe_logout setting.
>>
>>
>>
>
> No, I saw that, but I thought there were some cases that might be missed by checking for mounts. I also think having to set "safe" mode globally is inefficient, but that's a separate issue.

The safe logout code also checks for holders which covers things like lvm and
raid use. It doesn't handle things like general device openings though. Maybe we
want a general block layer use count file for that if we are worried about
that type of thing.

>
> Was the plan to retrace the changes submitted and accepted upstream for adding a sysfs entry for node_startup that won't be used? Or is the plan to populate that attribute for user consumption, even though it isn't needed for this particular problem?

Are you asking about the patch in this thread? If so, I don't think it's
merged.

Wenchao Hao

unread,
Dec 4, 2022, 8:02:17 AM12/4/22
to Lee Duncan, Wenchao Hao, Chris Leech, Mike Christie, open-...@googlegroups.com
Sorry I missed this message, I have modified open-iscsi to work
with this sysfs interface. But I think we do not need this any more
because the safe logout can avoid disks being removed.

Checking holders and if disk is mounted before logout seems enough,
so ignore this discussion.

Thank you very much for your reply.

Lee Duncan

unread,
Dec 5, 2022, 12:14:00 PM12/5/22
to Wenchao Hao, Wenchao Hao, Chris Leech, Mike Christie, open-...@googlegroups.com
On 12/4/22 05:02, Wenchao Hao wrote:
> On Thu, Dec 1, 2022 at 3:53 AM Lee Duncan <ldu...@suse.com> wrote:
>>
...
I have some philosophical issues with using safe_logout.

It is off by default, which implies to me that it has overhead. If I'm
doing a lot of iscsi session start/stops, I don't want the overhead.
Otherwise, why not just use it all the time.

Also, it only checks for mounts. What about if some process has the
device open but isn't using it for a filesystem?

And since it has overhead, I'd rather just use it on root iscsi volumes.
I have not had a single problem report from folks that have ended a
session by accident that is mounted on. Since ending your root volume
iscsi session is fatal, I _would_ like to proactively avoid that
possibility. So I want to only set this attribute on iscsi root volumes,
which means it'd have to be a per-node (or per-session) attribute, not a
global one.

Lastly, I can imagine a time when I want to override safe_logout, say if
some process is stuck. So it'd be nice to have a "--force" option to end
a session even if safe_logout is set.

But as I said, these objections are philosophical/theoretical.

And for the record, I like the idea of tracking the "start mode" of
sessions. Right now, if I list the iscsi sessions, I can't tell which
ones where started from firmware, which were started in the initrd, and
which were just manually started. So tracking (and being able to
display) the startup mode would only be a good thing IMHO.
--
Lee D


Wenchao Hao

unread,
Dec 6, 2022, 12:01:23 PM12/6/22
to Lee Duncan, Wenchao Hao, Chris Leech, Mike Christie, open-...@googlegroups.com
I think we have two things to discuss here which we should not confuse.

1. We can logout iscsi sessions even if the related iscsi disks are
still in used
2. iscsiadm/iscsid would overwrite session's startup_mode

The first issue can be fixed if we can fix the second one.

The original purpose of my patch is to record session's startup_mode
in the kernel, so iscsiadm/iscsid can refer to it before logout session,
this can fix both the two issues.

While the safe_logout mode can solve the first issue in another way by
checking if iscsi disk is in used. But we did not enable safe_logout as
default and it can not cover the sense when iscsi disks in not mounted
nor used by multipath or lvm.

The safe_logout mode can not address the issue which iscsiadm/iscsid
can overwrite session's startup_mode.

Firstly, we need to come to one same conclusion that if it is a bug to
be fixed, then
discuss how to fix it.

If we treat this issue as a bug, we have two ways to fix it:

1. Recording session's startup_mode in kernel is a way to fix it
2. Checking if session has been created for this node before updating
the node's configure file is another way. But we must be strict our
users would not edit the configure files by hand.

Wenchao Hao

unread,
Dec 11, 2022, 9:52:08 AM12/11/22
to open-iscsi
My previous reply was in a mess format, here is the formatted one:
Reply all
Reply to author
Forward
0 new messages