On 11/27/2014 12:31 AM, Gris Ge wrote:

...

NodeACL and MappedLUN are still used internally, but the API can be *Group classes and wwn strings entirely I think. It could be good.

Correct. If we have NodeACLGroup introduced, changing NodeACL as private/internal could be better. Or we will have two set of mapping work flow.

Maybe something like for #1 could be: NodeACLGroup.initiators() return ([wwn, ], [wwn_type])

Well what everyone else calls "initiators", LIO and rtslib call NodeACLs, so instead of using both words I think we would just stick with NodeACLs for consistency. So, have a NodeACLGroup.node_acls property that returns member wwns as strings.

What do you mean by wwn_type? All members of a group are within a tpg, which is within a target, which is of a fabric type (iscsi, etc.) so wwn_type should always be the same for all members of a group.

For #2: To hide NodeACL, we should do nodeacl.delete() inside of NodeACLGroup.remove_acl().

OK cool. That's what the commit I pushed last week does.

...

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?)

No MappedLUNGroup.parent_nodeaclgroup is not what I am looking for.

I am seeking a way to query a list of NodeACLGroup which have access to defined lun.

You can do this with a little work. Right now I am leaning towards targetd doing this work instead of rtslib -- rtslib should provide ways to get other immediately relevant objects but this is beyond that.

If MappedLUNGroup is a relationship from LUN to NodeACLGroup, then we might allow query this relationship in both directions:

 1. Query out which NodeACLGroups have access to LUN /tmp/lun_01.

 2. Query out which LUNs does NodeACLGroup test_ag_01 have access to.

OK, we can add MappedLUNGroup.tpg_lun, since all members of a MappedLUNGroup should refer to the same LUN. Then targetd can do:

def nags_using_lun(tpg, lun): matches = set() for nag in tpg.node_acl_groups: for mlung in nag.mapped_luns: if mlung.tpg_lun == LUN: matches.add(nag) return matches

or something.

Hopeful this wish list might help:

• Query NodeACLGroup and its initiator wwn, wwn_type, chap settings.

- initiator wwns, multiple, ok

- wwn_type, see above, should be able to get that from nag.parent_tpg.parent_target.fabric_module.name

- chap settings. should work.

• LUN mapping and unmap.

add for group with nag.mapped_lun(), unmap, first get the MappedLUNGroup object, then call MappedLUNGroup.delete().

• Add/Remove initiator into exist NodeACLGroup.

add_acl and remove_acl, ok

# Copy/Revoke existing mapping status and CHAP setting.

ok.

• Create NodeACLGroup with its first initiator. # New NodeACLGroup should hold no mapping or CHAP setting.
• Delete NodeACLGroup which delete initiator and their mapping, chap settings.

I think the way we want to go on this is that it's ok to have a NodeACLGroup with no members. This may happen before any members are added, or after the last one is deleted. We can tell if it's empty with NodeACLGroup.exists. NodeACLGroup.delete() still leaves a Python object in scope, it just doesn't refer to any NodeACLs, and .exists will return False.

• Get a list of LUN (maybe a list of target lun id) which current NodeACLGroup mapped to.

See above, doable in targetd if we have MappedLUNGroup.tpg_lun property.

• Get a list of NodeACLGroup (maybe a list of NodeACLGroup name) which current LUN mapped to.

MappedLUNGroup.tpg_lun should allow this too.

• Not allow empty NodeACLGroup.

Allowed but can test with NodeACLGroup.exists.

new stuff pushed to dev-groups github branch[1].

Regards -- Andy

[1] https://github.com/agrover/rtslib-fb/pull/56

On Wed, Dec 03, 2014 at 11:10:58AM -0800, Andy Grover wrote:

On 12/01/2014 05:12 PM, Andy Grover wrote:

...

def nags_using_lun(tpg, lun): matches = set() for nag in tpg.node_acl_groups: for mlung in nag.mapped_luns: if mlung.tpg_lun == lun: matches.add(nag) return matches

Hi Gris, I just had to push another commit to dev-groups to fix properties in MappedLUNGroup, but after doing so I was able to verify that the above code works properly.

Let me know how it goes.

Thanks! -- Andy

Hi Andy,

Thanks for the notice.

