Re: Fwd: initiator group support in rtslib
by Andy Grover
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
9 years, 3 months
- 2
- 12
- ← Newer
- 1
- Older →