On 5/12/20 6:19 AM, Bodo Stroesser wrote:
> On 05/11/20 22:16, Mike Christie wrote:
>> On 5/11/20 2:21 PM, Bart Van Assche wrote:
>>> On 2020-05-11 11:39, Bodo Stroesser wrote:
>>>> On 05/10/20 23:57, Mike Christie wrote:
>>>>> This patch adds helpers to add/remove a dir per session. There is
>>>>> only 2
>>>>> files/dirs initially.
>>>>>
>>>>
>>>> ...
>>>>
>>>>> +
>>>>> +int target_sysfs_add_session(struct se_portal_group *se_tpg,
>>>>> + struct se_session *se_sess)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + /*
>>>>> + * Copy ACL name so we don't have to worry about mixing configfs
>>>>> + * and sysfs refcounts.
>>>>> + */
>>>>> + if (!se_sess->se_node_acl->dynamic_node_acl) {
>>>>> + se_sess->acl_name =
>>>>> kstrdup(se_sess->se_node_acl->initiatorname,
>>>>> + GFP_KERNEL);
>>>>> + if (!se_sess->acl_name)
>>>>> + return -ENOMEM;
>>>>> + }
>>>>> +
>>>>> + ret = kobject_add(&se_sess->kobj, se_tpg->sessions_kobj, "%s-%d",
>>>>> + se_sess->tpt_id->name, se_sess->sid);
>>>>> + if (ret) {
>>>>> + pr_err("Could not add session%d to sysfs. Error %d.\n",
>>>>> + se_sess->sid, ret);
>>>>> + goto free_acl_name;
>>>>> + }
>>>>> +
>>>>> + ret = add_transport_id_attrs(se_sess);
>>>>> + if (ret)
>>>>> + goto del_kobj;
>>>>> +
>>>>> + if (se_sess->tfo->session_attrs) {
>>>>> + ret = sysfs_create_group(&se_sess->kobj,
>>>>> + se_sess->tfo->session_attrs);
>>>>> + if (ret)
>>>>> + goto rm_tpt_id_grps;
>>>>> + }
>>>>> +
>>>>> + ret = sysfs_create_link(tcm_core_sessions_kobj, &se_sess->kobj,
>>>>> + se_sess->kobj.name);
>>>>
>>>> I would prefer to have links named "session-%d" or "%d" only, of course
>>>> with se_sess->sid as the value for '%d'.
>>
>> Yeah for the part of your comment that got chopped I can see your point.
>> For the dynamic acl case (userspace did not create an ACL so the kernel
>> made a tmp one), then doing session-$id will be easier for userspace to
>> lookup a specific session since it does not know the initiator name and
>> only knows the session id.
>
> Yes, I meant, that "session-%d" or even "%d" only would be fine for the
> links in scsi_target/sessions. When looking for a session if session-id
> is known only, then this is easy to use.
>
> Regarding the session folders in the tpgt_X folder, I think the "%s-%d"
> format with acl or initiator name followed by the session-id would be
> fine for both cases, with acl and with dynamic acl. Again the above
> links can be used for dynamic acl lookup.
>
I don't think that's possible.
To handle Greg's comment we will use device structs instead of kobjects
directly. It's going to end up looking like
drivers/scsi_transport_iscsi.c where that initiator side session has a
device struct in it. You have a scsi_target_session class and device
structs show up in /sys/class/scsi_target_session. In this case, you do
not have control over the symlink name from
/sys/class/scsi_target_session to /sys/devices/. The dir name and
symlink name are going to be the device structs name.