I have finished the targetd and lsm access group create/delete/edit/list, and currently working on masking.

I will provide feedback once I tried all methods I am looking for.

Thank you again. Best regards.

On 12/04/2014 08:27 AM, Gris Ge wrote:

Hi Andy,

Hi Gris,

I am working on masking issue. But here are some issues I found for now:

1. NodeACLGroup.node_acls A list of string(wwn). # Renamed to NodeACLGroup.wwns? TPG.node_acls A list of NodeACL

OK, makes sense.

2. NodeACL.mapped_luns A list of MappedLUN NodeACLGroup.mapped_luns A list of MappedLUNGroup # Renamed to NodeACLGroup.mapped_luns

I don't understand. Did you mean to say it should be renamed to NodeACLGroup.mapped_lun_groups?

3. User case of maintaining the consistance of mapping status. Example: Current debugfs status:

configfs :)

     IQN_1                   LUN_1
     AG_1
         INIT_2              LUN_2, LUN_3
         INIT_3              LUN_2, LUN_3

 Got a API request:
     Map LUN_4 to INIT_2

 Expected behaviour:
     A. Raise error as INIT_2 is a member of Access Group. Refuse initiator
        level mapping.
     B. Map the LUN_4 to AG_1.
     C. Map the LUN_4 to INIT_2 only and leave the mess to end-user.

Unfortunately I think option C. To do option A, we would need to make previously ok use of NodeACL now raise an error.

We have an existing low-level API via NodeACL class, that NodeACLGroup even uses. Users should never mix the low-level API (NodeACL) with the higher-level API (NodeACLGroup). We don't have a way to prevent this in the code so we just need to make it very clear in the documentation. I think that's the best we can do. We should discourage the use of NodeACL, and maybe some day we can deprecate and remove it as a public API.

 Which layer should handle the check and error raise if needed?
     A. user itself or upper app/lib.
     B. targetd/targetcli
     C. rtslibs

If the client is not using NodeACL then this condition cannot happen :)

There's always going to be the ability to go around and make a mess (inconsistent group members) because it's all just a wrapper around LIO's configfs tree, which has no groups, only individual nodeacls and tags.

4. The 'lun' sometimes refer to LUN class object, while sometimes refer to a integer number of target side LUN ID. The 'mapped_lun' sometimes refer to object of MappedLUN or integer number of host side LUN ID.

True. But we cannot change what exists in NodeACL or MappedLUN classes. At least in the *Group versions we can be consistent with their inconsistency :-)

Regards -- Andy

On 12/05/2014 06:30 AM, Gris Ge wrote:

...

I don't understand. Did you mean to say it should be renamed to NodeACLGroup.mapped_lun_groups?

Yes, NodeACLGroup.mapped_lun_groups.

Done

Please kindly comment on these issues also: 5. list(NodeACLGroup('not_exist_group_tag').mapped_luns) # Got IndexError("Group is empty") list(NodeACL(tpg, 'iqn.2000-04.not.exist:iqn').mapped_luns) # Got []

Fixed

6. Can NodeACLGroup automatically chose the host LUN ID like LUN class do?

NodeACLGroup.mapped_lun()? You shouldn't need to give it a tpg_lun, only the LUN to map it to. I don't think I understand this issue.

7. What's the maximum number of host LUN ID(NodeACLGroup.mapped_lun)? LUN.MAX_LUN?

Yes. BTW this value was incorrectly increased so I've put it back to 255 for now.

8. How to check whether a NodeACL is in a NodeACLGroup? Options A: NodeACL.tag # This require user know NodeACLGroup internal design. Options B: NodeACL.is_group_member # True or False

I thought we were trying to make it possible to use NodeACLGroups with no NodeACL objects. Are you finding you still have NodeACL objects?

Instead if you have node_wwns:

if "iqn.2006-01.com.example:1234" in nag.wwns: print "in the group"

9. Should we store CHAP info in NodeACLGroup? Current design of LibStorageMgmt is initiator level CHAP setting with ONTAP[1] and targetd supported only. If we store CHAP info in NodeACLGroup, we cannot have back-ward compatibility on this in LibStorageMgmt which does not have group level CHAP level design yet.

   Options A: rtslibs does not change CHAP info when a NodeACL join NodeACLGroup. With this, member of NodeACLGroup could have different CHAP setting. The libstoragemgmt could get full backward compatibility.

   Options B: No change to current rtslibs design. Add group CHAP setting support to libstoragemgmt API. Break targetd backward compatibility. ONTAP still can get full backward compatibility.

