On 11/26/2014 01:17 AM, Gris Ge wrote:
On Tue, Nov 25, 2014 at 07:54:50AM -0800, Andy Grover wrote:
> Hi Gris, saw your irc msg.
>
> Fwding an email I sent to targetcli-fb and targetd lists, not sure
> if you're on those. But yes, I was not going to merge the rtslib
> changes until we knew that they were what we needed for targetd. It
> sounds like you're signed up for the targetd work, so please let me
> know of any bugs you find or API changes to rtslib that would make
> your work easier.
>
> And then once we're happy with both, we can commit to both repos.
>
> Thanks! -- Andy
>
Hi Andy,
Hi Gris. I'm CCing the lists on my response, just for the sake of having
good technical discussions like this in public.
These might make targetd life easier:
1. Change NodeACLGroup._node_acls as a public property.
# It is require to get a list of group member NodeACL
I did agree, but then changed my mind, based on the below.
2. How about let NodeACLGroup.remove_acl() remove mapping status
also?
# If not, targetd have to do the mapping removal when remove a
# initiator(ACL) from a initiator group(NodeACLGroup).
nodeacl.delete() deletes the nodeacl as well as any mappedluns within it.
# It means targetd will use both initiator mapping and group
# mapping.
#
# I thought the intention of adding NodeACLGroup and MappedLUNGroup is
# to hide/replace the NodeACL and MappedLUN. Is it?
I wasn't sure, to be honest. How would this work? No methods in
NodeACLGroup or MappedLUNGroup would take or return NodeACLs or
MappedLUNs. This would mean #1 suggestion above should not be done.
NodeACL and MappedLUN are still used internally, but the API can be
*Group classes and wwn strings entirely I think. It could be good.
#
# My understanding of initiator group is:
# * Volume mapping status is maintained at group level.
# * Initiator in this group just inherit that mapping status.
# * Initiator add or remove means grant or revoke that inheritance.
3. It seems we have no quick way to query masked NodeACLGroup for
given LUN.
# We have the revert one:
# nag = NodeACLGroup(tpg, nag_name)
# nag.mapped_luns()
#
# I am expecting something like this:
# lun = LUN(tpg, lun=<target_lun_id>, storage_object=<st_obj>)
# lun.mapped_node_acl_groups()
That's not the reverse, nag.mapped_luns gives MappedLUNGroups. The
reverse is MappedLUNGroup.parent_nodeaclgroup. (Should we change
nag.mapped_luns property name to nag.mapped_lun_groups perhaps?)
4. NodeACLGroup.add_acl() didn't override old iSCSI CHAP
setting.
It doesn't? That's what the "if model.has_feature("auth")"
section was
supposed to do. Is that not working right? Or are you referring to
something else that was missed?
OK I'm going to update the dev-groups branch to change the usage model
to be more that users just use Group'd class versions exclusively. I
think just add_acl and remove_acl have to change.
Not tested, sorry. BTW USA workers have Thursday and Friday off so I
won't be around, but if this change is an improvement then we can sync
up on Monday, and if it's not, then we can also sync up on Monday :)
Thanks! -- Regards -- Andy