On Thu, 2018-07-19 at 11:38 -0500, Mike Christie wrote: > On 07/19/2018 10:37 AM, Bart Van Assche wrote: > > The general recommendation for configfs is that each attribute contains a > > single value, just like for sysfs. Patch 11/15 exports two values through > > a single attribute. Have you considered to split the above into two > > What about just making it the initiator port transport id so it aligns > with the get_initiator_port_transport_id() comment for the other patch. > For iscsi it would be 1 value with the format from SPC/SAM > "target_name,i,0x,isid"?
That sounds fine to me. > > attributes, namely the initiator name and the ISID? Can the initiator name > > be changed into a soft link to the se_node_acl configfs directory to make > > it easy for shell scripts to retrieve additional initiator configuration > > information? > > Because the kernel is creating the session instead of it being driven > from a mkdir, there are no existing functions for this. I do not know > configfs code well, but I think making a function to do this is possible > though. Initially configfs did not support creation of a directory from the kernel side. Last time I brought this up with Christoph he replied that this functionality has been added to configfs (if I understood Christoph correctly). > What about the dynamic_acl case? Just make those a normal file? I'm not that familiar with dynamic ACLs. Is there a difference between "dynamic ACL" generation and "demo mode"? How does this interact with the generate_node_acls mode? > Just to make sure we are on the same page too. The initiator name is > always the name of the acl right? It looked like that from > target_fabric_make_nodeacl but I was wondering if you are asking for the > symlink because there are some fabric module quirks where that is not > the case. If it's the same names, then you know the acl already from the > initiator name file. It depends on what is called the "initiator name". E.g. the SRP target driver supports multiple formats to refer to a single initiator port. The first version of the ib_srpt driver supported only 128-bit GIDs as initiator name. Since the 64-bit prefix of a GID may change due to a reboot, later on support for 64-bit GUIDs was added. During login three formats for the initiator name are verified: GID, GUID without "0x" prefix and GUID with "0x" prefix. In any case, target_alloc_session() will store a pointer to the first struct se_node_acl that matches in sess->se_node_acl. I think using the name stored in struct se_node_acl is fine in all cases. Bart.