I've also been looking at ONTAP as a model but had not noticed that ontap igroups are not used for iscsi auth parameters, only lun mappings.

https://library.netapp.com/ecmdocs/ECMP1196995/html/GUID-CF01DCCD-2C24-4519-...

I am in a favor of option A which provide possible full backward
compatibility

I agree option A seems best. However, I think it's important to still enable the client to configure CHAP without having to use NodeACL objects. What I'd like to try is removing the chap_* properties from NodeACLGroup in favor of something like this:

def set_userids(self, value, wwn=None): ''' if wwn, set the userid for just that wwn, otherwise set it for all group members. ''' # ...

def get_userids(self): ''' returns a (wwn, userid) tuple for each member of the group. '''

repeat also for (password, mutual_userid, mutual_password).

This isn't quite as clean as the properties we have now but it would allow the client to configure different auth settings for each group member, as well as all-at-once, and read what is now a list instead of a scalar value.

We also would not set chap values in add_acl().

I also attached demo[2] patches for targetd and libstoragemgmt for quick test. The demo patch enabled targetd and libstoragemgmt with these features: 1. Access group create/destroy/query. 2. Add or remove initiator to/from access group. 3. Access group volume mask/mapping create/destroy/query.

Clearly, this indicate current design of rtslibs works and meet all libstoragemgmt access group requirements[3].

Great to hear! I'll take a look!

Regards -- Andy

On 12/10/2014 07:45 PM, Gris Ge wrote:

On Wed, Dec 10, 2014 at 11:17:22AM -0800, Andy Grover wrote:

...

...

6. Can NodeACLGroup automatically chose the host LUN ID like LUN class do?

NodeACLGroup.mapped_lun()? You shouldn't need to give it a tpg_lun, only the LUN to map it to. I don't think I understand this issue.

In LUN class, we have these codes for find next available target LUN ID: if lun is None: luns = [l.lun for l in self.parent_tpg.luns] for index in xrange(self.MAX_LUN+1): if index not in luns: lun = index break if lun is None: raise RTSLibError("All LUNs 0-%d in use" % self.MAX_LUN) else: lun = int(lun) if lun < 0 or lun > self.MAX_LUN: raise RTSLibError("LUN must be 0 to %d" % self.MAX_LUN)

So I am expecting to have similar code in NodeACLGroup.mapped_lun() to find next available host LUN ID.

Yes, the client should find (or create) a LUN object before creating a MappedLUN or MappedLUNGroup. targetd does this already when creating a MappedLUN, it should be exactly the same when creating a MappedLUNGroup.

...

...

8. How to check whether a NodeACL is in a NodeACLGroup? Options A: NodeACL.tag # This require user know NodeACLGroup internal design. Options B: NodeACL.is_group_member # True or False

I thought we were trying to make it possible to use NodeACLGroups with no NodeACL objects. Are you finding you still have NodeACL objects?

To achieve backward compatibility in libstoragemgmt targetd plugin(at lest we try to), we still want to provide initiator level LUN mapping in targetd. But for sure, it will be isolated from access group level LUN mapping. That's why we need to know we are not touching any access group member there.

In that case I'd say it's ok to just check if nodeacl.tag == None.

BTW I've renamed NodeACLGroup.mapped_lun() to .mapped_lun_group(), so that it matches the renamed mapped_lun_groups property name.

I'll be pushing a rebased and squashed set to the dev-groups branch today. If you want to take one last look, then I think we're close to being ready to merge it.

Thanks -- Regards -- Andy

On Thu, Jan 22, 2015 at 03:21:06PM -0800, Andy Grover wrote:

Hi Gris,

The rtslib changes are in so we can proceed with targetd changes.

Do you want to post some revised versions of these two patches (as they're marked "don't commit") or should I just take a look at these ones?

Thanks -- Regards -- Andy

Hi Andy,

I will send out the revised version next Monday for review.

Thanks. Best regards.