Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

targetcli: dump iscsi and vhost info as in targetcli #128

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

richl9
Copy link
Contributor

@richl9 richl9 commented Nov 19, 2024

Helper to reconstruct a targetcli-like structure and dump its iscsi and vhost sections info on iscsi target.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Nov 19, 2024
@richl9
Copy link
Contributor Author

richl9 commented Nov 25, 2024

Added struct type before address based on feedback from @biger410

Copy link
Member

@biger410 biger410 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for creating this helpers. Put some comments, please review, thanks.

"""
Get a list of tpg from tiqn

:param tiqn: ``struct iscsi_tiqn``
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

struct iscsi_tiqn *

"{}o- {} (struct iscsi_tiqn {})".format(
indent,
get_tiqn_name(tiqn),
hex(tiqn),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tiqn.value_()?

"{}o- {} (struct se_portal_group {}) ({})".format(
indent,
get_tpg_name(tpg.se_tpg),
hex(tpg),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tpg.value_()

"acl_list",
):
if not acl:
break
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question with above, why do this check?

Copy link
Contributor Author

@richl9 richl9 Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We check it since it may not be configured by users.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am not following. To me, acl is getting from a linked list, list_for_each_entry will never return a NULL until the linked list is corrupted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed it.

):
if not acl:
break
print(f"{indent * 3}o- {get_acl_name(acl)}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not clear about vhost, does it not have the lun mapping to each acl like iscsi target?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, but may not be configured by user.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the same, the output format for iscsi and vhost should be the same? I mean line 97 to 103 should be done for vhost as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I missed your point earlier. Just added it!

"struct se_lun", tpg.se_tpg.tpg_lun_hlist.address_of_(), "link"
):
if not lun:
break
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with above, why checking this?

@richl9
Copy link
Contributor Author

richl9 commented Dec 19, 2024

Thanks a lot for the comments. I have added the module checks and fixed the return type (e.g. struct iscsi_tiqn *) issues.

vhost_scsi_list = prog["vhost_scsi_list"]
except KeyError:
print("No vhost session found.")
return []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue with g_tiqn_list

print("o- vhost")
for tpg in for_each_vhost_tpg(prog):
if not tpg:
break
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above comment is still not addressed, i don't think we need the check in line 146 to 147.

"acl_list",
):
if not acl:
break
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am not following. To me, acl is getting from a linked list, list_for_each_entry will never return a NULL until the linked list is corrupted.

):
if not acl:
break
print(f"{indent * 3}o- {get_acl_name(acl)}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the same, the output format for iscsi and vhost should be the same? I mean line 97 to 103 should be done for vhost as well?

@@ -0,0 +1,286 @@
# Copyright (c) 2024, Oracle and/or its affiliates.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update to 2025

@richl9 richl9 force-pushed the richard/targetcli branch from 8adea83 to 9958f32 Compare January 4, 2025 00:10
Orabug: 37301968
Signed-off-by: Richard Li <[email protected]>
@richl9 richl9 force-pushed the richard/targetcli branch from 9958f32 to a1b2dc8 Compare January 4, 2025 00:11
Copy link
Member

@biger410 biger410 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for creating these helpers.

@biger410 biger410 merged commit b44af37 into oracle-samples:main Jan 7, 2025
4 checks passed
@richl9 richl9 deleted the richard/targetcli branch January 28, 2025 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants